Skip to content
This repository has been archived by the owner on Jul 15, 2023. It is now read-only.

Format-on-save screws up undo/redo stack #678

Closed
atombender opened this issue Dec 2, 2016 · 7 comments
Closed

Format-on-save screws up undo/redo stack #678

atombender opened this issue Dec 2, 2016 · 7 comments

Comments

@atombender
Copy link

For example, a simple Go file:

package main
func main() {}

Say I change main to foo and hit save. I'm using goimports to format; when save runs the formatter, nothing is changed because my code is already formatted correctly. However, VSCode-Go still seems to replace the entire document, which means that if I hit cmd-Z, nothing happens. I have to hit cmd-Z once more in order to undo my change.

VSCode-Go should realize that the newly formatted file is the same as the current file, and do nothing.

To be honest, I'd prefer if formatOnSave did not count as part of the undo stack. I'd prefer "save + undo" to undo the format and my last edit, which is a lot more rational; they do belong together, and I'd argue there's absolutely utility to undoing only the formatting but not your own edit. However, I don't know if VSCode offers the necessary plumbing to group undo operations like this. I noticed a very similar problem with the Autotrim extension.

@ramya-rao-a
Copy link
Contributor

go.formatOnSave is currently implemented in a hacky way. See https://github.com/Microsoft/vscode-go/blob/master/src/goMain.ts#L191

Which is basically

  • your edits are saved
  • formatting edits are saved
  • undo works on the last batch of edits which was formatting.

This was when there was no pre-save event supported by VS Code for extensions to use.

In VS Code 1.6, we now have API that supports this pre-save event. See onWillSaveTextDocument in http://code.visualstudio.com/updates/v1_6#_new-apis

With this, I believe we should be able to club user edits and formatting edits in a single save such that undo works on them together as you described.

I was planning to do this, but don't have time until the holidays are over.
You are welcome to give it a try though :)

@atombender
Copy link
Author

I discovered that there's also an icky race condition where undo is screwed up if you apply any edits before the formatter comes back. If this happens, something odd happens where if you undo, you can't redo to the newest change.

The formatter often takes 2-5 seconds on my machine (no idea why, I have a fast machine and it's not doing anything else; the files are often a thousand lines long, but that shouldn't cause slowness?) and of course I'm happily editing in the meantime.

I think the only sane thing is to ignore the formatter's output if you've made edits since it started.

@ramya-rao-a
Copy link
Contributor

@atombender The fact that the formatter takes so long is alarming.

Can you run the formatter from command line and compare the time taken?

Using the onWillSaveDocument will not help if formatter takes so long, because there is only so much time that it will wait for the extension to get back with edits from the formatter

@atombender
Copy link
Author

Well, it's not consistent. Go's tools sometimes misbehave — like godef using 100% CPU and having to be killed, or the gocode daemon crashing. The above formatter slowness might be related to a problem where goreturns would panic with a stack overflow (after a few seconds), something I discovered a while back and fixed with a go get -u. But I forget whether I wrote the above issue before or after that.

I think VSCode should work consistently even if the formatter is slow, though.

@atombender
Copy link
Author

I realized the behaviour I mentioned earlier isn't limited to a slow formatter, either. Steps:

  1. Make a change.
  2. Save.
  3. Undo the change.
  4. Save.
  5. Redo now doesn't work.

This is presumably because the formatter kicks in and adds a new operation on the stack, which effectively truncates the stack.

I've been burned by this a few times now, and lost changes as a result: I make an experimental undo and save, thinking I could redo to get back where I started, which isn't possible. From my perspective this is a major bug.

@atombender atombender changed the title No-op added to undo stack when formatOnSave is on and no formatting is done Format-on-save screws up undo/redo stack Dec 19, 2016
@ramya-rao-a
Copy link
Contributor

Closing in favor of #540 which will fix this issue as well

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Dec 19, 2017

With the work in #540 completed, the latest update to the Go extension (0.6.70) has the fix for this issue.

@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 23, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants