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

Make it clear what happens when multiple formatters for one document apply #41882

Closed
octref opened this issue Jan 19, 2018 · 68 comments
Closed
Assignees
Labels
feature-request Request for new features or functionality formatting Source formatter issues on-testplan
Milestone

Comments

@octref
Copy link
Contributor

octref commented Jan 19, 2018

  • VSCode Version: 1.19
  • OS Version: macOS 1.12.6

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.

@octref octref added the formatting Source formatter issues label Jan 19, 2018
@jrieken
Copy link
Member

jrieken commented Jan 23, 2018

The user should be given the choice to select a formatter for a specific language.

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 (file-scheme) and other TypeScript files from somewhere else, e.g. remote-scheme which are formatted by another entity, e.g. not the TS extension

@jrieken jrieken added info-needed Issue requires more information from poster under-discussion Issue is under discussion for relevance, priority, approach and removed info-needed Issue requires more information from poster labels Jan 23, 2018
@octref
Copy link
Contributor Author

octref commented Jan 23, 2018

prettier-vscode did add a config prettier.disableLanguages so this can be worked around. So I guess it's each extension's responsibility to make sure they don't clash with others.

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.

image

I feel no one would want multiple formatters being run on the same file, with order they can't control.

@jrieken
Copy link
Member

jrieken commented Jan 24, 2018

I feel no one would want multiple formatters being run on the same file,

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

@octref
Copy link
Contributor Author

octref commented Jan 24, 2018

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"?

@jrieken
Copy link
Member

jrieken commented Jan 25, 2018

VS Code defaults to use xxx 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

@amatiasq
Copy link

Not sure if this is the place for this question but can't we run formatters sequentially?

@jrieken
Copy link
Member

jrieken commented Feb 26, 2018

Interesting idea @amatiasq... Sequentially maybe but without restart because otherwise you might format endlessly.

@usernamehw
Copy link
Contributor

How about at least adding a way to get the current formatter(extension name - the one that will be used when you execute Format Document) ?

@RiFi2k
Copy link

RiFi2k commented Aug 12, 2018

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.

@RiFi2k
Copy link

RiFi2k commented Aug 12, 2018

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.

@jrieken jrieken added feature-request Request for new features or functionality and removed under-discussion Issue is under discussion for relevance, priority, approach labels Sep 11, 2018
@jrieken jrieken changed the title Allow users to select formatters when multiple formatters are available In the presence of multiple formatters run them sequentially Sep 11, 2018
@octref octref changed the title In the presence of multiple formatters run them sequentially Allow user to choose a formatter when multiple formatters are registered for the same language Nov 21, 2018
@octref
Copy link
Contributor Author

octref commented Dec 19, 2018

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:

#62996
#64446
#65199

@jrieken jrieken changed the title Allow user to choose a formatter when multiple formatters are registered for the same language In the presence of multiple formatters run them sequentially Dec 20, 2018
@jrieken
Copy link
Member

jrieken commented Dec 20, 2018

@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.

@jrieken
Copy link
Member

jrieken commented Mar 22, 2019

How do I set the default format for a file extension?

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. typescript.format.enable

@iansan5653
Copy link
Contributor

iansan5653 commented Mar 22, 2019

@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.

@iansan5653
Copy link
Contributor

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.

@tylerthehaas
Copy link

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.

@jrieken
Copy link
Member

jrieken commented Mar 25, 2019

...much sense for extensions that exist solely for formatting - at that point, the user might as well just disable the extension.

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)

@jrieken
Copy link
Member

jrieken commented Mar 25, 2019

Typically, formatters can be run successfully in sequence without any issues.

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.

@iansan5653
Copy link
Contributor

Maybe we should have an option that lets users select an ordered list of formatters to run on save/paste?

@jrieken
Copy link
Member

jrieken commented Mar 25, 2019

@iansan5653 Ordered in the sense of 'running multiple formatters in that order'?

@iansan5653
Copy link
Contributor

iansan5653 commented Mar 25, 2019 via email

jrieken added a commit that referenced this issue Mar 25, 2019
…, show message when formatting document with first provider, #41882
@jrieken
Copy link
Member

jrieken commented Mar 25, 2019

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
Copy link
Member

jrieken commented Mar 25, 2019

Next revision based on insiders-feedback. Thanks everyone so far. The goal is be less breaking from the behaviour in stable and yet be more explicit about what's happening:

Mar-25-2019 16-24-14

@jrieken jrieken closed this as completed Mar 25, 2019
@iansan5653
Copy link
Contributor

@jrieken I think that's definitely an improvement. Quick question though - what's the definition of the first formatter?

@lukejagodzinski
Copy link

How can I change default formatter after picking one from the list?

@Gama11
Copy link
Contributor

Gama11 commented Apr 8, 2019

Your settings.json should have a line like this you can modify:

"[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).

@lukejagodzinski
Copy link

lukejagodzinski commented Apr 8, 2019

@Gama11 sorry, my editor didn't refresh after update and I haven't been seeing these options (weird). Now everything works. Thanks anyway!

@amatiasq
Copy link

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.

@geri777
Copy link

geri777 commented Apr 11, 2019

It would even be helpful to see which extension has registered for which file type.

@vscodebot vscodebot bot locked and limited conversation to collaborators May 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality formatting Source formatter issues on-testplan
Projects
None yet
Development

No branches or pull requests