Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin support? #395

Closed
kachkaev opened this issue Mar 16, 2018 · 29 comments
Closed

Plugin support? #395

kachkaev opened this issue Mar 16, 2018 · 29 comments
Labels
enhancement locked Please open a new issue and fill out the template instead of commenting.
Milestone

Comments

@kachkaev
Copy link
Member

kachkaev commented Mar 16, 2018

Hi guys,

I'm curious if there is anything missing in prettier-vscode to support plugins. After locally installing prettier and prettier-plugin-elm (which I developed myself 😃 ), the plugin works from the command line, but not in the editor. In particular, pressing option+shift+f while editing a markdown file, does not format ```elm code blocks. Running yarn prettier same-file.md from the terminal does this job perfectly.

What could be causing the issue? Can it be that the way prettier is spawned does not let it search for prettier-plugin-* in node_modules?

UPD: Same issue in prettier/prettier-atom#395 (even the same id 😄)

@CiGit
Copy link
Member

CiGit commented Mar 17, 2018

As it doesn't work, seems we would need to investigate this...
We would need to understand how those plugins are listed by prettier. I think it has something to do with cwd()

Does it work if you install your plugin in $HOME/.vscode/extensions/esbenp.prettier-vscode[VERSION]

@kachkaev
Copy link
Member Author

kachkaev commented Mar 17, 2018

@CiGit when a code is being sent to Prettier by the extension, is it only a text or the metadata too? My conversation with @olsonpm regarding the same issue in Atom plugin suggests that Prettier may be picking a default language instead of markdown/elm/etc. and silently fails because the given code is not parsable. Can this be the case?

UPD: Probably not, since markdown itself gets parsed. Must be something to do with cwd 🤔

@CiGit
Copy link
Member

CiGit commented Mar 17, 2018

The minimum thing you have to pass to prettier.format is text and a parser if it's not JS. Seems we can also pass filepath for parser inference.

@kachkaev
Copy link
Member Author

kachkaev commented Mar 17, 2018

I installed the plugin into /Users/ak/.vscode/extensions/esbenp.prettier-vscode-1.2.2/node_modules/prettier-plugin-elm/ – still no effect. Also tried deleting local prettier and prettier-plugin-elm to fallback to the extension's instance, no effect either.

Running npm install -g prettier prettier-plugin-elm did not help as well. In all cases markdown gets formatted, but a fenced ```elm blocks do not.

I restarted VSCode a few times during these experiments to make sure there's no caching effect.

@kachkaev
Copy link
Member Author

kachkaev commented Mar 17, 2018

What does prettier.getSupportInfo() just before you call prettier.format(...)? Does it list languages from the plugins when these are installed?

@kachkaev
Copy link
Member Author

kachkaev commented Mar 19, 2018

Shared some thoughts related to the Atom package: prettier/prettier-atom#395 (comment). They might map onto the VSCode extension too.

@kachkaev
Copy link
Member Author

kachkaev commented May 11, 2018

Looks like prettier-vscode will partially work with plugins after they release Prettier 1.13 – I could test this by locally installing a beta version together with prettier-plugin-elm. Finally, formatting markdown files prettifies Elm code blocks in them too! I believe that this change in behaviour is related to prettier/prettier#4000, which is now closed.

Nevertheless, we can't say that plugins in VSCode are supported. Files with non-standard extensions (e.g. .elm or .php) are still not picked by vscode-prettier, which means that plugins only come into play in markdown code blocks 😕 A new method called getFileInfo() that is being introduced in 1.13 should be a solution here. This method can be called before an attempt to format any file and if the value of inferredParser returned is not null, then call format() (either with the just derived value for parser or by giving it a file path).

Another thing that's missing is falling back to a global Prettier if the local one is not found. The global copy can have some plugins installed, which is never the case for the built-in instance. See #232 (comment) for more details. I expect the use of a global Prettier with plugins to be pretty common, because not all people would not want to call npm install in PHP, Python, Java or Elm projects.

@CiGit
Copy link
Member

CiGit commented May 16, 2018

This method can be called before an attempt to format any file and if the value of inferredParser returned is not null, then call format() (either with the just derived value for parser or by giving it a file path).

Sadly we don't want to format any file but only files prettier supports.

I expect the use of a global Prettier with plugins to be pretty common, because not all people would not want to call npm install in PHP, Python, Java or Elm projects.

I think projects would have a local copy of their prettier + plugins.
You would not be able to have prettier@v1 + plugin-L1 in project A and prettier@v2 + plugin-L2 in project B on the same machine (vm)

@kachkaev
Copy link
Member Author

Sadly we don't want to format any file but only files prettier supports.

I understand. For that reason you can call a new method called getFileInfo() for any file and thus figure out if it's supported by the current instance of Prettier (given the plugins) or not.

I think projects would have a local copy of their prettier + plugins.
You would not be able to have prettier@v1 + plugin-L1 in project A and prettier@v2 + plugin-L2 in project B on the same machine (vm)

It'd be great if this could be avoided. Picking different plugins should be possible with the new API, no matter if you use Prettier as a node module or as a spawned CLI tool. Ideally, each project should be using its own instance of Prettier to avoid unexpected mismatch between what's happening in the editor and in CI later on. Depending on what pluginSearchDirs you pass to Prettier, different plugins can be loaded (see upcoming docs). Using a global copy of Prettier is what may non-JS users would want (e.g. PHP users, prettier/plugin-php#416).

@CiGit
Copy link
Member

CiGit commented May 16, 2018

I understand. For that reason you can call a new method called getFileInfo() for any file and thus figure out if it's supported by the current instance of Prettier (given the plugins) or not.

We register a formatter in VSCode API. In our implementation, we have to know on start which language id prettier supports.
If we would register a formatter for everything, other formatter would have trouble. even for languages prettier doesn't support.

You can't use a global copy if your projectS rely on a specific version (be it prettier or a plugin).

I don't see how useful pluginSearchDirs is. I would certainly install plugins alongside prettier and use plugin's auto-search feature.

@kachkaev
Copy link
Member Author

We register a formatter in VSCode API.

Can we do this dynamically (both in time and between projects)? If not, it might be worth opening an issue in VSCode to allow this.

Prettier does support an open list of languages and since recently, and this list cannot be derived in advance. Therefore, if VSCode does not know how to deal with it, it should learn to :–) Simply not doing what users will be expecting after installing Prettier plugins locally and globally does not look like the way to go. I'm sure the solution exists, it's just a matter of how many symbols in the equation we should change.

@CiGit
Copy link
Member

CiGit commented May 16, 2018

We use getSupportInfo to get all languages supported by prettier (vscodeLanguageIds). We get it in advance.

We can register a formatter at any time. Maybe not when you are formatting (it will be for next format I think).

@kachkaev
Copy link
Member Author

kachkaev commented May 16, 2018

Ideally, getSupportInfo() should be called every time Prettier's parent node_modules directory changes (regardless of it being local or global). In addition, a refresh should happen when Prettier config is saved. This way VSCode extension can pick new plugins after someone calls npm install -D @prettier/plugin-xyz or manually adds a bespoke plugin name to prettier.config.js.

One question though: what to do when a workspace contains several projects, each with their own Prettier plugins and so different values in gerSupportInfo()? 🤔

@dhirschfeld
Copy link

dhirschfeld commented Jul 3, 2018

I don't really understand the use case for having different versions of prettier? You use it to format your code - your code doesn't depend on it?

It's like I use VSCode to edit my code but my code doesn't depend on it and I don't want to install multiple versions of VSCode local to each project.

The only thing I would want to be project-local is configuration.

Just piping up here as a user who would love for the Python plugin to Just Work with VSCode. I have no knowledge of any technical hurdles/limitations why this might be difficult to support.

My envisaged usage would be a single global prettier instance where I could install the Python plugin and then have VSCode automatically able to format Python code in all of my projects.

Edit: Having just read #488 that seems a very good way forward!

@Epskampie
Copy link

Epskampie commented Jul 27, 2018

@dhirschfeld Let me explain. Two engineers working on the same project might have different global versions of prettier installed, which format slightly different. Every time they work on a file, their prettier makes those changes, leading to an endless string of meaningless git commits.

This is why you absolutely want the vscode plugin to work with a locally installed prettier that is fixed to a certain version.

@dhirschfeld
Copy link

dhirschfeld commented Jul 27, 2018

@Epskampie - isn't this solved by prettier picking up the local configuration file? i.e. a project commits their prettier configuration file to their repo then all users who clone the repo will get the same configuration and prettier will use the local config in preference to any global config.

i.e. you don't install a local version of the application in every repository - each repository simply has a local config which prettier uses for that repo.

@Epskampie
Copy link

Epskampie commented Jul 27, 2018

@dhirschfeld No, it isn't. The exact formatting that prettier does changes slightly for each version over time, as languages evolve and prettier evolves with it. This is normal for any code formatter, but you must take it into account.

@dhirschfeld
Copy link

If you specify a configuration for the formatting of your code I'd expect that to be respected by the formatter irrespective of whatever version it was.

I can appreciate that a configuration file might not imply a unique format but I'd expect changes in interpretation of the configuration to trigger a major version bump. I can also appreciate that it's not a perfect world though and things change and so I think it should be possible to install a local copy pinned to a particular version but it seems incredibly wasteful to me to require it.

Personally I'd be happy to trigger (minor) formatting changes with an Updated to prettier 2.0 commit. I do know people/projects where that wouldn't be acceptable practice so for them pinning a local version would be appropriate.

Anyway, just my 2c as a user - I have no insight into the code or design so take it FWIW!

@RobinMalfait
Copy link
Contributor

@Epskampie tooling in your editor are just nice to haves. If your team really wants to be consistent you install prettier locally and have pre commit hooks to guarantee that they use the same version.

Also requiring a local version is kind of useless, if you have personal projects you want prettier but don't need all the extra dependencies.

@Epskampie
Copy link

True, precommit hooks are also a good option.

I’m not opposed to a global version also working for personal stuff by the way, but explaining why you’d want local version. If I had to choose, I’d choose local version for reasons above.

@kachkaev
Copy link
Member Author

The latest versions of prettier-atom (>=0.55.0) finally use getFileInfo() to determine what files can be prettified and what not. This means that any prettier plugin is now supported in Atom 🎉 prettier-atom searches for the local version within a given project, then falls back to a globally installed one (npm install --global prettier prettier-plugin-XXX / yarn add --global prettier prettier-plugin-XXX) and if all this search has failed, picks prettier built into prettier-atom, which does not have any plugins attached. All works pretty nicely so far!

I'm sharing this because the new design of prettier-atom can serve as an inspiration for those who want to transform prettier-vscode. Most of the refactoring work was done in prettier/prettier-atom#404.

@jahvi
Copy link

jahvi commented Nov 23, 2018

@kachkaev's comment makes sense, hopefully something similar can be done for prettier-vscode

@felixfbecker
Copy link
Contributor

Format-on-save editor integration is crucial to make plugins useful and use Prettier in other languages like PHP

@RiFi2k
Copy link

RiFi2k commented Dec 6, 2018

Honestly more than anything the hamstring in VScode is only allowing one formatting extension to register itself as the provider for a certain language. You can activate two plugins that both register as the provider for say PHP but then it just comes down to a race more or less and only one can win and run during the save hook.

What I have always wanted to do was figure a way to configure multiple formatting extensions for a single file type and allow a config option to set the order they run in. Also it's rough you can't be aware in the vscode gui what extensions are even running on what file types, so a ton of people end up getting two extensions that both end up registering to format X file type then it causes major user confusion because they don't know why Z extension isn't working.

@esamattis
Copy link

Sent a PR for this #757

esamattis added a commit to esamattis/prettier-vscode that referenced this issue Mar 17, 2019
@kachkaev
Copy link
Member Author

kachkaev commented Aug 9, 2019

Some good news from the SalesForce Developer Tooling team – see #890 and https://twitter.com/ntotten/status/1159830608379437056 🎉

Looking forward 🚀

@ntotten ntotten added this to the 1.9.0 milestone Aug 9, 2019
@ntotten
Copy link
Member

ntotten commented Aug 10, 2019

Added with #899

@Shinigami92
Copy link
Member

🎉 Thank you all for making sure this works
I can confirm, that my own plugin for pug is working now with format on save

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot added the locked Please open a new issue and fill out the template instead of commenting. label Apr 12, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement locked Please open a new issue and fill out the template instead of commenting.
Projects
None yet
Development

Successfully merging a pull request may close this issue.