Add format-on-save support (instead of requiring use of explicit Format Document command)

[Gys]

The gopath seems to be correctly set because typing errors ('Undefined: ...') en vet messages ('exported function should have comment') are reported correctly.
However, the formatting is not corrected ? Not on disk and not in the editor. I tried to override the default goreturns with goimports and then with gofmt, but that does not seems to matter.
Any ideas ?

[captncraig]

I am experiencing this as well.

In User preferences: "go.formatTool": "gofmt",

[Gys]

Strange: Alt-Shft-F (or right-click menu) works correctly !
So it seems to be a bug.

[Gys]

Actually I just now realize that formatting on save is not in the feature list....

[jchannon]

Format on save would be a nice feature

[lukehoban]

Currently formatting is available through an explicit command (Format Code or alt-shift-f).

I'll keep this issue to track adding support for format on save.

[jchannon]

If I wanted to take a bash at this feature where can I modify the existing plugin on my mac?

[lukehoban]

@jchannon Great. There's some notes on how to build/debug the extension yourself here: https://github.com/Microsoft/vscode-go#building-and-debugging-the-extension.

[jchannon]

I assume this instruction is wrong:rm -rf ~/.vscode/extensions/go-code and should be rm -rf ~/.vscode/extensions/vscode-go ??

[lukehoban]

@jchannon Yep - that's left over from the old naming - fixed with eca07a3.

[dmitshur]

Huge +1 to adding format-on-save (and it should be on by default; use gofmt if neither goimports nor goreturns are installed). IMO it's the most basic, fundamental feature I expect out of a code editor with Go support. It's really hard to switch to something that doesn't have it.

By default, it simply shouldn't be possible to save a non-gofmted .go file (unless you explicitly want to do that, e.g., if you're debugging gofmt functionality or something, then you should explicitly disable format-on-save).

[jchannon]

Agreed. If Luke can do it quickly go ahead. I'm just poking around ...

On Wednesday, 18 November 2015, Dmitri Shuralyov [email protected]
wrote:

Huge +1 to adding format-on-save (and it should be on by default; use
gofmt if neither goimports nor goreturns are installed). IMO it's the
most basic, fundamental feature I expect out of a code editor with Go
support. It's really hard to switch to something that doesn't have it.

—
Reply to this email directly or view it on GitHub
#14 (comment).

[solher]

+1 to add format on save. I was using Atom and this is definitely what I miss the most :)

[jchannon]

OK I think I have this working, however I'm a bit stuck on writing it back to the file. I have added some code to goCheck.ts. Firstly is that the correct place? Secondly how do I write back to the file in goCheck?

[jchannon]

Actually I think it should go in goMain::startBuildOnSaveWatcher ?

[lukehoban]

@jchannon. I think what is probably best is the following:

1. Move the formatting implementation in goFormat.ts into a standalone Command, so that it can be invoked both from Format Document and from the new format-on-save implementation
2. Invoke that command inside goCheck.ts prior to invoking the compile/vet/lint checks.
3. Add an option to turn this behaviour on/off

The bug in #16 is also something that will need to be fixed before we can turn format-on-save on.

We'll also need to make sure that format-on-save interacts decently with Auto Save if the user has that turned on. I'm a little worried that these will interact poorly, since code will be changing unexpecedtly without the user taking any action. It may be that we'll need to turn of format-on-save if Auto Save is turned on.

[jchannon]

Yeah I don't think formatting with auto save turned on will work

On Thursday, 19 November 2015, Luke Hoban [email protected] wrote:

@jchannon https://github.com/jchannon. I think what is probably best is
the following:

1. Move the formatting implementation in goFormat.ts into a standalone
   Command, so that it can be invoked both from Format Document and from the
   new format-on-save implementation
2. Invoke that command inside goCheck.ts prior to invoking the
   compile/vet/lint checks.
3. Add an option to turn this behaviour on/off

The bug in #16 #16 is also
something that will need to be fixed before we can turn format-on-save on.

We'll also need to make sure that format-on-save interacts decently with
Auto Save if the user has that turned on. I'm a little worried that these
will interact poorly, since code will be changing unexpecedtly without the
user taking any action. It may be that we'll need to turn of format-on-save
if Auto Save is turned on.

—
Reply to this email directly or view it on GitHub
#14 (comment).

[jchannon]

WIP in PR added here - https://github.com/Microsoft/vscode-go/pull/40/files

On 19 November 2015 at 18:15, Jonathan Channon [email protected]
wrote:

Yeah I don't think formatting with auto save turned on will work

On Thursday, 19 November 2015, Luke Hoban [email protected]
wrote:

@jchannon https://github.com/jchannon. I think what is probably best
is the following:

1. Move the formatting implementation in goFormat.ts into a
   standalone Command, so that it can be invoked both from Format Document and
   from the new format-on-save implementation
2. Invoke that command inside goCheck.ts prior to invoking the
   compile/vet/lint checks.
3. Add an option to turn this behaviour on/off

The bug in #16 #16 is
also something that will need to be fixed before we can turn format-on-save
on.

We'll also need to make sure that format-on-save interacts decently with
Auto Save if the user has that turned on. I'm a little worried that these
will interact poorly, since code will be changing unexpecedtly without the
user taking any action. It may be that we'll need to turn of format-on-save
if Auto Save is turned on.

