Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

eglot-completion-at-point deleting extra text beyond the completion boundaries #1339

Closed
MatthewTromp opened this issue Dec 24, 2023 · 4 comments

Comments

@MatthewTromp
Copy link

  1. Install rust-analyzer
  2. Create a new rust project. Replace main.rs with the following:
fn main() {
    let v: usize = 1;
    1234.1234567890
}

(1234.1234567890 is a placeholder for some hypothetical method call on some hypothetical object)

  1. Enter either rust-ts-mode or rust-mode.
  2. M-x eglot (with rust-analyser)
  3. Place point on the 1234 line and insert v.c:
v.c1234.1234567890
  1. With point still on c, press M-<tab> to get eglot completions. Select count-ones. The result will be:
v.count_ones4567890

The completion has replaced text beyond the symbol it was operating on. The expected behavior is

v.count_ones.1234567890

That is, the symbol currently being completed (c1234) should be replaced with the suggested completion.

Note that if you insert v., then get completions and select one, everything behaves as expected. That is, starting with

v.1234.1234567890

with point after v., press M-<tab>, then select count_ones, the result is

v.count_ones.1234567890

as expected.

I think the cause is code in :exit-function in eglot-completion-at-point which tries to delete the text added by the accepted completion, which fails when the completion actually itself net removed text rather than adding it. Also related is #160.

Emacs version: GNU Emacs 30.0.50 (build 1, x86_64-pc-linux-gnu, X toolkit, cairo version 1.18.0, Xaw scroll bars) of 2023-12-24, commit ba3d3c699e12e2b236a353aa4dbfd1937d47f080
JSONRPC dump: eglot_jsonrpc_dump.txt

@joaotavora
Copy link
Owner

joaotavora commented Dec 24, 2023 via email

@joaotavora
Copy link
Owner

I've reproduced the problem and fix one of many cases. This is hairy stuff, and for now we'll just have to accept that partial completions or completions inside symbols in Eglot are very quirky. I'll leave the commit message here:

commit a6ef458e3831001b0acad57cf8fa75b77a4aff3f (HEAD -> master)
Author: João Távora <joaotavora@gmail.com>
Date:   Tue Dec 26 00:31:29 2023 +0000

    Eglot: partial fix for middle-of-symbol completions

    * lisp/progmodes/eglot.el (eglot-completion-at-point): Fix
    completion reversion in :exit-function.

    In a rust-ts-mode buffer such as this main.rs file

      fn main() {
        let v: usize = 1;
        v.c<cursor-here>1234.1234567890
      }

    the server wants to edit the line to read, after C-M-i and selecting
    "count_ones"

        v.count_ones<cursor-here>.1234567890

    But it couldn't apply the edit to the correct initial state because
    that state wasn't correctly restored.  This commit fixes that.

    However, if the initial state is

        v.count_on1234.1234567890

    then completion still fails, because the 'try-completion' call in
    eglot-completion-at-point will just return complete to "count_ones"
    and Emacs doesn't consider this a completion "exit", so it'll
    completely ignore the exit function.

    I think 'try-completion' (and 'test-completion') simply can't be used
    here (for one, they obey styles, and styles are off-limits in LSP),
    but I'll leave that for another commit.

    Github-reference: https://github.com/joaotavora/eglot/issues/1339

@joaotavora joaotavora added the bug label Dec 26, 2023
dickmao pushed a commit to commercial-emacs/commercial-emacs that referenced this issue Dec 26, 2023
* lisp/progmodes/eglot.el (eglot-completion-at-point): Fix
completion reversion in :exit-function.

In a rust-ts-mode buffer such as this main.rs file

  fn main() {
    let v: usize = 1;
    v.c<cursor-here>1234.1234567890
  }

the server wants to edit the line to read, after C-M-i and selecting
"count_ones"

    v.count_ones<cursor-here>.1234567890

But it couldn't apply the edit to the correct initial state because
that state wasn't correctly restored.  This commit fixes that.

However, if the initial state is

    v.count_on1234.1234567890

then completion still fails, because the 'try-completion' call in
eglot-completion-at-point will just return complete to "count_ones"
and Emacs doesn't consider this a completion "exit", so it'll
completely ignore the exit function.

I think 'try-completion' (and 'test-completion') simply can't be used
here (for one, they obey styles, and styles are off-limits in LSP),
but I'll leave that for another commit.

Github-reference: joaotavora/eglot#1339
@joaotavora
Copy link
Owner

The commit that I think closes this issue (as closed as it will ever get) is now pushed to Emacs.git master.

Note that the previous "insert only v." example now seems to fail, but it's not a failure. rust-analyzer doesn't want to gobble the "1234" in that case, and that can be seen clearly from the edit it provides. The fact that it did gobble it up previously was an artifact of the previous buggy implementation.

commit 4dcbf61c1518dc53061707aeff8887517e050003 (HEAD -> master)
Author: João Távora <joaotavora@gmail.com>
Date:   Tue Dec 26 07:47:29 2023 -0600

    Eglot: Make 'try-completion' less broken

    The 'try-completion' completion operation, used mostly in vanilla
    'completion-at-point' invoked with C-M-i is close to impossible to get
    right in LSP because of the arbitrary edits handled in
    ':exit-function'.

    When this operation is invoked on the table, returning the pattern
    argument unchanged somehow (TM) makes a sole completion show the
    *Completions* buffer, where selecting it will recover context
    necessary for `:exit-function' and call that function.  It doesn't
    break any other cases I know, and that's good enough for now.

    https://github.com/joaotavora/eglot/issues/1339

    * lisp/progmodes/eglot.el (eglot-completion-at-point): Return pattern
    when 'try-completion' is invoked.

dickmao pushed a commit to commercial-emacs/commercial-emacs that referenced this issue Dec 26, 2023
The 'try-completion' completion operation, used mostly in vanilla
'completion-at-point' invoked with C-M-i is close to impossible to get
right in LSP because of the arbitrary edits handled in
':exit-function'.

When this operation is invoked on the table, returning the pattern
argument unchanged somehow (TM) makes a sole completion show the
*Completions* buffer, where selecting it will recover context
necessary for `:exit-function' and call that function.  It doesn't
break any other cases I know, and that's good enough for now.

joaotavora/eglot#1339

* lisp/progmodes/eglot.el (eglot-completion-at-point): Return pattern
when 'try-completion' is invoked.
@joaotavora
Copy link
Owner

Nope, sorry. Doesn't work. Can never work as far as I can tell. I won't try anymore, but anyone else can:

commit d376462c7183752bf44b9bd20bf5020fe7eaf75a (HEAD -> master, origin/master, origin/HEAD)
Author: João Távora <joaotavora@gmail.com>
Date:   Tue Dec 26 08:10:04 2023 -0600

    Revert "Eglot: Make 'try-completion' less broken"

    This reverts commit 4dcbf61c1518dc53061707aeff8887517e050003.

    It's not correct, breaks tests.  I declare it impossible to make C-M-i
    use of 'try-completion' behave sanely with LSP in its current state.
    YMMV.  Use a completion tooltip, like Company.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants