doomemacs / doomemacs

An Emacs framework for the stubborn martian hacker
MIT License
19.59k stars 3.06k forks source link

With `lsp` and `snippets` enabled, the completion candidates in `python-mode` are not sorted by relevance #5262

Closed x6rulin closed 3 years ago

x6rulin commented 3 years ago

What did you expect to happen?

While editing Python code, the completions with more relevance should have higher priority. That means, when I type ab, candidates which start with ab should be the first priority, then ones containing the string ab and then the characters a and b, like below: Screenshot from 2021-07-15 11-44-17

What actually happened?

The completions are sorted likely with some confusing order, not relevance first.

Screenshot from 2021-07-15 11-38-54

Describe your attempts to resolve the issue

Figuring out that occurs after the commit ba716d6, I add below codes to my config.el:

(after! python
  :init
  (setq! +lsp-company-backends
         (if (featurep! :editor snippets)
             '(:separate company-yasnippet company-capf)
           'company-capf)))

It works, but just covers up the real problem. A thorough solution is still needed.

Steps to reproduce

  1. upgrade doom emacs to the most recent commit
  2. choose company as the code completion backend, enable lsp and snippets, set the +lsp flag in python-mode, then run doom sync
  3. open one .py file, edit some codes, observe the completion candidates

System Information

https://pastebin.com/jrkaCBb0

PatrickWulfe commented 3 years ago

What's happening is 'abs' is a snippet and your setting is moving snippets in front of the lsp results. I do this too with: (setq +lsp-company-backends '(:separate company-yasnippet company-capf))

The previous behavior was that snippets were being pushed after the lsp results, but just in the past week or so was changed so they are now mixed in with the lsp results. I guess moving them to the front would cause issues somehow. https://github.com/hlissner/doom-emacs/issues/5215

x6rulin commented 3 years ago

What's happening is 'abs' is a snippet and your setting is moving snippets in front of the lsp results. I do this too with: (setq +lsp-company-backends '(:separate company-yasnippet company-capf))

The previous behavior was that snippets were being pushed after the lsp results, but just in the past week or so was changed so they are now mixed in with the lsp results. I guess moving them to the front would cause issues somehow.

5215

It's okay to mix the lsp result with snippets. However, the completions should be present in a smart way that helps efficient coding, instead of some confusing and helpless ones. Actually experience the completion methods before and after the commit ba716d6, you will feel this difference:slightly_smiling_face:

RBckmnn commented 3 years ago

I experience the same issue with typescript and csharp language server. I asked about this issue on the lsp-discord and I was told this was caused by https://github.com/emacs-lsp/lsp-mode/issues/2970

juanscr commented 3 years ago

I'm having the same issue after the upgrading doom. It looks like the commit mentioned in the description caused that issue.

jpcima commented 3 years ago

I have such an issue with clangd and lsp-mode. An update has destroyed the quality of completion candidates to a point that they're useless, because of bad sorting.

Fortunately, I can bisect it, and I am able to confirm this commit as the culprit. Reverting fixes the issue. ba716d69f0f680125e2411ac050086d54d7c8a4b

hlissner commented 3 years ago

Sorry for the long wait. I reverted the problem commit in 77f78f0, which should fix this issue. Thanks for bringing it to my attention!

(A friendly reminder: github has :+1: reactions. Please use these to express "I have the same issue" if you don't have more to add to the original post. The maintainers of any github repo will appreciate it, not just Doom.)