all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Theodor Thornhill via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: 61532@debbugs.gnu.org
Cc: joaotavora@gmail.com
Subject: bug#61532: 30.0.50; [PATCH]: Make completions without sortText fall to back of the list
Date: Wed, 15 Feb 2023 15:52:00 +0100	[thread overview]
Message-ID: <87ttzn6kxb.fsf@thornhill.no> (raw)

[-- 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


             reply	other threads:[~2023-02-15 14:52 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-15 14:52 Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2023-02-18 22:18 ` 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
2023-02-19 11:13   ` João Távora
2023-02-19 16:08     ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-19 18:19       ` João Távora
2023-02-19 18:43         ` Basil L. Contovounesios via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-19 18:48           ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-19 22:47             ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-19 23:39               ` João Távora
2023-02-19 23:53                 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-19 18:52         ` Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-19 22:56         ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-02-21  8:24           ` Augusto Stoffel
2023-02-21  8:24         ` Augusto Stoffel
2023-02-21 12:47           ` João Távora
2023-02-21 14:08             ` Augusto Stoffel
2023-02-21 14:28               ` João Távora

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87ttzn6kxb.fsf@thornhill.no \
    --to=bug-gnu-emacs@gnu.org \
    --cc=61532@debbugs.gnu.org \
    --cc=joaotavora@gmail.com \
    --cc=theo@thornhill.no \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.