—
Reply to this email directly or view it on GitHub
#14 (comment)
.

[jchannon]

@lukehoban have pushed all I can do, but for some reason its not working when I save

[lukehoban]

I haven't reviewed carefully yet - but it looks like you aren't yet applying the edits.

I believe you need to use registerTextEditorCommand to register a new TextEditorCommand with VS Code to do the formatting itself. That is the step 1 in my list above. Then use executeCommand to invoke that action both during the explicit Format and during the save. This is step 2 and 3 in my earlier list.

[jchannon]

OK I've tried that and the command is executed but the document is not
updated with the formatted code. I'm chucking the towel in, I'll let you
implement it instead.

Cheers

On 19 November 2015 at 20:25, Luke Hoban [email protected] wrote:

I haven't reviewed carefully yet - but it looks like you aren't yet
applying the edits.

I believe you need to use registerTextEditorCommand
https://code.visualstudio.com/docs/extensionAPI/vscode-api#853 to
register a new TextEditorCommand with VS Code to do the formatting itself.
That is the step 1 in my list above. Then use executeCommand
https://code.visualstudio.com/docs/extensionAPI/vscode-api#862 to
invoke that action both during the explicit Format and during the save.
This is step 2 and 3 in my earlier list.

—
Reply to this email directly or view it on GitHub
#14 (comment).

[calmh]

I'd like to propose that the format-on-save command can be different than the explicit "format code" command. I like goimports for the explicit format command. But it can be quite slow on large files with many import and a large $GOPATH, so having it on every save can be painful and unnecessary. In that setup, having the editor do just gofmt on save is much faster and usually all I need.

[lukehoban]

@jchannon I've pushed a new PR which builds upon your changes.

This is still work in progress, but it does technically do a format-on-save. The lack of a preSave event in Code makes this much more awkward than it should be.

[mbxt]

Sounds like there's some really good progress on this. Is it possible to have allow for an array of tools to run on save? For instance, I enjoy how Atom's extension does both gofmt and goimports. For now, I have hacked my build task to handle this for me, but that's ugly at best.

[dmitshur]

For instance, I enjoy how Atom's extension does both gofmt and goimports.

Question about that specific example: do you know that goimports is a superset of gofmt, it already does what gofmt does? If so, what's the reason to do that?

[mbxt]

Oh wow, you're right, I totally missed that. Thanks for pointing that out :)

[jonnenauha]

Damn, was thinking something was broken until found "format code" from the menus. So used to fmt on save from LiteIDE. How often is the plugin updated, is it repo tags or how can one see when things go "live"?

[lukehoban]

For folks interested in this, I've added a new PR with a more-or-less complete implementation of this in #115. Along with some other formatting fixes from @newhook, I believe this is now working reasonably well.

If anyone has a chance to pick up and test it out (git cloning per the instructions in the README and checking out the PR branch), that would be great. The feature is behind a flag so I'll likely check this into master and push an update for broader testing in the next day or so.

[calmh]

Initial impression is that it does what it's supposed to. However a thing to note is that it doesn't play well with Auto Save, as the sudden reformat when you pause typing is a bit jarring and can move the cursor to somewhere other than where it was before saving. Edit: Actually, the cursor movement doesn't seem to be an issue. Let me try it a bit more. :)

[lukehoban]

I've merged the format-on-save support into master and it's in the 0.6.16 update of the extension.

It's still behind a flag for now - you'll want to set "go.formatOnSave": true in your settings to try it out.

As @calmh mentions, format-on-save doesn't play well with AutoSave, so you'll want to pick which one you want to use. Generally, this means two modes of use for the plugin:

1. FormatOnSave mode: User should generally save often, using this as a manual trigger to update formatting, save to disk and update errors and linting. Errors will be up to date less of the time, but formatting will be up to date more of the time.
2. AutoSave mode: Users don't need to manually save, but should run Format Document when they want to update formatting and adjust imports. Errors will be up to date less of the time, but formatting will be up to date less of the time.

[solher]

Works like a charm for me. Thanks a lot !

[dmitshur]

Thank you so much @lukehoban!

I consider format-on-save to be an absolute must minimum requirement to be able to consider using an editor for writing Go, and this was blocking me from trying VSC out further. This is very exciting! Now that it's resolved, I can try playing more with it.

I've briefly tried it, and first impressions are really nice, and I see this on the trajectory of becoming the best (non-terminal) Go editor/plugin I've seen (beating Sublime Text + GoSublime, and Atom + its best Go package).

I notice some buggy behavior when there are multiple "issues" to resolve on save, like a golint issue, go build issue, and gofmt issue. It seems it resolves one, but not the others at the same time, so I need to make a few dummy edits and re-save to get it all updated. I also couldn't get go vet warnings to show up. But go lint, go build, code autocompletion all worked! I'll try to create some reproducible steps for the issues I'm seeing and report them.

But overall, this is looking really incredible!

[lukehoban]

Thanks @shurcooL!

I notice some buggy behavior when there are multiple "issues" to resolve on save, like a golint issue, go build issue, and gofmt issue. It seems it resolves one, but not the others at the same time, so I need to make a few dummy edits and re-save to get it all updated.

Curious if you are able to reproduce this issue? Would love to fix that if there is still a problem.

[Hokutosei]

can confirm this is working formatOnsave! thank you @lukehoban !