completion ignores textEdit in reply

[PerMildner]

It seems the the :textEdit is ignored in the reply to "textDocument/completion".

The transcript below shows the events when completion is sent to my server. Note that the "bo" is supposed to be completed to "bork(" but instead its label "bork/1" is inserted (as can be seen from the "textDocument/didChange" message.

Using a :insertText works, i.e. it will be used even if it is different from the :label.

I use C-M-i (completion-at-point).

client-request (id:79) Fri Jul  5 09:34:35 2019:
(:jsonrpc "2.0" :id 79 :method "textDocument/completion" :params
	  (:textDocument
	   (:uri "file:///Users/perm/runtime-EclipseApplication/LSP%20Scratch/foo.pl")
	   :position
	   (:line 2 :character 10)
	   :context
	   (:triggerKind 1)))

server-reply (id:79) Fri Jul  5 09:34:35 2019:
(:jsonrpc "2.0" :id 79 :result
	  [(:label "bork/1" :sortText "bork(_)" :textEdit
		   (:range
		    (:start
		     (:line 2 :character 8)
		     :end
		     (:line 2 :character 10))
		    :newText "bork("))])

client-notification Fri Jul  5 09:34:35 2019:
(:jsonrpc "2.0" :method "textDocument/didChange" :params
	  (:textDocument
	   (:uri "file:///Users/perm/runtime-EclipseApplication/LSP%20Scratch/foo.pl" :version 85)
	   :contentChanges
	   [(:text "bork(X) :-\n        bork([X,Y,2], N),\n        bork/1\n        Y = N.\n")]))

client-request (id:80) Fri Jul  5 09:34:35 2019:
(:jsonrpc "2.0" :id 80 :method "textDocument/completion" :params
	  (:textDocument
	   (:uri "file:///Users/perm/runtime-EclipseApplication/LSP%20Scratch/foo.pl")
	   :position
	   (:line 2 :character 14)
	   :context
	   (:triggerKind 1)))

server-reply (id:80) Fri Jul  5 09:34:35 2019:
(:jsonrpc "2.0" :id 80 :result
	  [])

The server is currently not publicly available.

This is the ELPA version.

(Related to #245)

I did try to debug this with edebug. From what I could gather, eglot-completion-at-point intentionally uses the label and then tries to patch things up using a :exit-function to "Undo the just the completed bit [sic]" . It seems the patching is never done, because the exit function is never called, because completion--done (minibuffer.el) is never called, because completion--do-completion thinks the completion was not "exect".

Unrelated: as seen in the event log above, a second completion message is sent that was not triggered by the user. This was mentioned as a fixed bug in a comment to #235 but it does not seem fixed in the ELPA version. Should I file a separate ticket for that?

[joaotavora]

The server is currently not publicly available.

ACK to this all and these bugs. I'll have a look when I find the time. But it would be much simpler if you could somehow share the (Perl?) server with me.

[PerMildner]

Sharing the server is currently not possible.

In some other ticket someone mentioned a replay server, that could replay the event log. Is that available? It seems like a good way to debug things in a reproducible way (and avoids the need for installing various lsp servers).

[joaotavora]

In some other ticket someone mentioned a replay server, that could replay the event log.

I did.

Is that available? It seems like a good way to debug things in a reproducible way (and avoids the need for installing various lsp servers).

It is, isn't it? Volunteering? :-)

[PerMildner]

I got the impression such a server already existed. Does it?

Otherwise, I may look into it. No promises.

[joaotavora]

I got the impression such a server already existed. Does it?

Not that I know of, no.

Otherwise, I may look into it. No promises.

You can write it in any language you want, but perl and python seem the best choices.

[PerMildner]

Also, eglot does not follow the specification "When an edit is provided the value of insertText is ignored." but if both a :textEdit and a :insertText is present, the :insertText takes effect.

[nemethf]

I did write a dummy playback server. And it worked more or less. If there's a need, I can try to find it.

[PerMildner]

I found LanguageServerRobot which looks as if it may be exactly what we need. I have not tried to use it yet.

[joaotavora]

@nemeth, please do!

[joaotavora]

found LanguageServerRobot which looks as if it exactly what we need. I have not tried to use it yet.

But the C# is a turn-off for me :(

[nemethf]

@nemeth, please do!

I've uploaded here. The code should be cleaned up, though.

[joaotavora]

I've uploaded here. The code should be cleaned up, though.

Thanks!

[joaotavora]

@nemethf so I've created some commits in a new branch scratch/fix-completion-at-point-311-and-279 that should take care of this problem while also not breaking company and other completion frontends. Please review and test if you can. Read the commit message carefully and tell me if it makes sense to you. This is super-hairy, head-banging, spaghetti-generating stuff, so we must take care to leave good future-proof documentation.

[nemethf]

This time I actually tested the code and it works as expected. The commit message and the inline comments make sense, but I still don't fully understand how completion-at-point works. Reading C-h f completion-at-point RET or (info "(elisp)Completion") seems like going down in a rabbit hole. So I wonder it is OK to ignore the status argument in the exit-function. Thanks.

[joaotavora]

I think the most important node is (elisp)Programmed Completion, but you won't get by without chasing the code through reading minibuffer.el with edebug or something like that. I'll have a second look at the code today, and a special look at the status argument.

What completion frontends did you test with?

[nemethf]

What completion frontends did you test with?

Both. (Are there others besides completion-at-point and company? autocomplete's "repository has been archived by the owner. It is now read-only.")

---

I've just realized the new snippet-completions test ends like this:

      (beginning-of-line)
      (should (looking-at "foobarquux(a, b)")

If I remember correctly, then the previous, buggy version succeeds with this test. Because it used to complete to this, but it put the point at the end of line and offered no yasnippet functionality. I think It would be more bulletproof to have something like this instead:

     (should (looking-back "foobarquux("))
     (should (looking-at "a, b)"))

[joaotavora]

be more bulletproof to have something like this instead

Well, not bulletproof, but indeed better. You can fix it if you want.