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

x/tools/gopls: formatting resets cursor after save with CRLF line endings #31937

Closed
deanveloper opened this issue May 9, 2019 · 29 comments
Closed
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@deanveloper
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.12.5 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\[redacted]\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\[redacted]\go
set GOPROXY=
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=C:\Users\[redacted]\AppData\Local\Temp\go-build065388570=/tmp/go-build -gno-record-gcc-switches

What did you do?

Saved a .go file

What did you expect to see?

My cursor should stay in the same place after formatting

What did you see instead?

My cursor is reset to the beginning of the file. I have tried toggling the [go] > editor.codeActionsOnSave and go.lintOnSave settings to false and it still occurs.

VSCode Settings

Go extension: 0.10.2

{
    "git.autofetch": true,
    "files.associations": {
        "*.tmpl": "gotemplate"
    },
    "terminal.integrated.shell.windows": "C:\\Program Files\\Git\\bin\\bash.exe",
    "go.useLanguageServer": true,
    "go.languageServerExperimentalFeatures": {
            "diagnostics": true // for diagnostics as you type
    },
    "[go]": {
        "editor.snippetSuggestions": "none",
        "editor.codeActionsOnSave": {
            "source.organizeImports": true
        },
    },
    "gopls": {
        "usePlaceholders": true, // add parameter placeholders when completing a function
        "enhancedHover": true,   // experimental to improve quality of hover (will be on by default soon)
    },
    "go.lintOnSave": "off"
}
@gopherbot gopherbot added the gopls Issues related to the Go language server, gopls. label May 9, 2019
@stamblerre
Copy link
Contributor

Does this happen often or just a single time? Do you have a case that could reproduce it?

@deanveloper
Copy link
Author

It happens every time, a condition I have found is that it must be in a Go module. I have not tested with gopath yet, but when editing Go files that are not in Go modules, the editor does nothing on save and the cursor isn't affected.

I haven't gotten around to testing it more yet. It appeared that it formatted the code even when I had turned "format on save" off. It happens on my work computer, I haven't tested on my home computer yet but I'll get back when I do.

@stamblerre
Copy link
Contributor

My initial guess is that it has something to do with your configurations with gopls and other formatting tools. gopls intentionally returns diffs rather than a full file replacement to avoid this issue, so I wonder if you are also somehow running gofmt or goimports instead of gopls.

You could try explicitly turning off formatting and import organization through gopls to see if it still happens:

"go.languageServerExperimentalFeatures": {
            "format": false
},
"[go]": {
        "editor.codeActionsOnSave": {
            "source.organizeImports": true
        },
},

@andybons andybons added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 13, 2019
@andybons andybons modified the milestones: Unplanned, Unreleased May 13, 2019
@deanveloper
Copy link
Author

deanveloper commented May 15, 2019

It looks like if either go.languageServerExperimentalFeatures > format or [go] > editor.codeActionsOnSave > source.organizeImports are enabled that my cursor is reset. I'll check this on my home machine tonight, however I haven't had any issues from there from what I've tested on it.

When both are set to false, the file still gets formatted, but the cursor is not reset.

My configuration where it is "working"
{
    "git.autofetch": true,
    "files.associations": {
        "*.tmpl": "gotemplate",
        "*.html.tmpl": "gohtml",
    },
    "terminal.integrated.shell.windows": "C:\\Program Files\\Git\\bin\\bash.exe",
    "go.useLanguageServer": true,
    "go.languageServerExperimentalFeatures": {
        "format": false,
        "diagnostics": true // for diagnostics as you type
    },
    "[go]": {
        "editor.codeActionsOnSave": {
            "source.organizeImports": false
        },
    },
    "gopls": {
        "usePlaceholders": true, // add parameter placeholders when completing a function
        "enhancedHover": true, // experimental to improve quality of hover (will be on by default soon)
    },
    "go.lintOnSave": "off",
    "editor.formatOnSave": true
}

Edit: editor.formatOnSave does not seem to affect anything

@stamblerre
Copy link
Contributor

I should have asked earlier - when was the last time you updated gopls? My best guess is that you are using an old version. Can you update to the latest and confirm if the issue still happens (go get -u golang.org/x/tools/cmd/gopls)?

@deanveloper
Copy link
Author

I ran the command and it looks like nothing has changed

@stamblerre
Copy link
Contributor

