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 Document working, editor.formatOnSave=true broken #41194

Closed
gurgeous opened this issue Jan 5, 2018 · 16 comments · Fixed by #43702
Closed

Format Document working, editor.formatOnSave=true broken #41194

gurgeous opened this issue Jan 5, 2018 · 16 comments · Fixed by #43702
Assignees
Labels
formatting Source formatter issues verified Verification succeeded
Milestone

Comments

@gurgeous
Copy link

gurgeous commented Jan 5, 2018

  • VSCode Version: 1.19
  • OS Version: Mac OS 10.12.6

I am having some trouble with editor.formatOnSave=true when using the Ruby extension. It isn't formatting on save. This is strange, because a manual Format Document works fine. How is that possible?

I assumed this was user error or an extension bug. So I modified the extension locally and added some debug output to this file:

https://github.com/rubyide/vscode-ruby/blob/master/src/format/rubyFormat.ts

I verified that the correct edits were being returned from provideDocumentFormattingEdits. This makes sense since a manual Format Document works fine. What am I missing? Could this be a vscode bug? Not sure how to debug further.

@rebornix rebornix self-assigned this Jan 7, 2018
@bumbapartha
Copy link

bumbapartha commented Jan 11, 2018

This is happening for my TypeScript code as well. While saving, formatting is not working. Formatting is working if I do it separately. But my HTML files are getting formatted at the time of saving.

Here is my User Settings.

{
    "html.format.wrapLineLength": 80,
    "html.format.wrapAttributes": "force-aligned",
    "tslint.autoFixOnSave": true,
    "window.zoomLevel": 0,
    "editor.renderControlCharacters": true,
    "clang-format.executable": "C:/Users/MyUserId/AppData/Roaming/npm/clang-format",
    "editor.formatOnSave": true
}

@MikhailArkhipov
Copy link

This appears to happen when formatting takes longer than expected. I can repro this in Python as well. Python extension runs formatters in Python process and in a large file formatting may take 2-3-4 seconds. Format Document appears to be executing synchronously and succeeds which formatting on save appears to abandon formatting after a second or so. I.e. same file - when file is small, formatting on save works, but when content grows format on save stops working. Format Document always works.

@MikhailArkhipov
Copy link

https://github.com/Microsoft/vscode/blob/4fde7a37737b5ff2735568116f8ffa59084da509/src/vs/workbench/api/electron-browser/mainThreadSaveParticipant.ts

perhaps?

return new Promise<ISingleEditOperation[]>((resolve, reject) => {
			setTimeout(reject, 750);

@MikhailArkhipov
Copy link

Rather than setting hard budget, perhaps some sort of progress reporting would help

@MartinKavik
Copy link

@MikhailArkhipov Yeah, I've come to the same conclusion independently of this issue during debugging formatting in Elm extension (associated issue).

I think both time constants should be at least configurable from settings. Some progress reporting / throwing errors on timeouts to dev console will be nice.

I've confirmed this problem with 750ms limit when I've built VSC from source with that constant set to some bigger values (master branch from yesterday, Windows 10).

@rebornix I can try to create a PR if you or somebody else could suggest some correct way how to solve it or I'll try to investigate it deeper and come with some solution (I saw VSC's source for the first time yesterday ). Thanks!

@buyology
Copy link
Contributor

Seeing the same thing with the vscode-go plugin which recently switched over to this logic with that hard limit. This totally must be configurable.

@buyology
Copy link
Contributor

Sent a patch to make this configurable: #43702

@ramya-rao-a ramya-rao-a assigned jrieken and unassigned rebornix Feb 18, 2018
@ramya-rao-a ramya-rao-a added the formatting Source formatter issues label Feb 18, 2018
@ramya-rao-a
Copy link
Contributor

Also related #40030

@gurgeous
Copy link
Author

I think this is exacerbated by the opaqueness of the "format on save" feature. As a new user of vscode it's challenging to figure out what's happening:

  • Did my document reformat? (this isn't always obvious)
  • Did I configure vscode?
  • Did I configure the extension?
  • Did I configure the external formatting tool?
  • Is my PATH correct?

For formatters like prettier many of these questions don't apply, but I think it's an issue for other extensions. It would be worth adding some UX here to help the user understand what's happening.

@jrieken jrieken added this to the March 2018 milestone Feb 26, 2018
@jrieken
Copy link
Member

jrieken commented Feb 26, 2018

The problem for making this configurable is that it might not be clear that saving is blocked. You'd be surprised how often someone forgets that he/she made a settings change that affects your workflow. There should already be a progress UI that shows a message like "Running Save Participants..."

@ramya-rao-a
Copy link
Contributor

@jrieken Format on save is aborted in 750ms. There is no UI/message for either progress or for when formatting gets aborted for exceeding 750ms

@jrieken
Copy link
Member

jrieken commented Feb 26, 2018

Well, the message is delayed by 150ms (and showing for at least 150ms). The message is shown before calling any participant which run sequentially - that still makes 750ms for format on save and 1750 for onWillSave. That should make 2500ms in total (given a slow formatter and a slow will-save-handler)

@gurgeous
Copy link
Author

Now that you've pointed it out, I see a brief flash of Running Save Participants... when I hit Save. Might need some more consideration:

  • I never noticed it before. It's a long way from my cursor. In fact, I've usually started typing again so I have no reason to move my eyeball around the screen.
  • It's not on screen for very long (750ms?)
  • The message is inscrutable

@MikhailArkhipov
Copy link

MikhailArkhipov commented Feb 27, 2018

IMO any hard time limit is a problem. On large Python files formatting can take 3-4 seconds. Format Document command is not limited by time while saving is. This is at least inconsistent. provideDocumentFormattingEdits has cancellation token. When saving file VSC could display progress or at least status bar message with Press ESC to cancel and cancel the token if user decides so.

Python extension worked around this (for now) by detecting if document actually changed on save and if it was not, forcibly reformatting and then saving. This may not work if user decides to close dirty file and then clicks 'Yes' to the save prompt.

@buyology
Copy link
Contributor

Can't imagine a situation where one would sit and monitor the format on save-process and manually opt in to cancel it. That's just too much operational overhead. (If it's for debugging fatal errors in the extensions and the tools they call out to there already is the developer tool debug-route.)

In go at least, the pattern is that you hit save and expect fmt and imports to run all the time — sometimes in very quick succession for every little change you make — and you quickly get used to the fact that it takes some extra time from time to time when you a) work on large projects and b) your computer is hung up doing other stuff.

Also, a simple but great cue for understanding the progress of the formatting is the fact that your code is not yet correctly formatted, and you are quite OK with just waiting a bit longer or retrying by continuing working and hitting save again.

@MikhailArkhipov
Copy link

Looks like it is hard to discover and users are still confused. microsoft/vscode-python#1325. What's wrong with adding progress indicator with cancellation?

@vscodebot vscodebot bot locked and limited conversation to collaborators Apr 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
formatting Source formatter issues verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants