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

Format on Save #12449

Closed
jrieken opened this issue Sep 22, 2016 · 16 comments
Closed

Format on Save #12449

jrieken opened this issue Sep 22, 2016 · 16 comments
Assignees
Labels
feature-request Request for new features or functionality

Comments

@jrieken
Copy link
Member

jrieken commented Sep 22, 2016

Given the infrastructure we have built for #239 we can offer something like files.formatOnSave whichs mean format a document whenever we are about to save it.

@jrieken jrieken self-assigned this Sep 22, 2016
@jrieken jrieken added this to the September 2016 milestone Sep 22, 2016
@jrieken jrieken added the feature-request Request for new features or functionality label Sep 22, 2016
@jrieken
Copy link
Member Author

jrieken commented Sep 22, 2016

cc @ramya-rao-a , @mattetti, @lukehoban I see that vscode-go has this which won't be needed much longer. With #239 we will add the onWillSaveTextDocument event and with this isse we will utilise our new infrastructure to format on save.

I wanna poke your brains wrt to format on save and auto save. This is obviously a hot topic because auto-save happens while typing meaning formatting might kick in unpleasantly. Did you get feedback in that direction? Do you have ideas - like not formatting when it's auto-save and messing for your cursors?

@jrieken
Copy link
Member Author

jrieken commented Sep 22, 2016

re #12449 (comment)

@HookyQR same question. How do people react to auto-save and format on save?

@jrieken
Copy link
Member Author

jrieken commented Sep 22, 2016

Proposal is to ignore formatting when auto-save happens in an focused editor and when the edits overlap with any of the cursors - https://github.com/Microsoft/vscode/blob/joh/formatOnSave/src/vs/workbench/api/node/mainThreadSaveParticipant.ts#L124

@ramya-rao-a
Copy link
Contributor

@jrieken Below is an excerpt from the README for the Go extension

If you do turn Auto Save on, you may also want to turn format-on-save off ("go.formatOnSave": "false"), so that it is not triggered while typing

It is suggested to not use both together.

There have been a few instances (microsoft/vscode-go#368, microsoft/vscode-go#447) which left users baffled when their code kept disappearing which was Auto Save triggering formatting that deleted unused imports or multiple new lines.
Long time users of the extension do not use this combination due to the exact same issue.

Since files.autosave is off by default, it was safe for go.formatOnSave to be on by default as per microsoft/vscode-go#253. The same issue has some feedback about this combination of settings.

@ramya-rao-a
Copy link
Contributor

@jrieken Would the new files.formatOnSave format all files during Save All ? There is a feature ask for the same in Go extension microsoft/vscode-go#279

@jrieken
Copy link
Member Author

jrieken commented Sep 23, 2016

re #12449 (comment) - yes formatOnSave will work together with Save All

@jrieken
Copy link
Member Author

jrieken commented Sep 23, 2016

re #12449 (comment) - we now don't format on auto save

@jrieken
Copy link
Member Author

jrieken commented Sep 23, 2016

@bpasero Can you make sure the participant is called when auto-save is true but when the user pressed cmd+s explicitly? In would be good if that doesn't count as auto save but explicit save.

@bpasero
Copy link
Member

bpasero commented Sep 23, 2016

@jrieken today all save attempts are ignored if the model is not dirty. though for this milestone I fixed another feature request where people wanted the save to go to disk even when the model is not dirty so that external file watchers get triggered (e.g. nodemon). My solution is not going through the model, but now that we have a use case for this, I can revisit that solution and make sure we go through the model even if the model is not dirty. This would then also count as a normal save and not auto save.

@jrieken
Copy link
Member Author

jrieken commented Sep 23, 2016

I see - I was assuming cmd+s is off when auto save is on. Reality is already a lot better then I assumed.

@bpasero
Copy link
Member

bpasero commented Sep 23, 2016

@jrieken yup, if auto save is on but the file is still dirty (because it was not auto saved yet), the save will still happen when manually saving by the user.

So the question is: do we want format on save even when the file is not dirty and the user saves manually.

@jrieken
Copy link
Member Author

jrieken commented Sep 23, 2016

stretch - let's wait for that request.

@HookyQR
Copy link
Contributor

HookyQR commented Sep 24, 2016

@jrieken I don't recall any issues being raised with auto save and format on save both being set for my extensions. I guess most users would recognise it as a problem pretty quick and turn one of them off.

@jrieken jrieken closed this as completed Sep 26, 2016
@jrieken jrieken mentioned this issue Sep 26, 2016
2 tasks
@daviwil
Copy link
Contributor

daviwil commented Oct 11, 2016

Sorry for asking here, I didn't find this information elsewhere. How does an extension register a formatting provider for a language? I don't see it in the list of contribution points. Thanks!

@jrieken
Copy link
Member Author

jrieken commented Oct 11, 2016

It happens in code using the registerXYZFormattingEditProvider-functions. That's more or less how you get into the Format Code action and we invoke that when saving

@Lonefy
Copy link

Lonefy commented Nov 22, 2016

@jrieken Sorry for reply late. i'm really busy for this several months and i'll try to fix the problem soon.

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
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
Projects
None yet
Development

No branches or pull requests

6 participants