Skip to content
This repository has been archived by the owner on Jul 31, 2023. It is now read-only.

Tons of unrelated completion candidates shown when a candidate is fulfilled #79

Open
ianyepan opened this issue Jan 3, 2020 · 7 comments

Comments

@ianyepan
Copy link

ianyepan commented Jan 3, 2020

This issue only arises with mspyls in emacs. I've made sure by running pyls in emacs, as well as mspyls in VScode and none of the issues happened.

To reproduce the problem in a python buffer with mspyls + lsp-mode enabled, supposed we're trying to have autocompletion for print(). Start off by typing p followed by r and i, n etc. and we'll see the candidates narrow down and become fewer.

Specifically, at prin we'll see that only 5 candidates are present, namely, print, ImportWarning, StopIteration..... Of course, this is due to the fuzzy match result.

The strange thing now happens as soon as I hit t to complete print. Now with either mspyls in VSCode or pyls in emacs, only a single candidate print will remain in the tooltip. However, with mspyls in emacs, the tooltip is suddenly filled with hundreds (maybe more) of unrelated candidates such as if, else, bin, bool, break, AttributeError, compile, continue, copyright etc. As if it was a completion for a wildcard character.

P.s. I'm on macOS with Emacs 27.0.50

@yyoncho
Copy link
Member

yyoncho commented Jan 3, 2020

I think that the issue will go away if you enable the caching for mspyls in company-lsp - but either the issue will be fixed once lsp-mode's capf implementation matures.

@ianyepan
Copy link
Author

ianyepan commented Jan 3, 2020

I have set company-lsp-cache-candidates to t, nil, and 'auto and compared the results. Here's my observation:

  1. The behaviour I described in the title only happens if company-lsp-cache-candidates is set to 'auto.
  2. If company-lsp-cache-candidates is set to t, I will miss several fuzzy-matched candidates, but no unrelated candidates show up either.
  3. If set to nil, the completion works ideal as one would expect -- no unrelated candidates, and fuzzy-matched ones are present too. However, I feel that emacs slows down / lags a bit with this option.

Hope this description helps. At the moment it's overall still usable and I think I'll stick with the auto option

@seagle0128
Copy link
Collaborator

According to the description above, it seems the issue of company-lsp?

@ianyepan
Copy link
Author

ianyepan commented Jan 5, 2020

@seagle0128 That might be the case, I'll open a similar issue over at company lsp and mention this thread too.
Note that however, this only happens with Microsoft's pyls -- it never occurs in other programming languages I use, e.g. c/c++ with clangd, js and typescript with typescript-language-server, java with eclipse jdtls etc.

@yyoncho
Copy link
Member

yyoncho commented Jan 6, 2020

(add-to-list 'company-lsp-filter-candidates '(mspyls . t))
(setq company-lsp-cache-candidates 'auto)

(defun company-lsp--on-completion (response prefix)
  "Handle completion RESPONSE.

PREFIX is a string of the prefix when the completion is requested.

Return a list of strings as the completion candidates."
  (let* ((incomplete (and (hash-table-p response) (gethash "isIncomplete" response)))
         (items (cond ((hash-table-p response) (gethash "items" response))
                      ((sequencep response) response)))
         (candidates (mapcar (lambda (item)
                               (company-lsp--make-candidate item prefix))
                             (lsp--sort-completions items)))
         (server-id (lsp--client-server-id (lsp--workspace-client lsp--cur-workspace)))
         (should-filter (or (eq company-lsp-cache-candidates 'auto)
                            (and (null company-lsp-cache-candidates)
                                 (company-lsp--get-config company-lsp-filter-candidates server-id)))))
    (when (null company-lsp--completion-cache)
      (add-hook 'company-completion-cancelled-hook #'company-lsp--cleanup-cache nil t)
      (add-hook 'company-completion-finished-hook #'company-lsp--cleanup-cache nil t))
    (when (eq company-lsp-cache-candidates 'auto)
      ;; Only cache candidates on auto mode. If it's t company caches the
      ;; candidates for us.
      (company-lsp--cache-put prefix (company-lsp--cache-item-new candidates incomplete)))
    (if should-filter
        (company-lsp--filter-candidates candidates prefix)
      candidates)))

This would fix your issue. ATM company-lsp maintainer is missing and we cannot get the fixes in (e. g. tigersoldier/company-lsp#128) so we are working on alternative completion implementation in lsp-mode which hopefully will have everything working correctly).

@ianyepan
Copy link
Author

ianyepan commented Jan 6, 2020

we are working on alternative completion implementation in lsp-mode

Thank you @yyoncho, it does fix my issue for python completion. Should I keep this open until the implementation you mentioned is done? That way people will know they can safely remove this temporary workaround-snippet from their config.

@seagle0128
Copy link
Collaborator

I can also confirm the patch works well for me.

seagle0128 added a commit to seagle0128/.emacs.d that referenced this issue Jan 15, 2020
seagle0128 added a commit to seagle0128/.emacs.d that referenced this issue Jan 15, 2020
hlissner added a commit to doomemacs/doomemacs that referenced this issue Apr 22, 2020
daveduthie pushed a commit to daveduthie/doom-emacs that referenced this issue Apr 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants