On 23/08/2024 13:23, João Távora wrote: >> Great! Bump it to 1.17.60? > > No, keep it at 1.17.30. That's the 1.17-ish version of Eglot that will > ship with Emacs 30, just as 1.12.29 is the 1.12-ish version of Eglot > that shipped with Emacs 29. > > The change you're proposing will eventually also be merged to master > and be part of the 1.18 GNU Elpa release (earlier I wrote 1.17 GNU Elpa > by mistake). Perfect, thanks. > Foreign as it might seem to Emacs devs and users, LSP doesn't want any > concept of "prefix" (or suffix or commonality). In general, two very > closely related completions in the final insertion result can have two > wildly different labels displayed to the user. In this sense, the > "completions" popup in LSP is less like our all-too-familiar listing of > strings that can help complete the thing at point and more akin to a > specialized context menu for "things to _do_ at point". There's not > even an LSP mandate that this thing to _do_ must include something to > insert at point. Currently it could be just tidying up imports > elsewhere in the LSP document. In future LSP versions, it could be just > running an arbitrary server action, or sending an email. I see what you're saying, but insofar that current completion is mostly working out well, adding the special logic for parens would improve a lot of cases, and keep others the same degree of broken (the email-sending ones, for sure). So it might be worth a try. Anyway, stopping any partial completion (at first I didn't understand that you meant a more general notion that the partial-completion c-style) should be similarly easy to what the current patch does. You would just drop the second clause in eglot--dumb-tryc, I think. Or maybe both the first and the second. > The other "fix" is to lobby with the LSP spec people, but they're very > VSCode-inclined. From what I've seen, even other editors which are more > popular than Emacs have very little sway there. It seems we only have our hacks to help. They got pretty advanced, though. >> Yeah, it'd be great to have C-M-i stop before the paren. > > No, it wouldn't. It'd fix this particular case, but it could break in > some other future version of LSP where the LSP label isn't a prefix of > the thing that selecting that label would insert. Quite possibly, yes. Though reverting to the simpler behavior at that point would just require a 3-4 lines diff, as discussed. >>>> An alternative might be to use a completion-all-completions call, to >>>> assert the full list of completions. Or some completions that are in >>>> it anyway. >>> Yes, I generally prefer "end-to-end" tests using interactive >>> interfaces. >>> So unless you're duplicating the test I think this part should be >>> skipped. But your call. >> >> It's a different test (albeit with the same result as an existing >> one). I don't mind rewriting it in a different style, if you >> prefer. The result is kind of messy either way. > > My suggestion is just test that *Completions* pops up. Alas, this check does not succeed: (should (get-buffer-window "*Completions")) This works: (when (buffer-live-p "*Completions*") (kill-buffer "*Completions*")) (completion-at-point) (should (looking-back "foo")) (should (looking-at "123")) (should (get-buffer "*Completions*")) Is that better? >>>> It's the same case as '("foo123" . 3) in the test above, isn't it? >>> I don't think so. In a rust-analyzer hello world (which you can >>> make >>> with cargo init, if I'm not mistaken), both >>> v.c1234.12345676890 >>> v.count_on1234.12345676890 >>> should expand to >>> v.count_ones().1234567890 >>> In the first case, you'll have to manually select "count_ones" from >>> the >>> completions buffer. The expansion and removal of the 1234 happens via >>> LSP text edits, which are enacte by the :exit-function. >> >> Okay, I am seeing a difference too: C-M-i does eat the suffix in the >> Rust example (the "1234"). But completion with Company does not :-/ > > I can't even get Company to appear in those situations. You might have an older version installed? > But I wouldn't bother about it. The bug report was about C-M-i > originally. The important thing is that the LSP textEdit is run (via > exit-function) with the correct LSP document state that the server > expects. As far as I can tell, this is happening right now, so I'd be > nice to have a test to shore it up. Okay, see the attached patch with the added test. It took way longer than expected. Seems pretty brittle (note how eglot--wait-for-rust-analyzer is defined), but should be better than nothing. I also checked how VS Code behaves in such completions, since we were talking about what LSP's authors designed for or not. Works differently: 1) Completions are not filtered by suffix, ever. 2) Completing count_ones() does not delete the suffix ("1234" remains). It might mean that either behavior is okay, though.