Plugin support? - Githubissues

kachkaev commented 6 years ago

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 https://github.com/prettier/prettier-atom/issues/395 (even the same id 😄)

CiGit commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

kachkaev commented 6 years ago

Shared some thoughts related to the Atom package: https://github.com/prettier/prettier-atom/issues/395#issuecomment-374238950. They might map onto the VSCode extension too.

kachkaev commented 6 years ago

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 https://github.com/prettier/prettier/issues/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 https://github.com/prettier/prettier-vscode/issues/232#issuecomment-388325270 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 commented 6 years ago

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 commented 6 years ago

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, https://github.com/prettier/plugin-php/pull/416).

CiGit commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 https://github.com/prettier/prettier-atom/pull/404.

jahvi commented 5 years ago

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

felixfbecker commented 5 years ago

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

RiFi2k commented 5 years ago

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.