Complete common ends completion if the prefix contains a parenthesis

[Azkae]

Describe the issue

Completing common prefix causes completion to stop if the common prefix extends after a parenthesis '('.
This can happen in c++ if a function is overloaded, see example below:

After pressing tab:

Steps to reproduce

(use-package company
  :config
  (global-company-mode t))

(use-package eglot
  :hook
  (c++-mode . eglot-ensure)
  :config
  ;; Use --completion-style=detailed so that clangd returns overloaded candidates
  (add-to-list 'eglot-server-programs '(c++-mode . ("clangd" "--completion-style=detailed"))))

With the code below:

#include <string>
#include <vector>

class A
{
public:
    void test(const std::vector<int> &a);
    void test(const std::string &a);
};

int main(int ac, char **av)
{
    A a;
    a.tes
    // pressing <tab> will insert `a.test(const std::` and the completion will end.
}

Expected behavior

I would expect the completion not to end. company-complete-common could stop inserting the common prefix at the first parenthesis.

[dgutov]

Good question, and I've seen this problem mentioned in the past.

First of all, we should not that it's not the default configuration. I guess most language servers migrated to the other approach, with just printing ellipsis by default. But I'd be glad if we can support it.

The essence of the problem is that neither eglot not lsp-mode massage the lists of completions into separating the method names from the arguments. They don't put the arguments into "annotation". So the framework were doesn't know where the "common part" ends.

But maybe it's okay: the annotations are used to print types, and we don't have any additional field in the completions popup. Then we need some other way to find where the "common part" stops.

This problem isn't specific to Company either: pressing C-M-i instead would also complete to the character somewhere in the middle of the arguments list. I think that's where both eglot and lsp-mode could alter the behavior, if they can find the place where the args begin, based on some internal information. It might be possible to alter the try-completion behavior of the completion table, which will in turn affect the highlighting on the completions, which we could in turn refer to while computing the common part in company-update-candidates. I'm not sure how difficult it will be to implement that new behavior for try-completions, though.

Cc @joaotavora @yyoncho @kiennq

[joaotavora]

I've reproduced this. When configured thusly, clangd supplies these as "snippet" completions or "edit " completions (depending on whether or not you have Yasnippet installed). Here is the latter case:

[server-reply] (id:49) Sat Oct 14 17:27:34 2023:
(:id 49 :jsonrpc "2.0" :result
     (:isIncomplete :json-false :items
		    [(:detail "void" :filterText "test" :insertText
			      "test" :insertTextFormat 1 :kind 2
			      :label " test(const std::string &a)"
			      :score 0.9184742569923401 :sortText
			      "4094dedftest" :textEdit
			      (:newText "test" :range
					(:end (:character 6 :line 13)
					      :start
					      (:character 6 :line 13))))
		     (:detail "void" :filterText "test" :insertText
			      "test" :insertTextFormat 1 :kind 2
			      :label
			      " test(const std::vector<int> &a)"
			      :score 0.9184742569923401 :sortText
			      "4094dedftest" :textEdit
			      (:newText "test" :range
					(:end (:character 6 :line 13)
					      :start
					      (:character 6 :line 13))))]))

IOW these are completions which only make sense when inserted fully so that the :exit-function of the CAPF member can execute the associated edit. There is no point in inserting the string " test(const std::vector<int> &a)" as this isn't even valid C++.

Yet we must show something to the user if the completions are to be of any use.

Another way to look at this is to note that Emacs's traditional completion UI doesn't quite fit into LSP's concept of a completion UI, which is probably modeled after simpler clients like VS Code. It's not the first time that the marriage between LSP abstraction and Emacs abstractions isn't perfect.

Is there a solution? Well, not that I can see. Or rather, the solution is to somehow mark these completions so that the partial completion logic doesn't consider them. But this will often kill every completion and partial completion will be useless anyway. At least in Clangd's case, even when --completion-style=detailed is not passed. The fact that it works sometimes for --completion-style=bundled is only incidental, because the label happens to make sense in C++ when inserted up to a common part.

But absence of partial completion is still better than nonsensical results of partial completion, I guess.

Not sure this can be implemented in the CAPF, but maybe it can be done. I think @monnier is the specialist to ping here.

[joaotavora]

The essence of the problem is that neither eglot not lsp-mode massage the lists of completions into separating the method names from the arguments. They don't put the arguments into "annotation". So the framework were doesn't know where the "common part" ends.

Just commenting on this part. Eglot has no notion of what are "arguments" because LSP doesn't talk about that. Searching for ( will work in some languages, but not others, and these brittle hacks have been problems in the past.

So even with the "annotation" model there's no good fit. LSP uses a "label" model. The LSP :label field isn't an addendum to the :insertText field. So unless LSP completion protocol is overhauled with Emacs-appeasing complexity (and assuming any language server would even care about that), I think the easiest solution is just to find a way to disable partial completion for a given capf, in this case eglot-completion-at-point.

[dgutov]

Is there a solution? Well, not that I can see. Or rather, the solution is to somehow mark these completions so that the partial completion logic doesn't consider them.

Could the LSP client take a completion string and all associated information it already keeps about it, and determine that the opening paren and the text after it are not part of the "completion"? Then the try-completion method could do that and stop earlier.