Thanks for letting me know. My next guess is that perhaps it has something to do with the fact that you are on Windows? Can you try switching to Unix line endings by adding "files.eol": "\n" to your VSCode settings?

@deanveloper
Copy link
Author

Well that changed something - the cursor now resets to the beginning of the line rather than the beginning of the file.

@stamblerre stamblerre changed the title gopls: cursor reset after save x/tools/cmd/gopls: formatting resets cursor after save on Windows May 15, 2019
@stamblerre
Copy link
Contributor

Hm, that is so strange. I've never heard of anyone experiencing this kind of behavior on Windows. I will try to reproduce this and see if I can come up with a fix.

@deanveloper
Copy link
Author

Yeah, it's strange for sure. Doesn't happen on my home machine which really confuses me.

@deanveloper
Copy link
Author

Scratch that - it's happening on my home machine too now.

Conditions to reproduce for me:

  • Either go.languageServerExperimentalFeatures > format or (inclusive) [go] > editor.codeActionsOnSave > source.organizeImports must be true
  • go.useLanguageServer must be true
    • (i've commented out all of my other settings other than these 3 in settings.json and it is still occurring)

Weird behaviors:

  • Using CRLF linebreaks, the cursor resets to the beginning of the file.
    • Exception: If you save while your cursor is placed after the very last character of the file, it stays at the end of the file.
  • Using LF linebreaks, the cursor resets to the beginning of the line.
  • If both editor.codeActionsOnSave > source.organizeImports and [go] > editor.codeActionsOnSave > source.organizeImports are both false are false, the cursor works as intended and the file still gets formatted, and imports still get organized, even though both should in theory be disabled.

@stamblerre
Copy link
Contributor

So the reason I think that this happens even though the settings are set to false is that VSCode-Go will fall back to its own formatting mechanism, so it's just running goimports by default. That might be worth opening a VSCode-Go issue for. (For the record, I am not able to reproduce that behavior.)

I was actually able to reproduce the jumping to the beginning of the line behavior on my own Windows machine, as well as on my Mac in some cases. I will investigate further, but I'm starting to think this might not be a gopls issue. When we send formatting results, we send them as a diff that VSCode applies, and it has to determine where to place the cursor. We never send column positions -- only line numbers. However, after formatting it might be difficult for VSCode to understand where your cursor was relative to the changed line.

Can you give an example of a case where you would expect the cursor to stay put, but it instead gets moved to the beginning of the line?

@stamblerre stamblerre changed the title x/tools/cmd/gopls: formatting resets cursor after save on Windows x/tools/cmd/gopls: formatting resets cursor after save with CRLF line endings May 16, 2019
@stamblerre
Copy link
Contributor

Ok, I have been able to repro the cursor resetting to the start of the file behavior. It has to do with CRLF line endings - I will investigate this further now that I have a solid repro.

@deanveloper
Copy link
Author

I'm glad you could reproduce and that I'm not just crazy haha. Some more information:

  • With LF, the cursor only resets to the beginning of the line if the line had changed during formatting.
  • With CRLF, the cursor resets on every save, including when you press Ctrl+S after no changes occurred.

@stamblerre
Copy link
Contributor

It seems that this is a VSCode issue. gofmt only ever uses LF line endings, so when you have CRLF line endings, we return a replacement for the entire file. VSCode does not attempt to replace the cursor at its original position when the diff is that large, and it just moves it to the top of the file. We cannot preserve line endings (see #16355), so we don't really have another alternative other than to file an upstream VSCode issue: microsoft/vscode-languageserver-node#493.

For now, the correct workaround is to add "files.eol": "\n" to your VSCode settings.

@deanveloper
Copy link
Author

That makes sense. Were you ever able to reproduce the line reset bug with LF line endings?

@stamblerre
Copy link
Contributor

Yes, I think the line reset is another consequence of this behavior. Since the diffs are line by line, we just delete a line and replace it, so if VSCode doesn't understand how to replace the cursor, it just puts it back to the beginning of the line.

@stamblerre stamblerre added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 5, 2019
@stamblerre stamblerre changed the title x/tools/cmd/gopls: formatting resets cursor after save with CRLF line endings x/tools/gopls: formatting resets cursor after save with CRLF line endings Jul 2, 2019
@g0lang
Copy link

g0lang commented Jul 4, 2019

+1 I have the same problem

@stamblerre
Copy link
Contributor

There is a temporary workaround for this issue (setting "files.eol": "\n" in your VSCode settings). We are investigating this issue and will hopefully be able to resolve it by fixing #32586.

Also, a reminder to please not comment +1 on issues, but rather to use Github's upvoting mechanism: https://groups.google.com/forum/#!topic/golang-dev/OirJxPzfgug.

@tweakmy
Copy link

tweakmy commented Jul 10, 2019

I having issue when save, the cursor goes to start of the file on a vscode in a ubuntu machine
Below is my setting, what is the correct workaround

{
    "go.useLanguageServer": true,
    "go.languageServerExperimentalFeatures": {
            "diagnostics": true // for diagnostics as you type
    },
    "[go]": {
        "editor.snippetSuggestions": "none",
        "editor.codeActionsOnSave": {
            "source.organizeImports": true
        },
    },
    "files.eol": "\n",
}

@deanveloper
Copy link
Author

@tweakmy For now there seems to be no workaround to the line resetting other than disabling the language server

@Xjs
Copy link
Contributor

Xjs commented Jul 10, 2019

FTR:

{
    "go.formatTool": "goimports", // or whatever you like
    "[go]": {
        "editor.formatOnSave": true,
        "editor.codeActionsOnSave": {
            "source.organizeImports": false
        },
    },
    "go.languageServerExperimentalFeatures": {
        "format": false // other fields can be true
    }
}

disables formatting through gopls for the time being, but keeps the other language server features intact. I use this as workaround since "files.eol": "\n" didn't work for me on Windows, either.

@stamblerre
Copy link
Contributor

Setting "files.eol": "\n" doesn't reformat all of your files until you edit and format them. Additionally, at the bottom right corner of your VSCode window, there should be a toggle for "LF" and "CRLF", and you may need to set that too. You may also need to configure your git settings to check out repos with the correct line endings - see microsoft/vscode-go#2501 (comment).

@tensor-programming
Copy link

I am actually seeing the opposite effect. On windows when I save the cursor goes to the top of the file. Its a very odd behavior for sure. Disabling formatting via gopls as @Xjs suggested removes this problem; so it must be related. Just an fyi, when I say it moves the cursor to the top of the file, I mean it moves it right to the left of the p in package.

@deanveloper
Copy link
Author

@tensor-programming that is not the opposite effect, that is exactly what this bug is describing

@alinnert
Copy link

I just want to throw in there's another, not so drastic, workaround: Using EditorConfig

Create a file named .editorconfig in your project root and paste this into that file:

root = true

[*.go]
end_of_line = lf

And install the EditorConfig extension.

What this does is automatically setting the EOL to LF for every .go file in the project.

(root = true marks the current file as the root file, otherwise it look for more in every parent folder until it reaches the root folder of your file system)

I've tested it and it works (once your file uses LF). The cursor still jumps to the beginning of a line if it's inside a line that gets formatted, but it's better than nothing. Otherwise it stays just where it is.

If you don't know what EditorConfig is: Using EditorConfig is actually pretty common in web development (HTML/CSS/JS). Some editors even have built-in support for it (including Visual Studio).

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/191018 mentions this issue: gopls: use go-diff for edit generation

@stamblerre stamblerre modified the milestones: Unreleased, gopls v1.0 Aug 26, 2019
gopherbot pushed a commit to golang/tools that referenced this issue Oct 10, 2019
this supports sub-line diffs and is much faster

Fixes golang/go#33003
Fixes golang/go#32586
Updates golang/go#31937

Change-Id: I02f82c75828e7e3ec804e8beee916893d4c14b3d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/191018
Run-TryBot: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Rebecca Stambler <rstambler@golang.org>
@theryansmith
Copy link

It is not clear what the status of this bug is. The external references seem to all be closed, and only this one remains that I can find reporting this bug (which is still present):

#32586 (this is closed)
#16355 (this is closed)
microsoft/vscode-languageserver-node#493 (this is closed)

@stamblerre
Copy link
Contributor

This bug can be closed as well, I believe. The next version of gopls will use the go-diff library for diff generation, which allows us to in-line edits and thereby fix the issue with VSCode. You can try it out now by building gopls at master and adding the "go-diff": true config. The next gopls version will include this fix.

@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.4.0 Jul 22, 2020
@golang golang locked and limited conversation to collaborators Jul 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

10 participants