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

:label being used for completion insert instead of :newText #245

Closed
paulbdavis opened this issue Apr 3, 2019 · 13 comments
Closed

:label being used for completion insert instead of :newText #245

paulbdavis opened this issue Apr 3, 2019 · 13 comments

Comments

@paulbdavis
Copy link

Testing this go file using gopls as the lsp server (the official server from the go project)

package main

import "fmt"

const foo = "bar"

var bar = "foo"

func main() {
	// completing on "f"
	foo = "bar"
	// completing on "b"
	bar

	// completing on "fmt.Pr"
	fmt.Print(a ...interface{})
}

The data coming back for the matches in the events buffer have the correct data in the :newText field but instead the :label is applied

This is tested on the MELPA stable version of eglot without using the built in completion (no company)

This is the data coming back from the completion on the fmt.Pr in the example above

(:jsonrpc "2.0" :result
          (:isIncomplete :json-false :items
                         [(:label "Print(a ...interface{})" :kind 3 :detail "n int, err error" :sortText "00009" :filterText "int" :insertTextFormat 1 :textEdit
                                  (:range
                                   (:start
                                    (:line 15 :character 7)
                                    :end
                                    (:line 15 :character 7))
                                   :newText "int")
                                  :command
                                  (:title "" :command "editor.action.triggerParameterHints"))
                          (:label "Printf(format string, a ...interface{})" :kind 3 :detail "n int, err error" :sortText "00010" :filterText "intf" :insertTextFormat 1 :textEdit
                                  (:range
                                   (:start
                                    (:line 15 :character 7)
                                    :end
                                    (:line 15 :character 7))
                                   :newText "intf")
                                  :command
                                  (:title "" :command "editor.action.triggerParameterHints"))
                          (:label "Println(a ...interface{})" :kind 3 :detail "n int, err error" :sortText "00011" :filterText "intln" :insertTextFormat 1 :textEdit
                                  (:range
                                   (:start
                                    (:line 15 :character 7)
                                    :end
                                    (:line 15 :character 7))
                                   :newText "intln")
                                  :command
                                  (:title "" :command "editor.action.triggerParameterHints"))])
          :id 129)

When instead using bingo for the golang LSP, the data it returns for that same completion is a bit different (note the full text replacement in :newText)

(:id 6 :result
     (:isIncomplete :json-false :items
                    [(:label "Print(a ...interface{})" :kind 3 :detail "n int, err error" :sortText "00009" :insertText "Print" :insertTextFormat 1 :textEdit
                             (:range
                              (:start
                               (:line 15 :character 5)
                               :end
                               (:line 15 :character 7))
                              :newText "Print"))
                     (:label "Printf(format string, a ...interface{})" :kind 3 :detail "n int, err error" :sortText "00010" :insertText "Printf" :insertTextFormat 1 :textEdit
                             (:range
                              (:start
                               (:line 15 :character 5)
                               :end
                               (:line 15 :character 7))
                              :newText "Printf"))
                     (:label "Println(a ...interface{})" :kind 3 :detail "n int, err error" :sortText "00011" :insertText "Println" :insertTextFormat 1 :textEdit
                             (:range
                              (:start
                               (:line 15 :character 5)
                               :end
                               (:line 15 :character 7))
                              :newText "Println"))])
     :jsonrpc "2.0")

Is this an issue in eglot or an issue of gopls not following some spec correctly?

@nemethf
Copy link
Collaborator

nemethf commented Apr 4, 2019

The first log shows items with :filterText. Eglot currently doesn't follow the specification closely when it handles items with filterText. Hopefully, PR #235 is a step towards the right direction. (But beware, I haven't really studied this issue.)

@joaotavora
Copy link
Owner

Thanks very much @nemethf for handling all these issues minimally and providing sensible answers. I am so busy I just read them diagonally.

@nemethf
Copy link
Collaborator

nemethf commented Apr 5, 2019

No problem. I wonder, though, if we (you) could make the events buffer more useful, i.e., I'd like to replay the events, so that I could debug issues without bothering to install an exotic server.

Regarding this issue, I followed the instructions to install gopls, but I probably ran into the "no file information for" bug, similar to this one. But I did not use windows. So I stopped investigating further and recommend trying out the PR cited above.

