GNU bug report logs - #72705
31.0.50; eglot--dumb-tryc Filters out too much

Previous Next

Package: emacs;

Reported by: Dmitry Gutov <dmitry <at> gutov.dev>

Date: Mon, 19 Aug 2024 01:53:02 UTC

Severity: normal

Found in version 31.0.50

Done: Dmitry Gutov <dmitry <at> gutov.dev>

Bug is archived. No further changes may be made.

Full log


View this message in rfc822 format

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: João Távora <joaotavora <at> gmail.com>
Cc: 72705 <at> debbugs.gnu.org
Subject: bug#72705: 31.0.50; eglot--dumb-tryc Filters out too much
Date: Tue, 20 Aug 2024 05:08:19 +0300
[Message part 1 (text/plain, inline)]
On 19/08/2024 15:59, João Távora wrote:
> On Mon, Aug 19, 2024 at 12:39 PM Dmitry Gutov <dmitry <at> gutov.dev> wrote:
> 
>>> So I'd say this need more testing, especially with more servers.
>> Here's hoping.
> 
> If you want this change, you'll have to do the testing.

So far I've tested with gopls and clangd (when writing the new tests),
typescript-language-server and rust-analyzer.

>> Thanks for the references, I'll dig in more.
>>
>> Surprised to hear that exit-function can be affected.
> 
> exit-function needs a fully formed completion.  try-completion's
> and Emacs partial complete semantics aren't compatible.  ISTR
> that even a full completion obtained via try-completion would
> sometimes not run the function.  Maybe I'm misremembering.

All right.

The proposed change doesn't alter the kinds of strings that are 
inserted, only the cases when that happens. When the added predicate 
returns nil, we fall back to the next clause; and the check at the end 
also allows us to return nil, which is useful in certain rare contexts.

> This whole system is held  up by very thin wires unfortunately,
> and a lot of people are relying on those wires.  The best choice
> to use LSP's completions, which were obviously modelled after
> other IDEs where completions are much simpler and similar to
> the Company tooltip...   is  naturally to use something like
> company (or corfu with some patch applied).
> 
> I hope you don't change any Company behaviour backward-incompatibly
> (though I have my fork anyway, so no problem).

The change is in a pending PR, it's designed to be opt-in on the backend 
level, but company-capf is a separate backend, so... you know.

It alters what happens when you explicitly press TAB, which previously 
only did prefix-matching inside Company code, but now can delegate to 
backends for some extra smarts (longer expansions being the goal), which 
unfortunately don't work too well with LSP.

The report that's referenced in the 3 commits your mentioned does seem 
to have regressed is https://github.com/joaotavora/eglot/issues/1339 - 
not to the original behavior (exit-function still works), but C-M-i 
changes buffer text to

  v.call1234.1234567890

Not sure how important that case is, but it's a consequence of having 
the style return nil in try-completion and unavoidable failover to 
'basic' because of that (in completion--styles, because a category can't 
actually override, only prepend styles).

I suppose that could be fixed by moving some matching logic from the 
style into the completion table. Not sure how important/realistic the 
example is, although somebody did report it...

In the meantime, here's an updated patch with 3 new tests (and existing 
ones all passing). The implementation has been retouched too for better 
performance (the typescript server in particular made things slower with 
the previous proposal).
[eglot--dumb-tryc-more-checks.diff (text/x-patch, attachment)]

This bug report was last modified 267 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.