From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs Subject: bug#72705: 31.0.50; eglot--dumb-tryc Filters out too much Date: Fri, 23 Aug 2024 02:16:14 +0300 Message-ID: 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; format=flowed Content-Transfer-Encoding: 8bit Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="8812"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Mozilla Thunderbird Cc: 72705@debbugs.gnu.org To: =?UTF-8?Q?Jo=C3=A3o_?= =?UTF-8?Q?T=C3=A1vora?= Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Aug 23 01:17:31 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 1shH3a-00028T-Ng for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 23 Aug 2024 01:17:31 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1shH3O-0007bG-BB; Thu, 22 Aug 2024 19:17:18 -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 1shH3M-0007b3-AH for bug-gnu-emacs@gnu.org; Thu, 22 Aug 2024 19:17:16 -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 1shH3M-0002ad-1O for bug-gnu-emacs@gnu.org; Thu, 22 Aug 2024 19:17:16 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=In-Reply-To:From:References:MIME-Version:Date:To:Subject; bh=IgDgFQRrBhtPA2ghvDbE8kyf72sCdwBGjjEhD5TuWZQ=; b=kD42j+mLTCiuBBK3M8Ek8eqbeUk1P2bvaD1Snc06rSdeakIsN9t17BRdIqU6qiFhGkzmIPt7B84L4w36zgoXyE2tDsryNoXqh/1aHOfxCZaCxksExJsQpVJiiQoj2CnEHnUtDfCWuvfNvP53wfVhtk4fYMjlpLLyho/3u88Gv5lfuAStVPcMjV43zFK8+mjeC49WGbf9CHWzWGS4g6VpfoAxiCEw2+fkmJzFgI+ibsNLKkz//6RDERi0mUgpNXo2PRU4qwtEmBHgIGXnf412kiE8kj+K1jvm9xVnjwJK1cXBn1D7Re46dhAVTMvq771FvXy+jKoMRvd+tx2FqSGaMQ==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1shH46-00022K-C2 for bug-gnu-emacs@gnu.org; Thu, 22 Aug 2024 19:18:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 22 Aug 2024 23:18:02 +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.17243686327766 (code B ref 72705); Thu, 22 Aug 2024 23:18:02 +0000 Original-Received: (at 72705) by debbugs.gnu.org; 22 Aug 2024 23:17:12 +0000 Original-Received: from localhost ([127.0.0.1]:38545 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1shH3I-00021B-4y for submit@debbugs.gnu.org; Thu, 22 Aug 2024 19:17:12 -0400 Original-Received: from fout1-smtp.messagingengine.com ([103.168.172.144]:44529) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1shH3F-00020w-Rp for 72705@debbugs.gnu.org; Thu, 22 Aug 2024 19:17:10 -0400 Original-Received: from phl-compute-01.internal (phl-compute-01.nyi.internal [10.202.2.41]) by mailfout.nyi.internal (Postfix) with ESMTP id 40AC61390182; Thu, 22 Aug 2024 19:16:18 -0400 (EDT) Original-Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-01.internal (MEProxy); Thu, 22 Aug 2024 19:16:18 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gutov.dev; h=cc :cc:content-transfer-encoding:content-type:content-type:date :date:from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to; s=fm2; t=1724368578; x=1724454978; bh=IgDgFQRrBhtPA2ghvDbE8kyf72sCdwBGjjEhD5TuWZQ=; b= i+tAiUHHmj2KPHu93RfZqiiEKurpcHT8GuDHY9ii6/10mnBuwQqhnrhQNs37QgWQ /Y/rSvZTCWPZ8/YboS2y5JDSHuYzDMbHF2kqg6qiYdFKz1syO2L0OeZm9ynqo1Tm Z8/RaII4/YUngpxa9oL8EqiqogSF8v9QRCZIF3h0oK18Zi9M4SsCCz22R4T4yHi5 2xBeVUQnzGKzv4izbgbl4di9USCr5HxQOWyxR7G2hum533Wx/FG9oyQCjyXxTTG+ SHY9V7JmR+IEXFgRgRn+LDylMdpUWx1nIlrkFyqwx0RM+KLt9gzFfVxNq3elfpDR TnmoeRIs+L9bLtq84Qvbrg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-transfer-encoding :content-type:content-type:date:date:feedback-id:feedback-id :from:from:in-reply-to:in-reply-to:message-id:mime-version :references:reply-to:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1724368578; x= 1724454978; bh=IgDgFQRrBhtPA2ghvDbE8kyf72sCdwBGjjEhD5TuWZQ=; b=S RmwqZVHXYF4s7soA5pQF9wxYuIkAXiqqVgvcclaYl+dKycARQN+RzPL/88KQNgj/ he8D60OeuoX80yJ538dny+I1/SrayEhUkgHEI3nas/UZ15zWeFcjAfLZc2f8gfhK zD110xdAJ3x9rLnCp7WxYDdsMve3b8Rxk2VtSyJzZAVWk+QSY7gNilQtEAFsQntQ jrkFF2W/xVAtyGujutDDATsUgnsJdMkO7xwd3Bg9MZXm9AVT+CuA1r5Aoibzej8L t8WIu8429Zigz4mAcwUmyfAqvSlhS8j0kbnZVGdukaSLQ3X0GV8WSqo4Z6a8t2KX TPockopKwAnV065NGmrQA== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeftddruddvuddgvddtucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdggtfgfnhhsuhgsshgtrhhisggvpdfu rfetoffkrfgpnffqhgenuceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnh htshculddquddttddmnecujfgurhepkfffgggfuffvvehfhfgjtgfgsehtkeertddtvdej necuhfhrohhmpeffmhhithhrhicuifhuthhovhcuoegumhhithhrhiesghhuthhovhdrug gvvheqnecuggftrfgrthhtvghrnhepfedvjeeviefffeeukeelveeikeegtddtveeileev gfdvgffhtdfggeeffeegiefgnecuffhomhgrihhnpehgihhthhhusgdrtghomhenucevlh hushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpegumhhithhrhies ghhuthhovhdruggvvhdpnhgspghrtghpthhtohepvddpmhhouggvpehsmhhtphhouhhtpd hrtghpthhtohepjhhorghothgrvhhorhgrsehgmhgrihhlrdgtohhmpdhrtghpthhtohep jedvjedtheesuggvsggsuhhgshdrghhnuhdrohhrgh X-ME-Proxy: Feedback-ID: i0e71465a:Fastmail Original-Received: by mail.messagingengine.com (Postfix) with ESMTPA; Thu, 22 Aug 2024 19:16:17 -0400 (EDT) Content-Language: en-US In-Reply-To: <87o75kwl2i.fsf@gmail.com> 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:290565 Archived-At: On 22/08/2024 19:59, João Távora wrote: > Dmitry Gutov writes: > >> 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? >>> Unfortunately, if the the two ints were named "foo_bar_1" and >>> "foo_bar_2" it would also complete to foo_bar_, which is wrong in the >>> general case, since that text is not a fully formed completion. >>> exit-function cannot run here. >> >> Is it really a problem? > > Yes. Start 'clangd --completion-style=detailed' in this thingy.cpp C++ > file: > > int foobar(int x, char c) { > return x + c; > } > > int foobar(int x, float f) { > return x + f; > } > > int main() { > foob > } > > if you press C-M-i here this will expand to the partial 'foobar(int x, ' > which doesn't make C/C++ syntactic sense in that context. Emacs is just > inserting the LSP "label", which as the name implies, is just for > displaying, not for inserting. The goal of these two completions is to > expand a snippet and the snippet can only be expanded through the exit > function. Even if we complexified Eglot's capf to insert the > 'insertText' instead of the 'label' (assuming it existed), we'd be > inserting a partial snippet skeleton, even more nonsensical in C/C++. Thanks, now I remember: it's https://github.com/company-mode/company-mode/issues/1412. 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. >> 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. >> 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. >>> +(ert-deftest eglot-test-try-completion-inside-symbol () >>> + "Test completion table inside symbol, with only prefix matching." >>> + (skip-unless (executable-find "clangd")) >>> + (eglot--with-fixture >>> + `(("project" . (("coiso.c" . >>> + ,(concat >>> + "int foobar;" >>> + "int main() {foo123"))))) >>> + (with-current-buffer >>> + (eglot--find-file-noselect "project/coiso.c") >>> + (eglot--wait-for-clangd) >>> + (goto-char (- (point-max) 3)) >>> + (should >>> + (equal >>> + '("foo123" . 3) >>> + (completion-try-completion >>> + "foo123" >>> + (nth 2 (eglot-completion-at-point)) nil 3)))))) >>> I don't understand the point of this last test. Are you checking >>> that >>> nothing changes? What are you guarding against? >> >> With the input, I'm describing the actual prefix and suffix that get >> used. It might help someone who comes later to understand this >> separate case. Indeed "nothing happens", it's just that something did >> happen with the previous version of my patch, and we're testing >> against that. > > Fine. So maybe be a little bit more explicit in the comment or > docstring about what you _don't_ want to happen. AFAIU, you don't want > 'foobar123' or 'foobar' to be the result of that completion. Right! >> Might be more interesting to have a similar example where the prefix >> changes while the (non-emtpy) suffix stays the same, but I think we >> aren't going to support this in this c-style. >> >>> Finally, an interesting test could be the rust-analyzer one from the >>> issue 1339 issue, which starts with. >>> fn main() { >>> let v: usize = 1; >>> 1234.1234567890 >>> } >>> I've manually checked that neither the v.c1234.12345676890 nor the >>> v.count_on1234.1234567890 case has regressed. >> >> Yep, that's the one I fixed in the last revision. >> >> 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 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)?