@joaotavora
Copy link
Owner

No problem. I wonder, though, if we (you) could make the events buffer more useful, i.e., I'd like to replay the events, so that I could debug issues without bothering to install an exotic server.

This is actually a very good idea: an Emacs lisp program (but could also be some other language) that takes a file of events in eglot format and mimics a server. The server will kill itself if it receives input not in the form it expects. Some details would have to be ironed out regarding path names for example (they would have to be made relative to the events file, I think).

Unfortunately, I don't have time to make this right now.

You are right that installing exotic servers isn't a scalable solution.

@s-kostyaev
Copy link
Contributor

@nemethf

Regarding this issue, I followed the instructions to install gopls, but I probably ran into the "no file information for" bug

golang/go#31178 can be related

@paulbdavis
Copy link
Author

No problem. I wonder, though, if we (you) could make the events buffer more useful, i.e., I'd like to replay the events, so that I could debug issues without bothering to install an exotic server.

Regarding this issue, I followed the instructions to install gopls, but I probably ran into the "no file information for" bug, similar to this one. But I did not use windows. So I stopped investigating further and recommend trying out the PR cited above.

Those instructions will pull the latest tagged release they have, which is not in a good working state.

I installed it by cloning the repo and building

git clone https://github.com/golang/tools go-tools
cd go-tools/cmd/gopls
go build -o /somewhere/in/my/path/gopls2

Then I set up eglot to use the newly build binary instead

@paulbdavis
Copy link
Author

The first log shows items with :filterText. Eglot currently doesn't follow the specification closely when it handles items with filterText. Hopefully, PR #235 is a step towards the right direction. (But beware, I haven't really studied this issue.)

Isn't that just for filtering though? That parameter should not effect the insertion of the selected match right?

The second example using bingo has both :newText and :insertText with the full completion

The first only has :newText with the missing text from what is already in the buffer

@nemethf
Copy link
Collaborator

nemethf commented Apr 9, 2019

The filterText properties are completion candidates in emacs terms; a user should choose from them during completion. They do effect the whole completion process. The second example has full completions, therefore its ranges are different from the first example. It's not a problem that the first example has "partial" newText.

I decided I won't install any lsp servers unless it can be run in a docker container.

So it's kind of a stalemate :), as you won't try #235 out, and I can't replicate your issue without a server.

Unfortunately, @joaotavora's suggestion might work only if the event log is specially crafted since user events (cursor movements, etc.) are not stored there. However, I put together a very simple replay server. It can replay an event file if you follow these instructions when creating the event logs. Open the go file with eglot disabled. Go to the end of "fmt.Pr". Enable eglot with M-x eglot. Wait until the initial lsp-communication finishes (couple of seconds). Press C-M-i. Switch to the completion buffer (probably C-x o will do). Select the "Print" or the "int" item (with RET). Save the log file (probably with M-x eglot-events-buffer RET, C-x o, C-x C-w). This sounds complicated, but may pave the road to a more reliable debugging process that does not need an lsp server.

@paulbdavis
Copy link
Author

@nemethf It looks like #235 does fix the issue I am having with my LSP server

One other issue with that is that in the completion options listed it only shows the remaining text instead of the :label (So in the example above, hitting M-C-i on fmt.Pr has int, intln, etc. listed in the completion options instead of the full label)

@nemethf
Copy link
Collaborator

nemethf commented Apr 17, 2019

For a CompletionItem, the server may send label, detail, documentation, filterText, textEdit. It's not clear to me what's the best approach to mapping these fields to the M-C-i interface. Company-completion has richer interface, so there's a higher chance of creating a more intuitive interface, but it's more complicated, so I understand it even less. Hopefully, when the PR is accepted, this will be ironed out.

@nemethf
Copy link
Collaborator

nemethf commented Jan 14, 2020

So #235 wasn't merged into the master branch, but João rewrote this part of Eglot. @paulbdavis, do you still see the original problem?

@nemethf
Copy link
Collaborator

nemethf commented May 11, 2020

I assume this is fixed and close the issue. If it's not, comment here and I'll reopen.

@nemethf nemethf closed this as completed May 11, 2020
@joaotavora
Copy link
Owner

Thanks for this housekeeping @nemethf . It is very useful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants