bug#61532: 30.0.50; [PATCH]: Make completions without sortText fall to back of the list

[Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

[-- Attachment #1: Type: text/plain, Size: 1966 bytes --]


Hi Joao and others!

As I mentioned in a different Eglot bug I believed there was a problem
with sorting of candidates.  It seems there kinda is, but it's a little
convoluted.  To test this you need to have a server that supplies
snippets, or at least that sends some items with and without :sortText.
However, the code is simple enough, so it should be enough to read it.

So, what happens with completion in Eglot that I find confusing is that
sometimes, candidates in the completion frameworks (company, corfu etc)
display candidates in a seemingly random order compared to what the
server sends.  In particular, the Jdtls server sends snippets without
sorttext, which will be converted to the empty string in the sorter
function.  That in turn will make all the snippets fall to the front of
the list as (string-lessp "" "anything else") evaluates to t.  That
means we ignore the sorts supplied by the server.  Because (string-lessp
nil "anything else") evaluates to nil it will put them at the end of the
list, which IMO makes more sense.

In addition, the completion-category-defaults set by eglot uses flex, as
you know, so the display-sort-function will sort them very hungrily,
making them seem out of order.

For example, if the candidates are (delete, deleteAll, deleteById) when
buffer content is (| is point):

```
something.del|
```

completion will display them in this order: (deleteAll, deleteById,
delete), which yet again "doesn't respect sort order".  The latter issue
is fixed by setting

(setq completion-category-overrides
      '((eglot (styles basic partial-completion initials))))

or something else, which works, but is very convoluted and really not
documented anywhere, or did I miss it?

The former issue is fixed in the supplied patch.  Do you agree with what
I propose?  We could also default to something like :label or
:insertText, but that would in some cases also likely conflict with what
the server is expected to send.

Theo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-completions-without-sortText-fall-to-back-of-th.patch --]
[-- Type: text/x-patch, Size: 1454 bytes --]

From 6b5c37f00f73abf07a72d2bc2fe6edb99f8ee0a4 Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Wed, 15 Feb 2023 15:16:15 +0100
Subject: [PATCH] Make completions without :sortText fall to back of the list

* lisp/progmodes/eglot.el: String-lessp is safe with (string-lessp nil
"foo") and (string-lessp "foo" nil), but defaulting to the empty
string puts candidates to the top of the list.  Nil will make the
candidates fall to the end of the list.
---
 lisp/progmodes/eglot.el | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 5e761d3064f..c8620f4199f 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -2782,10 +2782,9 @@ eglot-completion-at-point
               (cl-sort completions
                        #'string-lessp
                        :key (lambda (c)
-                              (or (plist-get
-                                   (get-text-property 0 'eglot--lsp-item c)
-                                   :sortText)
-                                  "")))))
+                              (plist-get
+                               (get-text-property 0 'eglot--lsp-item c)
+                               :sortText)))))
            (metadata `(metadata (category . eglot)
                                 (display-sort-function . ,sort-completions)))
            resp items (cached-proxies :none)
-- 
2.34.1