company-lsp icon indicating copy to clipboard operation
company-lsp copied to clipboard

Broken java completion for candidates with the same identifier

Open aureumleonis opened this issue 4 years ago • 7 comments

When all candidates share the same class/method name but differ in imports/method arguments, company inserts the common prefix among these, including the method/argument signatures. Examples:

Given prefix @NotNul and candidates ("@NotNull - javax.validation.constraints" "@NotNull - org.jetbrains.annotations"), calling company-complete-common will insert @NotNull - without showing a popup to select from the candidates or inserting any imports. A similar issue happens with method completton with overloaded methods.

Another example, given prefix assertThro and candidates (" assertThrows(Class<T> expectedType, Executable executable) : T" "assertThrows(Class<T> expectedType, Executable executable, String message) : T"), company will insert assertThrows(Class<T> expectedType, Executable executable verbatim without going through snippet expansion.

I narrowed the issue down to company-update-candidates use of try-completion to compute the common part between candidates. It seems that once company gathers these, it inserts the common part, then attempts another completion. Since the text inserted is not completable (e.g. " - ") company cancels the completion.

I hacked my own company.el to avoid inserting text beyond the symbol name which seems to work

(defun company--try-completion ()
  (let* ((common (try-completion "" company-candidates)))
    (substring common 0 (string-match-p " \\|(" common))))

aureumleonis avatar Jan 03 '20 20:01 aureumleonis

This can be fixed if:

  1. company-lsp shows real text in the label - this could be a configurable option.
  2. company-mode asks the backend to calculate what is the correct common prefix.

CC @dgutov

yyoncho avatar Jan 04 '20 06:01 yyoncho

@yyoncho If 1 is doable, why make it an option? Sounds like a good default behavior: use the "real text" as the completion string, and put the rest into the annotation.

Regarding 2, I suppose company-update-candidates could call the match backend command to calculate company-common. If your backend supports it anyway. But that would be an unexpected use of that command.

dgutov avatar Jan 06 '20 11:01 dgutov

the nice thing about having that text in the candidate is that you can filter by java packages for classes with the same name. With option 1 this feature would simply stop working altogether right? How difficult is to create another backend command in company for this purpose in a backwards compatible way?

aureumleonis avatar Jan 06 '20 19:01 aureumleonis

No immensely difficult, and your usage is interesting but unexpected too.

Further, being able to do 1. should also fix the company-tng problems...

dgutov avatar Jan 07 '20 01:01 dgutov

If 1 is doable, why make it an option? Sounds like a good default behavior: use the "real text" as the completion string, and put the rest into the annotation.

It is doable but it is not TRT - as per spec we should show the label property. Consider that the insert text might be a snippet.

yyoncho avatar Jan 07 '20 07:01 yyoncho

I'm saying that you would show the rest of the text as annotation.

dgutov avatar Jan 07 '20 14:01 dgutov

So the whole string would be visible. And whatever text the snippet needs would be inserted in post-completion.

dgutov avatar Jan 07 '20 14:01 dgutov