feat(prettier): use npx for project-local prettier #136
Conversation
|
@lhchavez @infokiller Looks like you've previously worked on prettier in vim-codefmt. I'd be interested in your thoughts on this. |
|
The general idea makes sense and the code looks clean. |
|
@infokiller Thanks for looking it over. Regarding the opinionated options, I think they don't belong in vim-codefmt. Prettier already supplies defaults. The role of vim-codefmt is to bridge between Vim and the formatter, not to introduce a new set of defaults. Principle of least surprise. However I think it makes sense, within reason, to propagate editor settings to formatters. As a user, I don't expect a formatter to run contrary to my Vim settings. For example, if I always write code at 100 columns width, I would like formatters to respect that instead of needing to configure them individually. One of the nice outcomes is that it works with editorconfig. Per-project settings propagate seamlessly to formatters. In fact, it would probably make sense for the prettier plugin to additionally respect Possibly this default policy should be settable by users. Perhaps there should be a global vim-codefmt setting that controls whether to inform formatters about Vim settings, so that users can easily change the policy in one place. What do you think? |
|
Thanks for the great explanation. Sounds good to me, and I personally would also prefer vim to pass its settings to the formatters. |
|
One note on inheriting formatting settings: it's not always an uncontroversial improvement to treat the buffer's style settings as the correct ones. We do see plenty of cases where repo owners configure opinionated local project style settings like .clang-format and that's supposed to trump textwidth, shiftwidth, etc., which devs sadly tend to ignore and expect things to "just work" when they save the file. There are also plugins like https://github.com/tpope/vim-sleuth that configure those settings with guesses and don't pay attention to local project configs like that. It would be great if something like editorconfig could clean up this mess for us, but do tread carefully on trying to change that behavior. |
|
@infokiller @dbarnett Thanks to both of you for your thoughtful comments. I just pushed a commit that refactors things and adds a new codefmt setting The idea here is that vim-codefmt can optionally apply buffer settings to formatters across the board, and it's easy for the user to disable this behavior globally for vim-codefmt. I updated both the prettier and zprint plugins as a demonstration of using this, although zprint is a somewhat confusing example because it doesn't take normal command-line options, and I wanted If you like where this is going, then I'll update the tests and also rebase into a few sensible commits rather than the existing stream-of-development. |
|
Also, I made |
agriffis
force-pushed
the
feat/npx-prettier
branch
from
da781a3 to
3d77748
Compare
agriffis
changed the title
|
@dbarnett This is ready for proper review and pull when you're ready... I rebased into four atomic commits, with tests passing on each commit. This PR now does two things:
I could do this as separate pull requests, but they'd be sequential anyway, so I settled for atomic commits. Let me know what you think. |
|
Sorry for the code review bomb... On the overall direction, I do like being able to hook the vim settings into the formatter's settings better, but I still have reservations about setting up a global I like the flag helper. Would you be up for sending a quick PR to add that first and streamline the scope of this PR? |
|
Thanks @dbarnett. Busy at the moment but I'll get back to this soon |
agriffis
force-pushed
the
feat/npx-prettier
branch
from
3d77748 to
81c546d
Compare
|
@dbarnett I knocked this down to purely npx prettier and killing prettier options, as two separate commits. All the stuff related to apply_vim_settings I'll move to a separate PR for reworking. |
agriffis
force-pushed
the
feat/npx-prettier
branch
from
81c546d to
da87eef
Compare
|
@dbarnett Let me know if you have any more questions about this |
dbarnett
left a comment
dbarnett
left a comment
There was a problem hiding this comment.
Can you run vimdoc to update the generated help file with your changes?
|
PR description looks stale. Can you update it to clarify the latest state of the PR? |
Prettier has its own defaults which can be overridden by user's .prettierrc and project-local package.json as well. Don't impose a new and conflicting set of defaults in vim-codefmt.
agriffis
force-pushed
the
feat/npx-prettier
branch
from
da87eef to
1e0f293
Compare
If npx is available, use it in preference to calling prettier directly. This enables project-local prettier, which is important for teams to use matching versions. This doesn't change anything for non-project prettier installs, because npx will fall back to global prettier. Since the existence of npx doesn't actually imply the existence of prettier, we have to call it once to find out if it's usable. Since this is expensive (compared to not calling it, anyway), cache the result as s:prettier_is_available and invalidate if prettier_executable changes.
agriffis
force-pushed
the
feat/npx-prettier
branch
from
1e0f293 to
05614d0
Compare
Done
Done |
agriffis
changed the title
|
Merged! Thanks for bearing with me on this one. =) |
4 participants
If npx is available, use it for running prettier. This will find a project-local prettier, if available, which allows everybody on the team to run the same version. Otherwise people have different prettier versions, and code formatting toggles back and forth between commits.
Since npx can't tell us if prettier is available, we check with
npx prettier --versionand cache the result.Also, since by default npx will do a temporary install of things you try to run, we use the
--no-installflag to avoid that. We only want npx to find what's already there.Along the way, kill the opinionated prettier options that were in vim-codefmt, since they unhelpfully override local and project config files.