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

formatOnSave re-saves several times #1037

Closed
ghaabor opened this issue Jun 16, 2017 · 17 comments
Closed

formatOnSave re-saves several times #1037

ghaabor opened this issue Jun 16, 2017 · 17 comments

Comments

@ghaabor
Copy link

ghaabor commented Jun 16, 2017

So I've started using the Go extension yesterday and noticed a weird issue. When formatOnSave is on, I push Ctrl-S and the file is saved several times and the undo stack gets crowded with characters getting selected and deselected. It always selects the same characters that were before the cursor when I pressed Ctrl-S. I tried it with all three formatters and they all do the same.

I think there could be some bigger issue behind this because turning off formatOnSave stops this behavior but the characters in the file still get selected but only once (same here, the character before the cursor).

I use VSCode with Ruby as well and I haven't noticed anything similar. I use the latest Code (1.13.1 I think) and the latest extension. I'm on Ubuntu 17.04.

@ramya-rao-a
Copy link
Contributor

That sounds like some other extension is interfering with the save action. We have seen this before.

What extensions do you have? Can you try disabling all extensions, then enable just the Go extension and see if this still persists?

image

@wader
Copy link

wader commented Jul 4, 2017

Hello, i think i have the same issue.
macOS 10.12.5 (16F73)
Code 1.13.1 (1.13.1)
vscode-go 0.6.62

Get same issue (two file writes) when all extensions disabled except vscode-go. But I noticed that if i disable formatOnSave and just use "Format document" i still get one file write, could that be a lead or is that expected behaviour?

@wader
Copy link

wader commented Jul 4, 2017

Oh just noticed https://github.com/Microsoft/vscode-go/blob/bca4dd5f31f32ac49da79580c07b4000f06287a3/src/goFormat.ts#L69 is the save needed because the formatting tools can only read from file?

@ramya-rao-a
Copy link
Contributor

When you say 2 file writes do you mean the file state indicator next to file name goes from dirty - save - dirty - save?

The line you are pointing at would only cause dirty - save - save

@wader
Copy link

wader commented Jul 4, 2017

Ok the go formatting tools support reading from stdin but vscode-go is currently not using it.

So could the issue be that when onDidSaveTextDocument is fired the file has already been saved and then the formatting tools are executed and then the document is saved again?

@wader
Copy link

wader commented Jul 4, 2017

You mean the dot left of the filename? when i make a change the dot appears. Then when i save it flickers fast and disappears

Im using https://github.com/cortesi/modd to trigger builds and if i run it in debug mode (--debug) i see two writes.

18:42:35: notify.Write: "/path/to/main.go"
18:42:35: Delta:
Added: []
Deleted: []
Changed: [/path/to/main.go]
18:42:35: prep: go install
18:42:36: notify.Write: "/path/to/main.go"
>> done (580ms)
18:42:37: daemon: app
>> sending signal hangup
18:42:37: Delta:
Added: []
Deleted: []
Changed: [/path/to/main.go]
18:42:37: go install
18:42:37: daemon: app
exited: signal: hangup

@wader
Copy link

wader commented Jul 4, 2017

My thinking is that if vscode-go currently uses the go formatting tools by pointing them to the source file path (not using stdin or a temp file) then it would require two saves to make formatOnSave work. First one to save the maybe not properly formatted file to disk and then run the formatting tool on the file and then second one to save the change to make it properly formatted.

@wader
Copy link

wader commented Jul 4, 2017

Sorry maybe i should mention that i don't have any issues with the undo stack. Just the issue with two writes.

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Jul 5, 2017

I did try passing the file contents via stdin to the formatting tool once before, it caused a bug. I can't remember more on that. Am on vacation right now and so don't have access to my mac. Will check this once I get back.

Until then if you can, feel free to send a PR to use stdin

@wader
Copy link

wader commented Jul 5, 2017

Ah ok, and also use onWillSaveTextDocument instead? i might try to fix it if i get some free time.

But no hurry, enjoy your vacation!

wader added a commit to wader/vscode-go that referenced this issue Jul 5, 2017
Fixes double save issues as now we don't have to save
the document to do formatting.

Also fixes save on "Format Document" (now no save is done).

Related to microsoft#1037
@wader
Copy link

wader commented Jul 5, 2017

@ramya-rao-a do you remember what bug you saw when using stdin?

@ramya-rao-a
Copy link
Contributor

@wader I don't remember. Will have to look at the history of the goformat.ts file.

About onWillSaveTextDocument..
The catch here is that if formatting takes more than 750ms (am picking a number here, need to look at the code to get the actual number), then the action is cancelled.

I still haven't come up with a good solution for that. There us another issue tracking it. (will link it once am back from vacation and have something better than my phone to work on)

For now, go ahead and remove the save and pass file contents using stain and send a PR

@wader
Copy link

wader commented Jul 6, 2017

Ok, thanks! I've sent a PR to use stdin #1069

@wader
Copy link

wader commented Jul 6, 2017

@ramya-rao-a This commit i guess 068754e

@ramya-rao-a
Copy link
Contributor

Yes, thats the revert commit.

See #1069 (comment) for my thoughts on the usage of "onWillSaveTextDocument" and "stdin"

Also the reason for the double save is not due to https://github.com/Microsoft/vscode-go/blob/bca4dd5f31f32ac49da79580c07b4000f06287a3/src/goFormat.ts#L69. That line is only reached when you explicity run the "Format Document" command.

The actual reason for the second save is https://github.com/Microsoft/vscode-go/blob/bca4dd5f31f32ac49da79580c07b4000f06287a3/src/goMain.ts#L308

@ramya-rao-a
Copy link
Contributor

ramya-rao-a commented Jul 10, 2017

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

@wader Lets use #540 or #1069 for further discussions on this subject

@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. There won't be any re-saving of files on formatting

@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

3 participants