[joaotavora]

Could the LSP client take a completion string and all associated information it already keeps about it, and determine that the opening paren and the text after it are not part of the "completion"? Then the try-completion method could do that and stop earlier.

Not really. Hacks like these are done for font-locking Eldoc in some situations and they are hairy and brittle. Either you lobby LSP devs and servers devs sufficiently hard to support what they probably see as ancient technology, or I don't think there's a better solution than just giving up.

[joaotavora]

Not really.

I mean, I won't object to someone trying that and showing a patch, then we can evaluate in which servers it makes sense. If it doesn't break the majority use case of users that press RET to "select" a completion, and if the patch isn't extremely ugly, I'll merge it.

[dgutov]

Searching for ( will work in some languages, but not others, and these brittle hacks have been problems in the past.

Indeed, I was thinking that falling back to searching for ( might help. Or other arg delimiters. Perhaps combined with the search for the "beginning of the snippet field" annotation immediately following it.

That is, if a better solution couldn't be worked out.

I think the easiest solution is just to find a way to disable partial completion for a given capf, in this case eglot-completion-at-point

IIUC, "complete up to the first difference" is a pretty integral part of the C-M-i (or M-x completion-at-point) workflow. Perhaps we could disable this specifically in Company (relying on fuzzy matching most of the time, and iterating through completions with tab), but that would keep the problem around in the default UI. And it does feel unsatisfactory, personally speaking.

But as mentioned previously, as long as the default configuration is working okay, we don't have to choose a solution right away.

[joaotavora]

But as mentioned previously, as long as the default configuration is working okay, we don't have to choose a solution right away.

Oh, I think it's not impossible to trigger nonsensical stuff with the default clangd configuration too. It's just a coincidence that it doesn't happen in this very particular case. For example typing std:: C-M-i will usually insert a nonsensical bullet into the code.

but that would keep the problem around in the default UI.

The default UI can also be used to select "full completions", at least sometimes. And definitely not as practical as it is with company. But that should be definitely be improved in that default UI and other built-in UIs that allow this capability should make their way into Emacs. icomplete-in-buffer for example has the beginnings of one (but currently sucks).

[monnier]

Another way to look at this is to note that Emacs's traditional completion UI doesn't quite fit into LSP's concept of a completion UI, which is probably modeled after simpler clients like VS Code. It's not the first time that the marriage between LSP abstraction and Emacs abstractions isn't perfect.

AFAICT, the LSP protocol is designed under a similar model to the Web, i.e. the server is in control and the client is just a dumb executor. This goes against the grain of Emacs, of course. In the case of completions, I think that the sane/robust approach to deal with LSP is to use the model of "selection" rather than "completion" :-(

But absence of partial completion is still better than nonsensical results of partial completion, I guess. Not sure this can be implemented in the CAPF, but maybe it can be done. I think @monnier is the specialist to ping here.

BTW, rather than look for a paren, maybe another heuristic could be to use the `:insertedText` content for the completion and use the `:label` for annotations (which would be guessed by searching for the `:insertedText` inside the `:label`). :-(

[joaotavora]

for annotations (which would be guessed by searching for the :insertedText inside the :label).

There's no guarantee that :insertText is a prefix or even matches text in :label.

In the case of completions, I think that the sane/robust approach to deal with LSP is to use the model of "selection" rather than "completion" :-(

Exactly, well put. LSP is modelled after "selection" clients.

So. Is there a way to say this in a CAPF, that it is not good for the partial completion rituals?

[dgutov]

@Azkae

Looks like the intermediate conclusion is that fixing this while keeping the "complete common" behavior is not trivial.

What's your opinion on the proposed workaround? Which is to drop the latter, at least when used together with eglot/lsp-mode. That's easy enough to do with rebinding keys anyway. But do you see this problem often? What would you choose for your config in the meantime?

• Keep the current behavior, seeing the bug once in a while (is that often?),
• Rebind tab to "select next" and backtab to "select previous",
• or revert to the default Eglot's behavior, removing the --completion-style=detailed customization?

[monnier]

There's no guarantee that `:insertText` is a prefix or even matches text in `:label`.

I know. We'd be firmly in the domain of heuristics and ugly hacks.

LSP is modelled after "selection" clients. So. Is there a way to say this in a CAPF, that it is not good for the partial completion rituals?

The CAPF "protocol" does not distinguish the "what is presented" from "what is inserted", so the best you can come up with is probably something like: - return the `:label` as "the completion for the user to choose". - use the `:exit-function` to rewrite the erroneously inserted `:label` string, replacing it with (presumably) the `:insertedText` string.

[Azkae]

Thank you so much for the detailed explanation. There doesn't seem to be an easy solution that work for all languages.

I encounter this problem quite often with this clang setting, so for my use case I will simply skip calling company-complete-common if the prefix contains a parenthesis:

(defun conf--company-complete-common ()
  (interactive)
  (when (and company-common (not (string-match-p (regexp-quote "(") company-common)))
      (call-interactively 'company-complete-common)))

(use-package company
  :bind
  (:map company-active-map
   ("<tab>" . conf--company-complete-common)))