-
Notifications
You must be signed in to change notification settings - Fork 29.4k
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
Make it clear what happens when multiple formatters for one document apply #41882
Comments
How? A formatter has no name nor any other means of identification. We can use the extension name/id but that would only work if there is just one. Worst, the choice cannot be remembered because a formatter is registered with a document selector and (in theory) every document (even of the same language*) can be formatted by a different formatter. We have always recommended extensions to either ship more fine-grained, e.g. have a separate formatter extension (and let the user decide by installing/uninstalling) or to add an option to enable/disable their formatter (more commonly used). *) A sample is remote/live edit scenarios in which your extension host knows TypeScript files on your disk ( |
prettier-vscode did add a config However I still think there should be some visual indication when multiple formatters are being run on the same file. Now the order they are being run seem to be random and users thought the formatters are having bugs. I feel no one would want multiple formatters being run on the same file, with order they can't control. |
We don't run multiple formatters. We pick one and use that. It's just hard to pick one, esp when formatting happens from more subtle gestures like format on paste etc |
OK, running multiple formatter was a misunderstanding, but user not knowing which formatter getting picked isn't that good either. If the choice of formatter can't be given to users via a setting easily, how about showing a message like "Multiple formatter is available for this document. VS Code defaults to use xxx formatter. Please update your formatter setting if you would like to choose an alternative formatter"? |
Problem is the xxx because when an extension registers multiple formatters that are only available under certain condition... It's an uncommon scenario not something we should keep in mind |
Not sure if this is the place for this question but can't we run formatters sequentially? |
Interesting idea @amatiasq... Sequentially maybe but without restart because otherwise you might format endlessly. |
How about at least adding a way to get the current formatter(extension name - the one that will be used when you execute |
So let me ask you this, I've read the formatting extensions best practices, understand how it works. But in the case of my extension, which formats the HTML code inside of PHP files (which is very important to a large group of people including anyone who devs WordPress). Now currently I register this the suggested way, but this is more of a secondary type formatting extension, but now if someone wanted to say use phpcbf to format their PHP, now the user is stuck between which to use what to do. Me personally I run a shell command on save to run my phpcbf command and everything works fine. My only option if I want to truly play nice would be to do something like run my formatting (because I don't consider it to be the primary) in some sort of on save hook, spawn a shell command type situation. Or I would have to add the ability to to my extension for doing everything. Honestly I feel like it should work more like you register your extension as a formatting OPTION for said language, then have an option for say xyz language formatting where the value is an array of the registered extensions providing the service and you can just sort/remove whatever ones you want. If someone sorts my name higher, run mine first. If I don't show up, don't run it. If nothing is set at all you could basically do what is currently happening and run them in the extension activation order. Provider extension slug is the registered name. |
Just in case anyone was wondering I just changed to subscribing to onBeforeSave as to not force users to make a decision about what formatting they want more. Now it just picks up the slack and anyone could enable any other PHP extension that wants to register to the main hook. |
Recently prettier added HTML support: https://github.com/prettier/prettier/releases/tag/1.15.0 Users didn't know prettier hijacked the HTML formatter, as there is no indication as to which formatter is being used. Many users are opening issues: |
@octref Please to not change the title of this issue. As long as formatters are registered dynamically and as long as one extension can register many even for different documents, it's not feasible to make a user enumerate them. I you have strong opinion about this then please make a constructive proposal how this should work. |
You install 1 formatter, not more than that. For some languages like JS, TS built-in formatters exists and if you install an alternative formatter (like prettier) you must disable or uninstall the un-wanted formatter. Almost all formatters come with a setting for that, e.g. |
@jrieken I'm testing on the latest Insider. The new feature works great, however I'm not a fan of not running multiple formatters on save. Typically, formatters can be run successfully in sequence without any issues. For example: I have one extension for most of my code that doesn't affect docstrings, and one for docstrings. It's not worth running two commands every time I want to format - it should just happen. I think we really need both -- a "Format Document" command and a "Format Document With..." command, and the default behavior for "on Paste" and "on Save" should be all available formatters, but maybe allow configuring. Edit: realized you mentioned this above so it's not a bug. |
Also, recommending that all extensions include a setting to enable/disable formatting doesn't make much sense for extensions that exist solely for formatting - at that point, the user might as well just disable the extension. |
another side effect of the latest insiders release is once you do disable all formatters except 1 (prettier in my case). the cursor no longer retains its position but is moved to the bottom of the document. |
Yeah, that's correct. Extensions that only exist for format one file type a setting doesn't make sense. The setting is for extensions that format multiple different languages (like prettier) or for extensions that provider many things and a formatter (like veutur) |
This is a good suggestion but won't it be weird when format on save runs all formatters in sequence and format document makes you pick one? Somewhere along the road we decided that one formatter is all you want. I am happy to revisit that but then we should be consistent, e.g. offer "format with all" also as command. |
Maybe we should have an option that lets users select an ordered list of formatters to run on save/paste? |
@iansan5653 Ordered in the sense of 'running multiple formatters in that order'? |
Exactly. In that case, the user would expect multiple formatters to run so
there wouldn't be an issue.
The biggest issue I see with that is you'd have to set it per-language,
making for a somewhat complex setting.
Edit: guess I'm not bringing much new to the conversation; I'd only read the most recent comments. Anyway, that's the path I'd support personally.
|
…, show message when formatting document with first provider, #41882
re #41882 (comment). Format on Save and format on paste will now stay enabled and they continue to pick the first formatter. Which that is is printed on the status bar (when having many to choose from). Also, there is now 'Format Document With...' showing quick pick and 'Format Document' selecting a provider. |
@jrieken I think that's definitely an improvement. Quick question though - what's the definition of the first formatter? |
How can I change default formatter after picking one from the list? |
Your "[json]": {
"editor.defaultFormatter": "vscode.json-language-features"
} Alternatively, right-click, "Format Document With...", "Configure Default Formatter..." (in a file with the language whose associated formatter you want to change). |
@Gama11 sorry, my editor didn't refresh after update and I haven't been seeing these options (weird). Now everything works. Thanks anyway! |
I'm using prettier with Typescript Import Sort and both are formatting my TS files on save. I would like TS Import sort to do it's job and then prettier to make it standard but they are running in the opposite direction and I can see the flashing of prettier and then TS Import Sort breaking the whitespaces. |
It would even be helpful to see which extension has registered for which file type. |
Recently prettier added partial Vue support, and prettier-vscode starts to register itself as a Vue formatter. When user presses format, prettier-vscode was being used (instead of the formatter in Vetur).
vuejs/vetur#658
The user should be given the choice to select a formatter for a specific language.
The text was updated successfully, but these errors were encountered: