From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Newsgroups: gmane.emacs.bugs Subject: bug#72705: 31.0.50; eglot--dumb-tryc Filters out too much Date: Fri, 23 Aug 2024 11:23:50 +0100 Message-ID: <87h6bbwn9l.fsf@gmail.com> References: <54dacc71-4395-431f-abc4-c60dc070cb03@gutov.dev> <0ff5f767-be87-4d64-964c-0a20fa776acf@gutov.dev> <0632f40f-98fd-4388-aba0-629a32d415eb@gutov.dev> <87ed6hyg1o.fsf@gmail.com> <06cfce49-33ff-41a2-b999-469c4a0009c0@gutov.dev> <87o75kwl2i.fsf@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="19849"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 72705@debbugs.gnu.org To: Dmitry Gutov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Aug 23 12:24:41 2024 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1shRTE-00051e-Tb for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 23 Aug 2024 12:24:41 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1shRSz-0003u7-Ml; Fri, 23 Aug 2024 06:24:25 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1shRSv-0003t9-SC for bug-gnu-emacs@gnu.org; Fri, 23 Aug 2024 06:24:22 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1shRSp-000391-Nx for bug-gnu-emacs@gnu.org; Fri, 23 Aug 2024 06:24:21 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:Date:References:In-Reply-To:From:To:Subject; bh=IjAENOtGO21tnrzHFw6nxI0GpBXtfr1fwdxfbwnUJEg=; b=VTv9mj7bkbQuq3JtPPM1et3ES+1VxNKWhpLKZ+xdKpLPYnxKlR+lCp83sQ4U53BH+k6iwefGWYhNCU1B01BVngshsHlLMPa8eVVZvnEG2XsnBQnwrhy+YXMVGoJKwB6EgzS3vrI2XUV1XeEK4KMUMWFQnw5KAq9OULb8ljX7xgWmz/aIkRIV8lb04PLqD0d2+TEU6NSipAVe1HjSWx65/rB+NqURkUDp98SmJgT8LUS+QY5poI4v/VrKf6O2RPFxur/11SIxR+UPdfWHxMEUMBcPTglmsCIYh3am04c+Xx275Z31mbVE0e2Bmm6PtCIyCkUrOu5VpdijBOPYpktzUA==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1shRTZ-0003GA-MW for bug-gnu-emacs@gnu.org; Fri, 23 Aug 2024 06:25:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 23 Aug 2024 10:25:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 72705 X-GNU-PR-Package: emacs Original-Received: via spool by 72705-submit@debbugs.gnu.org id=B72705.172440866412472 (code B ref 72705); Fri, 23 Aug 2024 10:25:01 +0000 Original-Received: (at 72705) by debbugs.gnu.org; 23 Aug 2024 10:24:24 +0000 Original-Received: from localhost ([127.0.0.1]:39048 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1shRSx-0003F5-2W for submit@debbugs.gnu.org; Fri, 23 Aug 2024 06:24:23 -0400 Original-Received: from mail-wr1-f46.google.com ([209.85.221.46]:42247) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1shRSt-0003Em-Dn for 72705@debbugs.gnu.org; Fri, 23 Aug 2024 06:24:21 -0400 Original-Received: by mail-wr1-f46.google.com with SMTP id ffacd0b85a97d-37182eee02dso960059f8f.1 for <72705@debbugs.gnu.org>; Fri, 23 Aug 2024 03:23:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1724408546; x=1725013346; darn=debbugs.gnu.org; h=content-transfer-encoding:mime-version:user-agent:message-id:date :references:in-reply-to:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=IjAENOtGO21tnrzHFw6nxI0GpBXtfr1fwdxfbwnUJEg=; b=QrWwAQ+Tcg3tDHrUbqkdTbPSL2NSL8LEPi9bciK7z7Rn9J/IR/N2+rEt0/xtaVsGC/ bJDKNvB+ZOonTMilr5l+rZBpJ6CbiA52Nc3dw3Z4hhDb0dopkDpPIPHT+ofdIXq8kUx5 ohi+BZ+hMOsrBadcJWTSObuz82JSF4QWNwkZg6vYXdkoKN7GhtPodEz1bRglsmEoDJWP fpHoigRPstYUDdXFfoIUYvIucV6LqTwRTSVhx9+qEymzDZhyzWldXwCYsx3VgvMgGCB9 uHZJXYxfR5j72cTFGzk5yzH0LSbtfHY3ifBKZhrPUoMyxN0lUuMMi4JVaN0VIIx0PxF7 607g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724408546; x=1725013346; h=content-transfer-encoding:mime-version:user-agent:message-id:date :references:in-reply-to:subject:cc:to:from:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=IjAENOtGO21tnrzHFw6nxI0GpBXtfr1fwdxfbwnUJEg=; b=RacoxswExNeGSgyDRinh2OOQiXWSm4pSRGdGz3yS/W2LogGo5bYANhM9tv/OPnkFG+ 2cYSYmleqLlaodA9TrBkhaWT1HX1iu0bvTmEllEhRV60HLbAiMgMJGH93FMUHXRZeY0g xAkiC6xVenEgnWbe0Q/VD13tNHhYVJrUrLBe19wS9qPfuzf9YqaF25hUjz+pAqsbje+L PIV5FawfP0ZoBWMP/gcxOrxSkksYdBp1Ubhc99xxCmPIsXkOfEW7fA7s5jIPcvNENFiy PzNOBHhYll8q46Qidn6WZ+wVOi6/6Zsg7C2RrUonzCse1SRppL7qIYtv4KjxwukLqhAB z5QQ== X-Gm-Message-State: AOJu0YxFe2y8lSB9sxpOtoWtu2ujbDc3qdMTQ1zmDummG9i1nKjpX3FC gTV66LKNJy1suN2FzzS2c7nSw8onxn76YyhOJRdtYg4cYxOTKlTC1PdCkw== X-Google-Smtp-Source: AGHT+IEnEcuAI+EIyahT1PPziB9M7AO5Umu2Cau3vc2hMiZRiEur2yoWPOlRl1OPeXBcTlGiImIthw== X-Received: by 2002:a5d:5547:0:b0:363:ac4d:c44f with SMTP id ffacd0b85a97d-37310ebfa3bmr1073000f8f.17.1724408545805; Fri, 23 Aug 2024 03:22:25 -0700 (PDT) Original-Received: from krug (87-196-102-145.net.novis.pt. [87.196.102.145]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-3730817a076sm3802182f8f.60.2024.08.23.03.22.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 23 Aug 2024 03:22:25 -0700 (PDT) In-Reply-To: (Dmitry Gutov's message of "Fri, 23 Aug 2024 02:16:14 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:290589 Archived-At: Dmitry Gutov writes: > On 22/08/2024 19:59, Jo=C3=A3o T=C3=A1vora wrote: >> Dmitry Gutov writes: >>=20 >>> Like with the previous discussions about Eglot in Emacs 29, I think >>> there will be a lot of users who just use the version of it that comes >>> with the release, without upgrading. >> Hopefully upgrading should be a matter of M-x eglot-upgrade-eglot. >> But no problem on my side. In a separate "don't merge" commit, look >> into editing the etc/EGLOT-NEWS there which I've already bumped to >> 1.17.30 (the version of Eglot to be bundled with Emacs 30). I'll then >> copy that text to master eventually and release 1.17 to GNU Elpa. > > 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). > The current proposal doesn't fix it, though it probably makes things a > bit better (simply because the "common" string is on average computed > to be shorter). A somewhat complete solution would probably include > per-language "stop character" map - where we could check the `probe' > for chars such as "(" (or whatever the language uses) and shorten the > return value accordingly if found. > > It would replace the currently added "string-prefix-p" check, I guess. Sounds clever but I really doubt that would work in the general case. I'm not getting my point across I think. An LSP label is just a label. Except for in very primitive uses of LSP, it's not meant to be inserted or to be used in the computation of prefixes or commonality between completions. These primitives uses are deprecated as I hope the LSP quote from the spec confirmed. 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. In conclusion, Emacs's partial completion makes no sense in LSP (except for the aforementioned fringe/deprecated cases). The correct fix for this separate issue to disable partial completion: either a full completion can be inserted or a listing of all applicable completions should be shown. 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. >>> completing to 'foo_bar_' keeps the popup active, so I just press RET >>> to pick the first completion. >> With the above 'foobar(int x, ' example and with bare C-M-i it's >> even >> worse: it's hard (if not impossible) to complete to the full completion >> and thus get the desired snippet expansion because the common prefix of >> both completion labels ends with a space. > > 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. >>> 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. >>> 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. 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. > I guess the difference might come from C-M-i always going through > try-completion, whereas the Company popup relies on all-completions. > > There's a similar difference when using gopls. > > But Clangd actually looks special (just filed separate bug#72765). > >> This scenario >> wasn't working a while ago and I'd like to protect it against >> regression. > > Would we have a test that launches rust-analyzer (and depends on it > being installed, and a Cargo project initialized)? Yes. Look in the existing eglot-tests.el: there is already such a test: eglot-test-rust-analyzer-watches-files. I'd just cargo-cult the lines until the "cargo init" call. Jo=C3=A3o