From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: dmitry@gutov.dev, monnier@iro.umontreal.ca, bug-gnu-emacs@gnu.org Resent-Date: Mon, 18 Aug 2025 18:54:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: report 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: 79265@debbugs.gnu.org Cc: Dmitry Gutov , Stefan Monnier X-Debbugs-Original-To: bug-gnu-emacs@gnu.org X-Debbugs-Original-Xcc: Dmitry Gutov , Stefan Monnier Received: via spool by submit@debbugs.gnu.org id=B.175554321713128 (code B ref -1); Mon, 18 Aug 2025 18:54:01 +0000 Received: (at submit) by debbugs.gnu.org; 18 Aug 2025 18:53:37 +0000 Received: from localhost ([127.0.0.1]:51235 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uo4zA-0003Pf-T3 for submit@debbugs.gnu.org; Mon, 18 Aug 2025 14:53:37 -0400 Received: from lists.gnu.org ([2001:470:142::17]:55600) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uo4z7-0003PN-Ok for submit@debbugs.gnu.org; Mon, 18 Aug 2025 14:53:34 -0400 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 1uo4z2-0008IR-1f for bug-gnu-emacs@gnu.org; Mon, 18 Aug 2025 14:53:28 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1uo4z0-0002D1-3r for bug-gnu-emacs@gnu.org; Mon, 18 Aug 2025 14:53:27 -0400 From: Spencer Baugh Date: Mon, 18 Aug 2025 14:53:24 -0400 Message-ID: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1755543204; bh=0UOMvyLshxG8yMIIAnetR9YdN2VH1648Mip8uiHAa2Y=; h=From:To:Subject:Date; b=FXe1YYyjTMlyCwzamuyxGl4bVyF5o1qIU/OTIkELXLNz//69g9a89dtVGaWw+5KWR Ovi5kQObQC4VaeJYJot3bfFn/JofZgG/2fEpGN0qg1qBX97tnIhzufN2obAqqD4r4S Sy88/gF26izeXL4xvCe0MBDYmUxNBCy5EnvFFuhIpgwvItCKRWoRoUYGFuOv5uaea0 yLgcoYNt59xCgoCGvxbZI/wmMFSB7rQtW1FsTZPRPG0TGSD5GLoPDocrPJGrTkYR+b +MtqK7Y3fa0/aVaf50a8VHN3VouEIJU3V0Kwsv76RGNfnzCBnCUtsLhhCDge8lz1NI 8UQ+nv+cKJQbg== Received-SPF: pass client-ip=64.215.233.18; envelope-from=sbaugh@janestreet.com; helo=mxout5.mail.janestreet.com X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: 0.9 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.1 (/) --=-=-= Content-Type: text/plain Tags: patch Properly fix bug#38458, which is fundamentally an issue with completion-ignore-case, by checking if the completions are unique ignoring case. When the completions are unique, the normal code to delete a wildcard naturally causes point to be moved to the end of the minibuffer, which is the correct behavior. Now that the bug is fixed properly, remove a hack which previously was used to "fix" it, which made point behave inconsistently if it was in the middle of the minibuffer versus at the end of the minibuffer. * lisp/minibuffer.el (completion-pcm--merge-completions): Respect completion-ignore-case when checking for completion uniqueness. (completion-pcm--string->pattern) (completion-pcm--optimize-pattern): Allow point at the end of the pattern. * test/lisp/minibuffer-tests.el (completion-table-test-quoting) (completion-test--pcm-bug38458, completion-pcm-test-8): Update tests for more correct behavior. In GNU Emacs 30.1.90 (build 9, x86_64-pc-linux-gnu, X toolkit, cairo version 1.15.12, Xaw scroll bars) of 2025-08-14 built on igm-qws-u22796a Repository revision: 6adc26ffa74aedbd1cfa9a1ee72073ebccea2b96 Repository branch: emacs-30 Windowing system distributor 'The X.Org Foundation', version 11.0.12011000 System Description: Rocky Linux 8.10 (Green Obsidian) Configured using: 'configure --with-x-toolkit=lucid --without-gpm --without-gconf --without-selinux --without-imagemagick --with-modules --with-gif=no --with-cairo --with-rsvg --without-compress-install --with-tree-sitter --with-native-compilation=aot PKG_CONFIG_PATH=/usr/local/home/garnish/libtree-sitter/0.22.6-1/lib/pkgconfig/' --=-=-= Content-Type: text/patch Content-Disposition: attachment; filename=0001-Treat-point-more-consistently-in-PCM-completion.patch >From dbb5099a511f90e19761c0cf105c929e71a3c081 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Mon, 18 Aug 2025 14:50:44 -0400 Subject: [PATCH] Treat point more consistently in PCM completion Properly fix bug#38458, which is fundamentally an issue with completion-ignore-case, by checking if the completions are unique ignoring case. When the completions are unique, the normal code to delete a wildcard naturally causes point to be moved to the end of the minibuffer, which is the correct behavior. Now that the bug is fixed properly, remove a hack which previously was used to "fix" it, which made point behave inconsistently if it was in the middle of the minibuffer versus at the end of the minibuffer. * lisp/minibuffer.el (completion-pcm--merge-completions): Respect completion-ignore-case when checking for completion uniqueness. (completion-pcm--string->pattern) (completion-pcm--optimize-pattern): Allow point at the end of the pattern. * test/lisp/minibuffer-tests.el (completion-table-test-quoting) (completion-test--pcm-bug38458, completion-pcm-test-8): Update tests for more correct behavior. --- lisp/minibuffer.el | 15 +++++++-------- test/lisp/minibuffer-tests.el | 28 +++++++++++++++++++++------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 689cda7edab..046d792b27d 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4237,7 +4237,7 @@ completion-pcm--string->pattern "Split STRING into a pattern. A pattern is a list where each element is either a string or a symbol, see `completion-pcm--merge-completions'." - (if (and point (< point (length string))) + (if (and point (<= point (length string))) (let ((prefix (substring string 0 point)) (suffix (substring string point))) (append (completion-pcm--string->pattern prefix) @@ -4298,12 +4298,6 @@ completion-pcm--optimize-pattern (pcase p (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_) (setq p (cdr p))) - ;; This is not just a performance improvement: it turns a - ;; terminating `point' into an implicit `any', which affects - ;; the final position of point (because `point' gets turned - ;; into a non-greedy ".*?" regexp whereas we need it to be - ;; greedy when it's at the end, see bug#38458). - (`(point) (setq p nil)) ;Implicit terminating `any'. (_ (push (pop p) n)))) (nreverse n))) @@ -4757,7 +4751,12 @@ completion-pcm--merge-completions (let* ((prefix (try-completion fixed comps)) (unique (or (and (eq prefix t) (setq prefix fixed)) (and (stringp prefix) - (eq t (try-completion prefix comps)))))) + (eq t (try-completion prefix comps))) + (and completion-ignore-case + ;; They're all the same string, ignoring case. + (seq-every-p + (lambda (elem) (string-equal-ignore-case elem prefix)) + comps))))) ;; If there's only one completion, `elem' is not useful ;; any more: it can only match the empty string. ;; FIXME: in some cases, it may be necessary to turn an diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el index f9a26d17e58..59b72899e22 100644 --- a/test/lisp/minibuffer-tests.el +++ b/test/lisp/minibuffer-tests.el @@ -100,10 +100,10 @@ completion-table-test-quoting ;; Test that $$ in input is properly unquoted. ("data/m-cttq$$t" "data/minibuffer-test-cttq$$tion") ;; Test that env-vars are preserved. - ("lisp/c${CTTQ1}et/se-u" "lisp/c${CTTQ1}et/semantic-utest") - ("lisp/ced${CTTQ2}se-u" "lisp/ced${CTTQ2}semantic-utest") + ("lisp/c${CTTQ1}et/se-u-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") + ("lisp/ced${CTTQ2}se-u-c" "lisp/ced${CTTQ2}semantic-utest-c.test") ;; Test that env-vars don't prevent partial-completion. - ("lis/c${CTTQ1}/se-u" "lisp/c${CTTQ1}et/semantic-utest") + ("lis/c${CTTQ1}/se-u-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") )) (should (equal (completion-try-completion input #'completion--file-name-table @@ -118,11 +118,11 @@ completion-table-test-quoting ;; When an env var is in the completion bounds, try-completion ;; won't change letter case. ("lisp/c${CTTQ1}E" "lisp/c${CTTQ1}Et/") - ("lisp/ced${CTTQ2}SE-U" "lisp/ced${CTTQ2}SEmantic-utest") + ("lisp/ced${CTTQ2}SE-U-c" "lisp/ced${CTTQ2}SEmantic-utest-c.test") ;; If the env var is before the completion bounds, try-completion ;; *will* change letter case. - ("lisp/c${CTTQ1}et/SE-U" "lisp/c${CTTQ1}et/semantic-utest") - ("lis/c${CTTQ1}/SE-U" "lisp/c${CTTQ1}et/semantic-utest") + ("lisp/c${CTTQ1}et/SE-U-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") + ("lis/c${CTTQ1}/SE-U-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") )) (should (equal (car (completion-try-completion input #'completion--file-name-table @@ -224,7 +224,11 @@ completion-test--pcm-bug38458 (completion-pcm--merge-try '("tes" point "ing") '("Testing" "testing") "" "")) - '("testing" . 4)))) + '("testing" . 7))) + (should (equal + (let ((completion-ignore-case t)) + (completion-pcm-try-completion "tes" '("Testing" "testing") nil 3)) + '("testing" . 7)))) (ert-deftest completion-pcm-test-1 () ;; Point is at end, this does not match anything @@ -318,6 +322,16 @@ completion-pcm-test-7 '(prefix any "bar" any) '("xbarxfoo" "ybaryfoo") "" "") '("bar" . 3)))) +(ert-deftest completion-pcm-test-8 () + ;; try-completion inserts the common prefix and suffix at point. + (should (equal (completion-pcm-try-completion + "r" '("fooxbar" "fooybar") nil 0) + '("foobar" . 3))) + ;; Even if point is at the end of the minibuffer. + (should (equal (completion-pcm-try-completion + "" '("fooxbar" "fooybar") nil 0) + '("foobar" . 3)))) + (ert-deftest completion-substring-test-1 () ;; One third of a match! (should (equal -- 2.43.7 --=-=-=-- From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 19 Aug 2025 04:05:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Spencer Baugh , 79265@debbugs.gnu.org Cc: Stefan Monnier Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175557625429503 (code B ref 79265); Tue, 19 Aug 2025 04:05:02 +0000 Received: (at 79265) by debbugs.gnu.org; 19 Aug 2025 04:04:14 +0000 Received: from localhost ([127.0.0.1]:52145 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uoDa2-0007fm-3C for submit@debbugs.gnu.org; Tue, 19 Aug 2025 00:04:14 -0400 Received: from fout-b4-smtp.messagingengine.com ([202.12.124.147]:58667) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uoDa0-0007fN-26 for 79265@debbugs.gnu.org; Tue, 19 Aug 2025 00:04:13 -0400 Received: from phl-compute-12.internal (phl-compute-12.internal [10.202.2.52]) by mailfout.stl.internal (Postfix) with ESMTP id BCA061D0024F; Tue, 19 Aug 2025 00:04:05 -0400 (EDT) Received: from phl-mailfrontend-01 ([10.202.2.162]) by phl-compute-12.internal (MEProxy); Tue, 19 Aug 2025 00:04:05 -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=1755576245; x=1755662645; bh=+VqopdhMavV4pj/R2c3GifN3T8+drqC0OHpiwQI3Ib0=; b= cqXzAhtCd1Ik1USVv2T52HoHmFJ+owc2eS5GInfgbNamRyLkMJFzXHg1zMCb0IS8 66a+P9OG4swP8HiaMWAgkCDRDqdTYSVkYGiG2VvhXJYUZx3epu+2G8Z3khBPJ5H8 m5Zl0zfmVbxcx/qLi9T/LttkkN0knKm4NWwBfA6hp5BgeNzTNXvaKMIlrrZJid5b 6yi1Og3DhjJrlDIas5lLjQgjEDQLXn9mDkBEoVRm1dDcWmhFmkOt/KzfSPHwanOR OoNNzFt1xvUdYvyAQIqaexkM9+FPTTjxO3dsYntdiP+LCekQOqR9Eq/8Wyl9MvLN uLYw4Xlx7Bl1THK1tW42DA== 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-sender:x-me-sender:x-sasl-enc; s=fm3; t=1755576245; x= 1755662645; bh=+VqopdhMavV4pj/R2c3GifN3T8+drqC0OHpiwQI3Ib0=; b=k 38prDqRC5RzYGUpkLfd9k5gDeKqwoTIJDTHE7StOK2BTe9yH/ND/WCrO7ymRdDqt MSp0bbaDLVOMYLhDuA+sfkz9Sg3FxHiDnGTOVsseKduPDx1zsC21cyVdYzmPI5OH /SEF2y6syAm8d/aLmVJW04h3ywfD8yXipe+JmHmdWryT6HrmERjM5rFgcY0vo6yq h8mQGQ1lkRn0PfyKiWR33EjUFxDkjiqkLrzANoC1jxaxRdTw44nlSvMfKDzKqIst O6F6Bv3PPQDHR83CQBcz3IEfHHrDMus/ZhPkQdU/wbt2sF/1KTOGNyevFvsHX4hH SEUELeKNgG7vop1ru7pUg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdefgdduheeggeekucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhepkfffgggfuffvvehfhfgjtgfgsehtjeertddtvdejnecuhfhrohhmpeffmhhithhr hicuifhuthhovhcuoegumhhithhrhiesghhuthhovhdruggvvheqnecuggftrfgrthhtvg hrnhepteduleejgeehtefgheegjeekueehvdevieekueeftddvtdevfefhvdevgedujeeh necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepughmih htrhihsehguhhtohhvrdguvghvpdhnsggprhgtphhtthhopeefpdhmohguvgepshhmthhp ohhuthdprhgtphhtthhopehssggruhhghhesjhgrnhgvshhtrhgvvghtrdgtohhmpdhrtg hpthhtohepjeelvdeiheesuggvsggsuhhgshdrghhnuhdrohhrghdprhgtphhtthhopehm ohhnnhhivghrsehirhhordhumhhonhhtrhgvrghlrdgtrg X-ME-Proxy: Feedback-ID: i07de48aa:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Tue, 19 Aug 2025 00:04:04 -0400 (EDT) Message-ID: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> Date: Tue, 19 Aug 2025 07:04:02 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird References: Content-Language: en-US From: Dmitry Gutov In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) On 18/08/2025 21:53, Spencer Baugh wrote: > Properly fix bug#38458, which is fundamentally an issue with > completion-ignore-case, by checking if the completions are > unique ignoring case. When the completions are unique, the > normal code to delete a wildcard naturally causes point to be > moved to the end of the minibuffer, which is the correct > behavior. > > Now that the bug is fixed properly, remove a hack which > previously was used to "fix" it, which made point behave > inconsistently if it was in the middle of the minibuffer versus > at the end of the minibuffer. Thanks, the description makes sense and the tests are great to have. I wonder if Stefan will want to comment though. Just one thing: > + ;; They're all the same string, ignoring case. > + (seq-every-p > + (lambda (elem) (string-equal-ignore-case elem prefix)) > + comps))))) Is this performance-sensitive? Using a less abstracted function like cl-every of even a (cl-loop for ... always ...) could be faster. There is a (dolist) above it which iterates across segments of the pattern, so whatever overhead is added could be multiplied; OT2H I haven't tested it myself yet, maybe all bottlenecks are somewhere else. From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Tue, 19 Aug 2025 16:04:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Dmitry Gutov Cc: Stefan Monnier , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175561941532053 (code B ref 79265); Tue, 19 Aug 2025 16:04:02 +0000 Received: (at 79265) by debbugs.gnu.org; 19 Aug 2025 16:03:35 +0000 Received: from localhost ([127.0.0.1]:54565 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uoOoA-0008Ks-JU for submit@debbugs.gnu.org; Tue, 19 Aug 2025 12:03:35 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:36703) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uoOo4-0008KU-E1 for 79265@debbugs.gnu.org; Tue, 19 Aug 2025 12:03:32 -0400 From: Spencer Baugh In-Reply-To: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> (Dmitry Gutov's message of "Tue, 19 Aug 2025 07:04:02 +0300") References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> Date: Tue, 19 Aug 2025 12:03:22 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1755619402; bh=SOklNgKWD8FFpM2A9vOPMDyqsMT4jOVz9hedntJEXho=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=V0/vJ8HpPgmhANp2Q4e3aeW9kazq4KlhO0HOhLCxtZixqrZy9yOnU52yhIg6kGhpf aH5p4rQg7tiHip70LHUhmYs+mR0Yo52Z4sRKv+GAYhNZXiDg/fR/XAsm4/RO5yqllm 8HmX05lOTKZAvkbCv02szY5e8qwzALnAgQTBGWvzXY2MTlRX4Vs+QZuG/U9UPdBJd4 9iptJnte7Pyupro0Xtqii/9DeehwSqknc2QjCeFxMVGhTFF1XDo7nu6ZB79aUlc/ye u61bVjTJ3yhv0gK7pbBz2aL7i5cIzQEAYGYvzcwGnpQq5DP03KE2tUDrr1y9jJFRkk cNhNRa0fDq6Vw== X-Spam-Score: 0.0 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) --=-=-= Content-Type: text/plain Dmitry Gutov writes: > On 18/08/2025 21:53, Spencer Baugh wrote: >> Properly fix bug#38458, which is fundamentally an issue with >> completion-ignore-case, by checking if the completions are >> unique ignoring case. When the completions are unique, the >> normal code to delete a wildcard naturally causes point to be >> moved to the end of the minibuffer, which is the correct >> behavior. >> Now that the bug is fixed properly, remove a hack which >> previously was used to "fix" it, which made point behave >> inconsistently if it was in the middle of the minibuffer versus >> at the end of the minibuffer. > > Thanks, the description makes sense and the tests are great to have. > > I wonder if Stefan will want to comment though. > > Just one thing: > >> + ;; They're all the same string, ignoring case. >> + (seq-every-p >> + (lambda (elem) (string-equal-ignore-case elem prefix)) >> + comps))))) > > Is this performance-sensitive? Using a less abstracted function like > cl-every of even a (cl-loop for ... always ...) could be faster. > > There is a (dolist) above it which iterates across segments of the > pattern, so whatever overhead is added could be multiplied; OT2H I > haven't tested it myself yet, maybe all bottlenecks are somewhere > else. Hm, good point. After thinking about it more, I realized that the second try-completion call above somewhat duplicates the check I added, and that I could refactor in general to improve performance here. So I reworked the change a bit to combine my check with the second try-completion call. This should have better performance even though we aren't using the C implementation of try-completion anymore, since we're now doing much less work than try-completion does. (I also refactored to use a cond instead of (or (and ...) (and ...)), which I think makes this logic a fair bit clearer) --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Treat-point-more-consistently-in-PCM-completion.patch >From 9465045d19a8676847cc8932ca66a40aabe2af59 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Mon, 18 Aug 2025 14:50:44 -0400 Subject: [PATCH] Treat point more consistently in PCM completion Properly fix bug#38458, which is fundamentally an issue with completion-ignore-case, by checking if the completions are unique ignoring case. When the completions are unique, the normal code to delete a wildcard naturally causes point to be moved to the end of the minibuffer, which is the correct behavior. Now that the bug is fixed properly, remove a hack which previously was used to "fix" it, which made point behave inconsistently if it was in the middle of the minibuffer versus at the end of the minibuffer. * lisp/minibuffer.el (completion-pcm--merge-completions): Respect completion-ignore-case when checking for completion uniqueness. (bug#79265) (completion-pcm--string->pattern) (completion-pcm--optimize-pattern): Allow point at the end of the pattern. * test/lisp/minibuffer-tests.el (completion-table-test-quoting) (completion-test--pcm-bug38458, completion-pcm-test-8): Update tests for more correct behavior. --- lisp/minibuffer.el | 25 +++++++++++++++---------- test/lisp/minibuffer-tests.el | 28 +++++++++++++++++++++------- 2 files changed, 36 insertions(+), 17 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 689cda7edab..c7efba10fec 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4237,7 +4237,7 @@ completion-pcm--string->pattern "Split STRING into a pattern. A pattern is a list where each element is either a string or a symbol, see `completion-pcm--merge-completions'." - (if (and point (< point (length string))) + (if (and point (<= point (length string))) (let ((prefix (substring string 0 point)) (suffix (substring string point))) (append (completion-pcm--string->pattern prefix) @@ -4298,12 +4298,6 @@ completion-pcm--optimize-pattern (pcase p (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_) (setq p (cdr p))) - ;; This is not just a performance improvement: it turns a - ;; terminating `point' into an implicit `any', which affects - ;; the final position of point (because `point' gets turned - ;; into a non-greedy ".*?" regexp whereas we need it to be - ;; greedy when it's at the end, see bug#38458). - (`(point) (setq p nil)) ;Implicit terminating `any'. (_ (push (pop p) n)))) (nreverse n))) @@ -4755,9 +4749,20 @@ completion-pcm--merge-completions ;; In practice, it doesn't seem to make any difference. (setq ccs (nreverse ccs)) (let* ((prefix (try-completion fixed comps)) - (unique (or (and (eq prefix t) (setq prefix fixed)) - (and (stringp prefix) - (eq t (try-completion prefix comps)))))) + (unique + (cond + ((eq prefix t) + (setq prefix fixed) + t) + ((stringp prefix) + ;; `try-completion' is almost this, but it doesn't + ;; handle `completion-ignore-case' the way we want. + (not (cl-some + (lambda (comp) + ;; PREFIX could complete to COMP, so it's not unique. + (and (string-prefix-p prefix comp completion-ignore-case) + (not (= (length prefix) (length comp))))) + comps)))))) ;; If there's only one completion, `elem' is not useful ;; any more: it can only match the empty string. ;; FIXME: in some cases, it may be necessary to turn an diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el index f9a26d17e58..59b72899e22 100644 --- a/test/lisp/minibuffer-tests.el +++ b/test/lisp/minibuffer-tests.el @@ -100,10 +100,10 @@ completion-table-test-quoting ;; Test that $$ in input is properly unquoted. ("data/m-cttq$$t" "data/minibuffer-test-cttq$$tion") ;; Test that env-vars are preserved. - ("lisp/c${CTTQ1}et/se-u" "lisp/c${CTTQ1}et/semantic-utest") - ("lisp/ced${CTTQ2}se-u" "lisp/ced${CTTQ2}semantic-utest") + ("lisp/c${CTTQ1}et/se-u-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") + ("lisp/ced${CTTQ2}se-u-c" "lisp/ced${CTTQ2}semantic-utest-c.test") ;; Test that env-vars don't prevent partial-completion. - ("lis/c${CTTQ1}/se-u" "lisp/c${CTTQ1}et/semantic-utest") + ("lis/c${CTTQ1}/se-u-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") )) (should (equal (completion-try-completion input #'completion--file-name-table @@ -118,11 +118,11 @@ completion-table-test-quoting ;; When an env var is in the completion bounds, try-completion ;; won't change letter case. ("lisp/c${CTTQ1}E" "lisp/c${CTTQ1}Et/") - ("lisp/ced${CTTQ2}SE-U" "lisp/ced${CTTQ2}SEmantic-utest") + ("lisp/ced${CTTQ2}SE-U-c" "lisp/ced${CTTQ2}SEmantic-utest-c.test") ;; If the env var is before the completion bounds, try-completion ;; *will* change letter case. - ("lisp/c${CTTQ1}et/SE-U" "lisp/c${CTTQ1}et/semantic-utest") - ("lis/c${CTTQ1}/SE-U" "lisp/c${CTTQ1}et/semantic-utest") + ("lisp/c${CTTQ1}et/SE-U-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") + ("lis/c${CTTQ1}/SE-U-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") )) (should (equal (car (completion-try-completion input #'completion--file-name-table @@ -224,7 +224,11 @@ completion-test--pcm-bug38458 (completion-pcm--merge-try '("tes" point "ing") '("Testing" "testing") "" "")) - '("testing" . 4)))) + '("testing" . 7))) + (should (equal + (let ((completion-ignore-case t)) + (completion-pcm-try-completion "tes" '("Testing" "testing") nil 3)) + '("testing" . 7)))) (ert-deftest completion-pcm-test-1 () ;; Point is at end, this does not match anything @@ -318,6 +322,16 @@ completion-pcm-test-7 '(prefix any "bar" any) '("xbarxfoo" "ybaryfoo") "" "") '("bar" . 3)))) +(ert-deftest completion-pcm-test-8 () + ;; try-completion inserts the common prefix and suffix at point. + (should (equal (completion-pcm-try-completion + "r" '("fooxbar" "fooybar") nil 0) + '("foobar" . 3))) + ;; Even if point is at the end of the minibuffer. + (should (equal (completion-pcm-try-completion + "" '("fooxbar" "fooybar") nil 0) + '("foobar" . 3)))) + (ert-deftest completion-substring-test-1 () ;; One third of a match! (should (equal -- 2.43.7 --=-=-=-- From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 20 Aug 2025 12:13:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Spencer Baugh Cc: Stefan Monnier , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175569194019104 (code B ref 79265); Wed, 20 Aug 2025 12:13:02 +0000 Received: (at 79265) by debbugs.gnu.org; 20 Aug 2025 12:12:20 +0000 Received: from localhost ([127.0.0.1]:56358 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uohfw-0004y4-De for submit@debbugs.gnu.org; Wed, 20 Aug 2025 08:12:20 -0400 Received: from fout-a8-smtp.messagingengine.com ([103.168.172.151]:45887) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uohft-0004xp-BC for 79265@debbugs.gnu.org; Wed, 20 Aug 2025 08:12:18 -0400 Received: from phl-compute-01.internal (phl-compute-01.internal [10.202.2.41]) by mailfout.phl.internal (Postfix) with ESMTP id ADCADEC08CA; Wed, 20 Aug 2025 08:12:11 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-01.internal (MEProxy); Wed, 20 Aug 2025 08:12:11 -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=1755691931; x=1755778331; bh=k1KS1K+lCNg3oheSG1J9696/9gYN/+Eud2KyRBfN32Y=; b= X3n/VOD6bV+ns4aY410avrjPpJ/9VJ6YFyzZgCF+izZK8Zw9jrIeoX0MUvvv8XcY fLMK8u08AD1/qWGCDt07PRQYR8jONe6uop7NPstcAXLiixRPrBr5CJ/V7z6JqsB3 19P3i+IDuK+SGheBqddfPQ5DQy2H1cae8M+ACWNkeWclsKKMHlEIgSEZJ6EiGMNP tj7nqEFMlaoTzZaZha0MS3xG02s1BHjs1amzn28QDZevJpnvIeF7aOzUXzuPsedU b5Ly0dhLHRosj4qa7XJYnGnA4O7xVbQgKfNbiC3Ux1JmkcCR56uRLh6RY459p1c3 /i/oyqrDDSp78FH7SE5XJg== 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-sender:x-me-sender:x-sasl-enc; s=fm3; t=1755691931; x= 1755778331; bh=k1KS1K+lCNg3oheSG1J9696/9gYN/+Eud2KyRBfN32Y=; b=h qec9I6akMN6UDzEn4iMIGIEZmteqM7afVfGs2ND0il3iCMksP6ASUblrOt8SPRxy qL0rc08sjDq4i45wZYXlqMKOUHpCZh5aV6YJatqBbOOt0LtNe/Ih1QYzpa/sO+y7 JuTTnNBlvrDvQrF9euggjH+o7FR3FYT9glST4QBLTf+6ksznMCGM/Ggbmayj1SWU M2qiyN22BlrALKN89IX9gnVv6015jwyv5+osFbEpkY2QKzCbSN2W1w9hbjWM3vwN DlvA7nbD5SzePn7K6gxs9U92PGuPPtFOEhUSO4YqS/qrbnQey0/aEECk7rH4F6VS xkxGbASqzoSEF7Qb4xsyQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdefgdduheekfeegucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhepkfffgggfuffvvehfhfgjtgfgsehtkeertddtvdejnecuhfhrohhmpeffmhhithhr hicuifhuthhovhcuoegumhhithhrhiesghhuthhovhdruggvvheqnecuggftrfgrthhtvg hrnhepudegteevgedvhfegieetueeujedvhffhfedutdeiieejkeefheehhfekteehteeg necuffhomhgrihhnpegvlhdrshhonecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrg hmpehmrghilhhfrhhomhepughmihhtrhihsehguhhtohhvrdguvghvpdhnsggprhgtphht thhopeefpdhmohguvgepshhmthhpohhuthdprhgtphhtthhopehssggruhhghhesjhgrnh gvshhtrhgvvghtrdgtohhmpdhrtghpthhtohepjeelvdeiheesuggvsggsuhhgshdrghhn uhdrohhrghdprhgtphhtthhopehmohhnnhhivghrsehirhhordhumhhonhhtrhgvrghlrd gtrg X-ME-Proxy: Feedback-ID: i07de48aa:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 20 Aug 2025 08:12:10 -0400 (EDT) Message-ID: <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Wed, 20 Aug 2025 15:12:08 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> Content-Language: en-US From: Dmitry Gutov In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Score: -0.7 (/) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) On 19/08/2025 19:03, Spencer Baugh wrote: > Hm, good point. After thinking about it more, I realized that the > second try-completion call above somewhat duplicates the check I added, > and that I could refactor in general to improve performance here. > > So I reworked the change a bit to combine my check with the second > try-completion call. This should have better performance even though we > aren't using the C implementation of try-completion anymore, since we're > now doing much less work than try-completion does. > > (I also refactored to use a cond instead of (or (and ...) (and ...)), > which I think makes this logic a fair bit clearer) Hi, this looks sensible. Just a couple of questions. > - (unique (or (and (eq prefix t) (setq prefix fixed)) > - (and (stringp prefix) > - (eq t (try-completion prefix comps)))))) > + (unique > + (cond > + ((eq prefix t) > + (setq prefix fixed) > + t) > + ((stringp prefix) > + ;; `try-completion' is almost this, but it doesn't > + ;; handle `completion-ignore-case' the way we want. > + (not (cl-some This gets a warning "the function ‘cl-some’ might not be defined at runtime" because cl-lib is only loaded for macros, in minibuffer.el. So IDK - maybe it's better to use the alien cl-loop, or maybe to go back to seq-some, if no measurements show a cause for worry. > + (lambda (comp) > + ;; PREFIX could complete to COMP, so it's not unique. > + (and (string-prefix-p prefix comp completion-ignore-case) > + (not (= (length prefix) (length comp))))) > + comps)))))) Just to check: in our limited test examples all elements of COMP match PREFIX already. And the beginning of 'completion-pcm--merge-completions' even 'error'-s if string-match comparison fails. So it's possible that we could omit this comparison from the lambda: (and (string-prefix-p prefix comp completion-ignore-case) But I suppose that would make it a bigger change, a bit riskier. From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 20 Aug 2025 14:36:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Dmitry Gutov Cc: Stefan Monnier , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175570055414777 (code B ref 79265); Wed, 20 Aug 2025 14:36:02 +0000 Received: (at 79265) by debbugs.gnu.org; 20 Aug 2025 14:35:54 +0000 Received: from localhost ([127.0.0.1]:57223 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uojur-0003qD-N7 for submit@debbugs.gnu.org; Wed, 20 Aug 2025 10:35:54 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:54979) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uojuo-0003pr-EI for 79265@debbugs.gnu.org; Wed, 20 Aug 2025 10:35:51 -0400 From: Spencer Baugh In-Reply-To: <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> (Dmitry Gutov's message of "Wed, 20 Aug 2025 15:12:08 +0300") References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Wed, 20 Aug 2025 10:35:44 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1755700544; bh=/9rC4O3qkhNogJURq+KNXSKRYF80EH7RwIMecFJ4Ulc=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=AaYJ9VL7RM6s0pt6I3dMvrsf7LKxghdvvM96a+iFiH+LmsSIxecnJwxb3f6XsMm9S MBbs/WL1JaveWn2OFWJiWoUFoD1QltikEAEZdEaXktPCzC3EJr1xbU3zPwxrO8JkXB Z0u77ITo1KeUrDdSBMO5LqoDk0OuzRcI2QyDqXZQIkowA6uOcTIJg9pzumuSFPuGjM 8qviUTuMXZPQC5Eqit8zuWnksnvotpjyBA7hg94X6Wpec1H4VjZdQSsilSb8LnFHXk dAIwew7cjbhobxYzephYRr7ZOEv4YTE4ZhmSu321vjP0KL4ruRXSmtIPXjkQKRCyBt r+o0QP3rSPMyQ== X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Dmitry Gutov writes: > On 19/08/2025 19:03, Spencer Baugh wrote: >> Hm, good point. After thinking about it more, I realized that the >> second try-completion call above somewhat duplicates the check I added, >> and that I could refactor in general to improve performance here. >> So I reworked the change a bit to combine my check with the second >> try-completion call. This should have better performance even though we >> aren't using the C implementation of try-completion anymore, since we're >> now doing much less work than try-completion does. >> (I also refactored to use a cond instead of (or (and ...) (and >> ...)), >> which I think makes this logic a fair bit clearer) > > Hi, this looks sensible. Just a couple of questions. > >> - (unique (or (and (eq prefix t) (setq prefix fixed)) >> - (and (stringp prefix) >> - (eq t (try-completion prefix comp= s)))))) >> + (unique >> + (cond >> + ((eq prefix t) >> + (setq prefix fixed) >> + t) >> + ((stringp prefix) >> + ;; `try-completion' is almost this, but it does= n't >> + ;; handle `completion-ignore-case' the way we w= ant. >> + (not (cl-some > > This gets a warning "the function =E2=80=98cl-some=E2=80=99 might not be = defined at > runtime" because cl-lib is only loaded for macros, in minibuffer.el. Oops, sorry, just missed that. > So IDK - maybe it's better to use the alien cl-loop, or maybe to go > back to seq-some, if no measurements show a cause for worry. Eh, I went back to seq-every-p for now. This shouldn't really cause a performance problem, since it will usually just return nil quickly. Also it's only run when the user hits TAB. (I'd rather not use cl-loop, this code is already hard enough to understand :) ) >> + (lambda (comp) >> + ;; PREFIX could complete to COMP, so it= 's not unique. >> + (and (string-prefix-p prefix comp compl= etion-ignore-case) >> + (not (=3D (length prefix) (length = comp))))) >> + comps)))))) > > Just to check: in our limited test examples all elements of COMP match > PREFIX already. And the beginning of > 'completion-pcm--merge-completions' even 'error'-s if string-match > comparison fails. > > So it's possible that we could omit this comparison from the lambda: > > (and (string-prefix-p prefix comp completion-ignore-case) > > But I suppose that would make it a bigger change, a bit riskier. Ah, you're right. I missed that we already have this property guaranteed, but indeed we do. So thanks to that, the change can be further simplified. Removing my cond refactoring, the patch is now just replacing (eq t (try-completion prefix comps)) with (seq-every-p (lambda (comp) (=3D (length prefix) (length comp))) comps) which is a nice simple change. I added some comments to clarify why this is correct, too. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Treat-point-more-consistently-in-PCM-completion.patch >From 292b02d873d99ec22385681a4622512fa3d8ce25 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Mon, 18 Aug 2025 14:50:44 -0400 Subject: [PATCH] Treat point more consistently in PCM completion Properly fix bug#38458, which is fundamentally an issue with completion-ignore-case, by checking if the completions are unique ignoring case. When the completions are unique, the normal code to delete a wildcard naturally causes point to be moved to the end of the minibuffer, which is the correct behavior. Now that the bug is fixed properly, remove a hack which previously was used to "fix" it, which made point behave inconsistently if it was in the middle of the minibuffer versus at the end of the minibuffer. * lisp/minibuffer.el (completion-pcm--merge-completions): Respect completion-ignore-case when checking for completion uniqueness. (bug#79265) (completion-pcm--string->pattern) (completion-pcm--optimize-pattern): Allow point at the end of the pattern. * test/lisp/minibuffer-tests.el (completion-table-test-quoting) (completion-test--pcm-bug38458, completion-pcm-test-8): Update tests for more correct behavior. --- lisp/minibuffer.el | 19 +++++++++++-------- test/lisp/minibuffer-tests.el | 28 +++++++++++++++++++++------- 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 689cda7edab..36202566136 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4237,7 +4237,7 @@ completion-pcm--string->pattern "Split STRING into a pattern. A pattern is a list where each element is either a string or a symbol, see `completion-pcm--merge-completions'." - (if (and point (< point (length string))) + (if (and point (<= point (length string))) (let ((prefix (substring string 0 point)) (suffix (substring string point))) (append (completion-pcm--string->pattern prefix) @@ -4298,12 +4298,6 @@ completion-pcm--optimize-pattern (pcase p (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_) (setq p (cdr p))) - ;; This is not just a performance improvement: it turns a - ;; terminating `point' into an implicit `any', which affects - ;; the final position of point (because `point' gets turned - ;; into a non-greedy ".*?" regexp whereas we need it to be - ;; greedy when it's at the end, see bug#38458). - (`(point) (setq p nil)) ;Implicit terminating `any'. (_ (push (pop p) n)))) (nreverse n))) @@ -4754,10 +4748,19 @@ completion-pcm--merge-completions ;; different capitalizations in different parts. ;; In practice, it doesn't seem to make any difference. (setq ccs (nreverse ccs)) + ;; FIXED is a prefix of all of COMPS. Try to grow that prefix. (let* ((prefix (try-completion fixed comps)) (unique (or (and (eq prefix t) (setq prefix fixed)) (and (stringp prefix) - (eq t (try-completion prefix comps)))))) + ;; If PREFIX is equal to all of COMPS, + ;; then PREFIX is a unique completion. + (seq-every-p + ;; PREFIX is still a prefix of all of + ;; COMPS, so if COMP is the same length, + ;; they're equal. + (lambda (comp) + (= (length prefix) (length comp))) + comps))))) ;; If there's only one completion, `elem' is not useful ;; any more: it can only match the empty string. ;; FIXME: in some cases, it may be necessary to turn an diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el index f9a26d17e58..59b72899e22 100644 --- a/test/lisp/minibuffer-tests.el +++ b/test/lisp/minibuffer-tests.el @@ -100,10 +100,10 @@ completion-table-test-quoting ;; Test that $$ in input is properly unquoted. ("data/m-cttq$$t" "data/minibuffer-test-cttq$$tion") ;; Test that env-vars are preserved. - ("lisp/c${CTTQ1}et/se-u" "lisp/c${CTTQ1}et/semantic-utest") - ("lisp/ced${CTTQ2}se-u" "lisp/ced${CTTQ2}semantic-utest") + ("lisp/c${CTTQ1}et/se-u-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") + ("lisp/ced${CTTQ2}se-u-c" "lisp/ced${CTTQ2}semantic-utest-c.test") ;; Test that env-vars don't prevent partial-completion. - ("lis/c${CTTQ1}/se-u" "lisp/c${CTTQ1}et/semantic-utest") + ("lis/c${CTTQ1}/se-u-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") )) (should (equal (completion-try-completion input #'completion--file-name-table @@ -118,11 +118,11 @@ completion-table-test-quoting ;; When an env var is in the completion bounds, try-completion ;; won't change letter case. ("lisp/c${CTTQ1}E" "lisp/c${CTTQ1}Et/") - ("lisp/ced${CTTQ2}SE-U" "lisp/ced${CTTQ2}SEmantic-utest") + ("lisp/ced${CTTQ2}SE-U-c" "lisp/ced${CTTQ2}SEmantic-utest-c.test") ;; If the env var is before the completion bounds, try-completion ;; *will* change letter case. - ("lisp/c${CTTQ1}et/SE-U" "lisp/c${CTTQ1}et/semantic-utest") - ("lis/c${CTTQ1}/SE-U" "lisp/c${CTTQ1}et/semantic-utest") + ("lisp/c${CTTQ1}et/SE-U-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") + ("lis/c${CTTQ1}/SE-U-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") )) (should (equal (car (completion-try-completion input #'completion--file-name-table @@ -224,7 +224,11 @@ completion-test--pcm-bug38458 (completion-pcm--merge-try '("tes" point "ing") '("Testing" "testing") "" "")) - '("testing" . 4)))) + '("testing" . 7))) + (should (equal + (let ((completion-ignore-case t)) + (completion-pcm-try-completion "tes" '("Testing" "testing") nil 3)) + '("testing" . 7)))) (ert-deftest completion-pcm-test-1 () ;; Point is at end, this does not match anything @@ -318,6 +322,16 @@ completion-pcm-test-7 '(prefix any "bar" any) '("xbarxfoo" "ybaryfoo") "" "") '("bar" . 3)))) +(ert-deftest completion-pcm-test-8 () + ;; try-completion inserts the common prefix and suffix at point. + (should (equal (completion-pcm-try-completion + "r" '("fooxbar" "fooybar") nil 0) + '("foobar" . 3))) + ;; Even if point is at the end of the minibuffer. + (should (equal (completion-pcm-try-completion + "" '("fooxbar" "fooybar") nil 0) + '("foobar" . 3)))) + (ert-deftest completion-substring-test-1 () ;; One third of a match! (should (equal -- 2.43.7 --=-=-=-- From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 20 Aug 2025 17:27:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Dmitry Gutov Cc: Stefan Monnier , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175571078815449 (code B ref 79265); Wed, 20 Aug 2025 17:27:01 +0000 Received: (at 79265) by debbugs.gnu.org; 20 Aug 2025 17:26:28 +0000 Received: from localhost ([127.0.0.1]:57548 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uomZv-000416-P9 for submit@debbugs.gnu.org; Wed, 20 Aug 2025 13:26:28 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:56189) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uomZq-00040m-NC for 79265@debbugs.gnu.org; Wed, 20 Aug 2025 13:26:26 -0400 From: Spencer Baugh In-Reply-To: (Spencer Baugh's message of "Wed, 20 Aug 2025 10:35:44 -0400") References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Wed, 20 Aug 2025 13:26:16 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1755710777; bh=OyccelAaF8mQglJtzI7XrmVC1D6uGmzVMduwrAQKSWU=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=NwRRbyzBny6tkOiDqbg6zWetNvlVarFsD8btqnyaXOgAqHPl6FEKd/cr5DW0B2pSU GiSkxW2iDHkrgG3DQD8yyW75NszHwYOGIyR67p3VsxbYhY6ADh0re5LS8dKMIa6UlG g99pnqdYwNtRjgU4Og306x2kEW7UvBtwoUC9GkdgN2oGYh9NTit+LFFvpGx3b2WQt8 cadtXJPbXexOwb8kKMS6yILVI2GlwvVphngItlk85eREd5JezjPzkbwAXxuUk+GSI7 1vrg8Rr69ifWoSYKQEtiOPsCvyTGy47oEK07JRP7T6OB1w8sUN7f0XcUtnl4Aky4Or F40xSwQ1ryl5Q== X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Spencer Baugh writes: > Dmitry Gutov writes: > >> On 19/08/2025 19:03, Spencer Baugh wrote: >>> Hm, good point. After thinking about it more, I realized that the >>> second try-completion call above somewhat duplicates the check I added, >>> and that I could refactor in general to improve performance here. >>> So I reworked the change a bit to combine my check with the second >>> try-completion call. This should have better performance even though we >>> aren't using the C implementation of try-completion anymore, since we're >>> now doing much less work than try-completion does. >>> (I also refactored to use a cond instead of (or (and ...) (and >>> ...)), >>> which I think makes this logic a fair bit clearer) >> >> Hi, this looks sensible. Just a couple of questions. >> >>> - (unique (or (and (eq prefix t) (setq prefix fixed= )) >>> - (and (stringp prefix) >>> - (eq t (try-completion prefix com= ps)))))) >>> + (unique >>> + (cond >>> + ((eq prefix t) >>> + (setq prefix fixed) >>> + t) >>> + ((stringp prefix) >>> + ;; `try-completion' is almost this, but it doe= sn't >>> + ;; handle `completion-ignore-case' the way we = want. >>> + (not (cl-some >> >> This gets a warning "the function =E2=80=98cl-some=E2=80=99 might not be= defined at >> runtime" because cl-lib is only loaded for macros, in minibuffer.el. > > Oops, sorry, just missed that. > >> So IDK - maybe it's better to use the alien cl-loop, or maybe to go >> back to seq-some, if no measurements show a cause for worry. > > Eh, I went back to seq-every-p for now. This shouldn't really cause a > performance problem, since it will usually just return nil quickly. > Also it's only run when the user hits TAB. > > (I'd rather not use cl-loop, this code is already hard enough to > understand :) ) > >>> + (lambda (comp) >>> + ;; PREFIX could complete to COMP, so i= t's not unique. >>> + (and (string-prefix-p prefix comp comp= letion-ignore-case) >>> + (not (=3D (length prefix) (length= comp))))) >>> + comps)))))) >> >> Just to check: in our limited test examples all elements of COMP match >> PREFIX already. And the beginning of >> 'completion-pcm--merge-completions' even 'error'-s if string-match >> comparison fails. >> >> So it's possible that we could omit this comparison from the lambda: >> >> (and (string-prefix-p prefix comp completion-ignore-case) >> >> But I suppose that would make it a bigger change, a bit riskier. > > Ah, you're right. I missed that we already have this property > guaranteed, but indeed we do. > > So thanks to that, the change can be further simplified. Removing my > cond refactoring, the patch is now just replacing > > (eq t (try-completion prefix comps)) > > with > > (seq-every-p > (lambda (comp) (=3D (length prefix) (length comp))) > comps) > > which is a nice simple change. I added some comments to clarify why > this is correct, too. And this followup patch makes it much clearer that this is correct, and also improves performance further. I think we should apply both. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Avoid-duplicating-strings-in-pcm-merge-completions.patch >From fcaab60e765961b2fffe064c1facf078bce1a101 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Wed, 20 Aug 2025 13:23:34 -0400 Subject: [PATCH] Avoid duplicating strings in pcm--merge-completions Make completion-pcm--merge-completions operate only on the text matched by the wildcards, instead of also the text in between the wildcards. This improves performance and simplifies the code by removing the need for the previous mutable variable "fixed". * lisp/minibuffer.el (completion-pcm--merge-completions): Operate only on text matched by wildcards. (bug#79265) --- lisp/minibuffer.el | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 36202566136..999d699c488 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4706,38 +4706,35 @@ completion-pcm--merge-completions (cond ((null (cdr strs)) (list (car strs))) (t - (let ((re (completion-pcm--pattern->regex pattern 'group)) + (let ((re (concat + (completion-pcm--pattern->regex pattern 'group) + ;; The implicit trailing `any' is greedy. + "\\([^z-a]*\\)")) (ccs ())) ;Chopped completions. - ;; First chop each string into the parts corresponding to each - ;; non-constant element of `pattern', using regexp-matching. + ;; First match each string against PATTERN as a regex and extract + ;; the text matched by each wildcard. (let ((case-fold-search completion-ignore-case)) (dolist (str strs) (unless (string-match re str) (error "Internal error: %s doesn't match %s" str re)) (let ((chopped ()) - (last 0) (i 1) next) - (while (setq next (match-end i)) - (push (substring str last next) chopped) - (setq last next) + (while (setq next (match-string i str)) + (push next chopped) (setq i (1+ i))) - ;; Add the text corresponding to the implicit trailing `any'. - (push (substring str last) chopped) (push (nreverse chopped) ccs)))) - ;; Then for each of those non-constant elements, extract the - ;; commonality between them. + ;; Then for each of those wildcards, extract the commonality between them. (let ((res ()) - (fixed "") ;; Accumulate each stretch of wildcards, and process them as a unit. (wildcards ())) ;; Make the implicit trailing `any' explicit. (dolist (elem (append pattern '(any))) (if (stringp elem) (progn - (setq fixed (concat fixed elem)) + (push elem res) (setq wildcards nil)) (let ((comps ())) (push elem wildcards) @@ -4748,18 +4745,13 @@ completion-pcm--merge-completions ;; different capitalizations in different parts. ;; In practice, it doesn't seem to make any difference. (setq ccs (nreverse ccs)) - ;; FIXED is a prefix of all of COMPS. Try to grow that prefix. - (let* ((prefix (try-completion fixed comps)) - (unique (or (and (eq prefix t) (setq prefix fixed)) + (let* ((prefix (try-completion "" comps)) + (unique (or (and (eq prefix t) (setq prefix "")) (and (stringp prefix) ;; If PREFIX is equal to all of COMPS, ;; then PREFIX is a unique completion. (seq-every-p - ;; PREFIX is still a prefix of all of - ;; COMPS, so if COMP is the same length, - ;; they're equal. - (lambda (comp) - (= (length prefix) (length comp))) + (lambda (comp) (= (length prefix) (length comp))) comps))))) ;; If there's only one completion, `elem' is not useful ;; any more: it can only match the empty string. @@ -4774,7 +4766,7 @@ completion-pcm--merge-completions ;; `prefix' only wants to include the fixed part before the ;; wildcard, not the result of growing that fixed part. (when (seq-some (lambda (elem) (eq elem 'prefix)) wildcards) - (setq prefix fixed)) + (setq prefix "")) (push prefix res) ;; Push all the wildcards in this stretch, to preserve `point' and ;; `star' wildcards before ELEM. @@ -4798,8 +4790,7 @@ completion-pcm--merge-completions (unless (equal suffix "") (push suffix res)))) ;; We pushed these wildcards on RES, so we're done with them. - (setq wildcards nil)) - (setq fixed ""))))) + (setq wildcards nil)))))) ;; We return it in reverse order. res))))) -- 2.43.7 --=-=-=-- From unknown Thu Sep 11 09:00:07 2025 MIME-Version: 1.0 X-Mailer: MIME-tools 5.505 (Entity 5.505) X-Loop: help-debbugs@gnu.org From: help-debbugs@gnu.org (GNU bug Tracking System) To: Spencer Baugh Subject: bug#79265: closed (Re: bug#79265: [PATCH] Treat point more consistently in PCM completion) Message-ID: References: <0a5e4c88-2b50-4a69-b1db-845a41f234cb@gutov.dev> X-Gnu-PR-Message: they-closed 79265 X-Gnu-PR-Package: emacs X-Gnu-PR-Keywords: patch Reply-To: 79265@debbugs.gnu.org Date: Thu, 21 Aug 2025 01:06:02 +0000 Content-Type: multipart/mixed; boundary="----------=_1755738362-4173-1" This is a multi-part message in MIME format... ------------=_1755738362-4173-1 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Your bug report #79265: [PATCH] Treat point more consistently in PCM completion which was filed against the emacs package, has been closed. The explanation is attached below, along with your original report. If you require more details, please reply to 79265@debbugs.gnu.org. --=20 79265: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D79265 GNU Bug Tracking System Contact help-debbugs@gnu.org with problems ------------=_1755738362-4173-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at 79265-done) by debbugs.gnu.org; 21 Aug 2025 01:05:51 +0000 Received: from localhost ([127.0.0.1]:58136 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uotkU-00014s-Ii for submit@debbugs.gnu.org; Wed, 20 Aug 2025 21:05:50 -0400 Received: from fhigh-a6-smtp.messagingengine.com ([103.168.172.157]:47807) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uotkR-00014a-FS for 79265-done@debbugs.gnu.org; Wed, 20 Aug 2025 21:05:48 -0400 Received: from phl-compute-01.internal (phl-compute-01.internal [10.202.2.41]) by mailfhigh.phl.internal (Postfix) with ESMTP id B222314000CC; Wed, 20 Aug 2025 21:05:41 -0400 (EDT) Received: from phl-mailfrontend-02 ([10.202.2.163]) by phl-compute-01.internal (MEProxy); Wed, 20 Aug 2025 21:05:41 -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=1755738341; x=1755824741; bh=S4x7XSV64o+N3d9qDvsB6v15g3vBnoGx4hYITFr8yB8=; b= 1i2Lz9N75LZlKzCviCeqJv3Q04u/j6ePpT6CWV+MVKuhp0Vrk7WSTwby3MB2njVB Pwp0K0owxucrTBS0uK76FxSy0WUVj7M3glL/fJt1/jJ9/6NFyqwMlyfTEBEr5SiJ jeF6Jyrjxb5skQ1QSTYb0ea5eogQxyjUjRhC77xwjDTkIP/hlFe0ER/4Imbufq0e M1dIsndYhcQvXl6sa8IsaNzPtoA1O1PNdxkYYthfo6tg3fOqqbLsa76XE2gk4wez Oona3uAUu0SS4OVeZFduUTfEbWMACVzbWH5E88AwORsPD60cHY0ZimlUWYNhzEga 8APQ7xgO919fT/lfIlF++Q== 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-sender:x-me-sender:x-sasl-enc; s=fm3; t=1755738341; x= 1755824741; bh=S4x7XSV64o+N3d9qDvsB6v15g3vBnoGx4hYITFr8yB8=; b=C jhepg3a/AqDLkGvPC5+INuQ2kvn9Gr6agTcb0ZQ9fJJirMHyokSqlqQLCCDbY7rp m3Xfd6fRWDxF9JNO5unLsNhqiS/+p0mzDiO7pb209jFFaLOeu4D64gvXFKd3saVA 925HEZ1WjyixBmUYLPwUIcgtr9GTGoeruXVjLIuoSydYe5TZs+1ZKiTZPBIJl7H6 4b9uj6XoUBzmQsTyP7eTW6SwBPkzq5m+VTIy2n7ledvl4sbGA/QhPgL7kO1y5D5R Mf9/ZfvPMaKv/pu19hi6ye8lHrNfWNul9RT87twtnDCUeT7GDK7hCVnklawhgI8w btoZy9pl6/SZ6iNNyJ02Q== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeeffedrtdefgdduheelkeejucetufdoteggodetrf dotffvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfurfetoffkrfgpnffqhgenuceu rghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujf gurhepkfffgggfuffvvehfhfgjtgfgsehtjeertddtvdejnecuhfhrohhmpeffmhhithhr hicuifhuthhovhcuoegumhhithhrhiesghhuthhovhdruggvvheqnecuggftrfgrthhtvg hrnhepteduleejgeehtefgheegjeekueehvdevieekueeftddvtdevfefhvdevgedujeeh necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepughmih htrhihsehguhhtohhvrdguvghvpdhnsggprhgtphhtthhopeefpdhmohguvgepshhmthhp ohhuthdprhgtphhtthhopehssggruhhghhesjhgrnhgvshhtrhgvvghtrdgtohhmpdhrtg hpthhtohepmhhonhhnihgvrhesihhrohdruhhmohhnthhrvggrlhdrtggrpdhrtghpthht ohepjeelvdeihedqughonhgvseguvggssghughhsrdhgnhhurdhorhhg X-ME-Proxy: Feedback-ID: i07de48aa:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Wed, 20 Aug 2025 21:05:40 -0400 (EDT) Message-ID: <0a5e4c88-2b50-4a69-b1db-845a41f234cb@gutov.dev> Date: Thu, 21 Aug 2025 04:05:38 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: bug#79265: [PATCH] Treat point more consistently in PCM completion To: Spencer Baugh References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Content-Language: en-US From: Dmitry Gutov In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Score: -0.7 (/) X-Debbugs-Envelope-To: 79265-done Cc: Stefan Monnier , 79265-done@debbugs.gnu.org X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -1.7 (-) On 20/08/2025 17:35, Spencer Baugh wrote: >> So IDK - maybe it's better to use the alien cl-loop, or maybe to go >> back to seq-some, if no measurements show a cause for worry. > > Eh, I went back to seq-every-p for now. This shouldn't really cause a > performance problem, since it will usually just return nil quickly. > Also it's only run when the user hits TAB. Sounds good. Its overhead it two dynamic dispatches, so probably fine for most uses. > (I'd rather not use cl-loop, this code is already hard enough to > understand :) ) All right then. (I like cl-loop ;-( ) >> Just to check: in our limited test examples all elements of COMP match >> PREFIX already. And the beginning of >> 'completion-pcm--merge-completions' even 'error'-s if string-match >> comparison fails. >> >> So it's possible that we could omit this comparison from the lambda: >> >> (and (string-prefix-p prefix comp completion-ignore-case) >> >> But I suppose that would make it a bigger change, a bit riskier. > > Ah, you're right. I missed that we already have this property > guaranteed, but indeed we do. > > So thanks to that, the change can be further simplified. Removing my > cond refactoring, the patch is now just replacing > > (eq t (try-completion prefix comps)) > > with > > (seq-every-p > (lambda (comp) (= (length prefix) (length comp))) > comps) > > which is a nice simple change. I added some comments to clarify why > this is correct, too. Thanks, pushed to master, this and the the following patch (nice optimization!). And with that, time to close. ------------=_1755738362-4173-1 Content-Type: message/rfc822 Content-Disposition: inline Content-Transfer-Encoding: 7bit Received: (at submit) by debbugs.gnu.org; 18 Aug 2025 18:53:37 +0000 Received: from localhost ([127.0.0.1]:51235 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uo4zA-0003Pf-T3 for submit@debbugs.gnu.org; Mon, 18 Aug 2025 14:53:37 -0400 Received: from lists.gnu.org ([2001:470:142::17]:55600) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uo4z7-0003PN-Ok for submit@debbugs.gnu.org; Mon, 18 Aug 2025 14:53:34 -0400 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 1uo4z2-0008IR-1f for bug-gnu-emacs@gnu.org; Mon, 18 Aug 2025 14:53:28 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1uo4z0-0002D1-3r for bug-gnu-emacs@gnu.org; Mon, 18 Aug 2025 14:53:27 -0400 From: Spencer Baugh To: bug-gnu-emacs@gnu.org Subject: [PATCH] Treat point more consistently in PCM completion X-Debbugs-Cc: Dmitry Gutov , Stefan Monnier Date: Mon, 18 Aug 2025 14:53:24 -0400 Message-ID: MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1755543204; bh=0UOMvyLshxG8yMIIAnetR9YdN2VH1648Mip8uiHAa2Y=; h=From:To:Subject:Date; b=FXe1YYyjTMlyCwzamuyxGl4bVyF5o1qIU/OTIkELXLNz//69g9a89dtVGaWw+5KWR Ovi5kQObQC4VaeJYJot3bfFn/JofZgG/2fEpGN0qg1qBX97tnIhzufN2obAqqD4r4S Sy88/gF26izeXL4xvCe0MBDYmUxNBCy5EnvFFuhIpgwvItCKRWoRoUYGFuOv5uaea0 yLgcoYNt59xCgoCGvxbZI/wmMFSB7rQtW1FsTZPRPG0TGSD5GLoPDocrPJGrTkYR+b +MtqK7Y3fa0/aVaf50a8VHN3VouEIJU3V0Kwsv76RGNfnzCBnCUtsLhhCDge8lz1NI 8UQ+nv+cKJQbg== Received-SPF: pass client-ip=64.215.233.18; envelope-from=sbaugh@janestreet.com; helo=mxout5.mail.janestreet.com X-Spam_score_int: -43 X-Spam_score: -4.4 X-Spam_bar: ---- X-Spam_report: (-4.4 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_MED=-2.3, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, RCVD_IN_VALIDITY_SAFE_BLOCKED=0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-Spam-Score: 0.9 (/) X-Debbugs-Envelope-To: submit X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -0.1 (/) --=-=-= Content-Type: text/plain Tags: patch Properly fix bug#38458, which is fundamentally an issue with completion-ignore-case, by checking if the completions are unique ignoring case. When the completions are unique, the normal code to delete a wildcard naturally causes point to be moved to the end of the minibuffer, which is the correct behavior. Now that the bug is fixed properly, remove a hack which previously was used to "fix" it, which made point behave inconsistently if it was in the middle of the minibuffer versus at the end of the minibuffer. * lisp/minibuffer.el (completion-pcm--merge-completions): Respect completion-ignore-case when checking for completion uniqueness. (completion-pcm--string->pattern) (completion-pcm--optimize-pattern): Allow point at the end of the pattern. * test/lisp/minibuffer-tests.el (completion-table-test-quoting) (completion-test--pcm-bug38458, completion-pcm-test-8): Update tests for more correct behavior. In GNU Emacs 30.1.90 (build 9, x86_64-pc-linux-gnu, X toolkit, cairo version 1.15.12, Xaw scroll bars) of 2025-08-14 built on igm-qws-u22796a Repository revision: 6adc26ffa74aedbd1cfa9a1ee72073ebccea2b96 Repository branch: emacs-30 Windowing system distributor 'The X.Org Foundation', version 11.0.12011000 System Description: Rocky Linux 8.10 (Green Obsidian) Configured using: 'configure --with-x-toolkit=lucid --without-gpm --without-gconf --without-selinux --without-imagemagick --with-modules --with-gif=no --with-cairo --with-rsvg --without-compress-install --with-tree-sitter --with-native-compilation=aot PKG_CONFIG_PATH=/usr/local/home/garnish/libtree-sitter/0.22.6-1/lib/pkgconfig/' --=-=-= Content-Type: text/patch Content-Disposition: attachment; filename=0001-Treat-point-more-consistently-in-PCM-completion.patch >From dbb5099a511f90e19761c0cf105c929e71a3c081 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Mon, 18 Aug 2025 14:50:44 -0400 Subject: [PATCH] Treat point more consistently in PCM completion Properly fix bug#38458, which is fundamentally an issue with completion-ignore-case, by checking if the completions are unique ignoring case. When the completions are unique, the normal code to delete a wildcard naturally causes point to be moved to the end of the minibuffer, which is the correct behavior. Now that the bug is fixed properly, remove a hack which previously was used to "fix" it, which made point behave inconsistently if it was in the middle of the minibuffer versus at the end of the minibuffer. * lisp/minibuffer.el (completion-pcm--merge-completions): Respect completion-ignore-case when checking for completion uniqueness. (completion-pcm--string->pattern) (completion-pcm--optimize-pattern): Allow point at the end of the pattern. * test/lisp/minibuffer-tests.el (completion-table-test-quoting) (completion-test--pcm-bug38458, completion-pcm-test-8): Update tests for more correct behavior. --- lisp/minibuffer.el | 15 +++++++-------- test/lisp/minibuffer-tests.el | 28 +++++++++++++++++++++------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 689cda7edab..046d792b27d 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4237,7 +4237,7 @@ completion-pcm--string->pattern "Split STRING into a pattern. A pattern is a list where each element is either a string or a symbol, see `completion-pcm--merge-completions'." - (if (and point (< point (length string))) + (if (and point (<= point (length string))) (let ((prefix (substring string 0 point)) (suffix (substring string point))) (append (completion-pcm--string->pattern prefix) @@ -4298,12 +4298,6 @@ completion-pcm--optimize-pattern (pcase p (`(,(or 'any 'any-delim) ,(or 'any 'point) . ,_) (setq p (cdr p))) - ;; This is not just a performance improvement: it turns a - ;; terminating `point' into an implicit `any', which affects - ;; the final position of point (because `point' gets turned - ;; into a non-greedy ".*?" regexp whereas we need it to be - ;; greedy when it's at the end, see bug#38458). - (`(point) (setq p nil)) ;Implicit terminating `any'. (_ (push (pop p) n)))) (nreverse n))) @@ -4757,7 +4751,12 @@ completion-pcm--merge-completions (let* ((prefix (try-completion fixed comps)) (unique (or (and (eq prefix t) (setq prefix fixed)) (and (stringp prefix) - (eq t (try-completion prefix comps)))))) + (eq t (try-completion prefix comps))) + (and completion-ignore-case + ;; They're all the same string, ignoring case. + (seq-every-p + (lambda (elem) (string-equal-ignore-case elem prefix)) + comps))))) ;; If there's only one completion, `elem' is not useful ;; any more: it can only match the empty string. ;; FIXME: in some cases, it may be necessary to turn an diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el index f9a26d17e58..59b72899e22 100644 --- a/test/lisp/minibuffer-tests.el +++ b/test/lisp/minibuffer-tests.el @@ -100,10 +100,10 @@ completion-table-test-quoting ;; Test that $$ in input is properly unquoted. ("data/m-cttq$$t" "data/minibuffer-test-cttq$$tion") ;; Test that env-vars are preserved. - ("lisp/c${CTTQ1}et/se-u" "lisp/c${CTTQ1}et/semantic-utest") - ("lisp/ced${CTTQ2}se-u" "lisp/ced${CTTQ2}semantic-utest") + ("lisp/c${CTTQ1}et/se-u-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") + ("lisp/ced${CTTQ2}se-u-c" "lisp/ced${CTTQ2}semantic-utest-c.test") ;; Test that env-vars don't prevent partial-completion. - ("lis/c${CTTQ1}/se-u" "lisp/c${CTTQ1}et/semantic-utest") + ("lis/c${CTTQ1}/se-u-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") )) (should (equal (completion-try-completion input #'completion--file-name-table @@ -118,11 +118,11 @@ completion-table-test-quoting ;; When an env var is in the completion bounds, try-completion ;; won't change letter case. ("lisp/c${CTTQ1}E" "lisp/c${CTTQ1}Et/") - ("lisp/ced${CTTQ2}SE-U" "lisp/ced${CTTQ2}SEmantic-utest") + ("lisp/ced${CTTQ2}SE-U-c" "lisp/ced${CTTQ2}SEmantic-utest-c.test") ;; If the env var is before the completion bounds, try-completion ;; *will* change letter case. - ("lisp/c${CTTQ1}et/SE-U" "lisp/c${CTTQ1}et/semantic-utest") - ("lis/c${CTTQ1}/SE-U" "lisp/c${CTTQ1}et/semantic-utest") + ("lisp/c${CTTQ1}et/SE-U-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") + ("lis/c${CTTQ1}/SE-U-c" "lisp/c${CTTQ1}et/semantic-utest-c.test") )) (should (equal (car (completion-try-completion input #'completion--file-name-table @@ -224,7 +224,11 @@ completion-test--pcm-bug38458 (completion-pcm--merge-try '("tes" point "ing") '("Testing" "testing") "" "")) - '("testing" . 4)))) + '("testing" . 7))) + (should (equal + (let ((completion-ignore-case t)) + (completion-pcm-try-completion "tes" '("Testing" "testing") nil 3)) + '("testing" . 7)))) (ert-deftest completion-pcm-test-1 () ;; Point is at end, this does not match anything @@ -318,6 +322,16 @@ completion-pcm-test-7 '(prefix any "bar" any) '("xbarxfoo" "ybaryfoo") "" "") '("bar" . 3)))) +(ert-deftest completion-pcm-test-8 () + ;; try-completion inserts the common prefix and suffix at point. + (should (equal (completion-pcm-try-completion + "r" '("fooxbar" "fooybar") nil 0) + '("foobar" . 3))) + ;; Even if point is at the end of the minibuffer. + (should (equal (completion-pcm-try-completion + "" '("fooxbar" "fooybar") nil 0) + '("foobar" . 3)))) + (ert-deftest completion-substring-test-1 () ;; One third of a match! (should (equal -- 2.43.7 --=-=-=-- ------------=_1755738362-4173-1-- From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 21 Aug 2025 18:59:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Spencer Baugh Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175580270023969 (code B ref 79265); Thu, 21 Aug 2025 18:59:02 +0000 Received: (at 79265) by debbugs.gnu.org; 21 Aug 2025 18:58:20 +0000 Received: from localhost ([127.0.0.1]:32994 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1upAUO-0006EX-1H for submit@debbugs.gnu.org; Thu, 21 Aug 2025 14:58:20 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:49094) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1upAUM-0006EC-BU for 79265@debbugs.gnu.org; Thu, 21 Aug 2025 14:58:19 -0400 Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 27F0F1000BC; Thu, 21 Aug 2025 14:58:12 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1755802691; bh=d8+FNrfk/ENexuU3K76uiFp1ljBVHUhKcZDIBmqOTeM=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=CG6buZYWNCfC7qrqHxjGdc7aCMcbHlJfJNSnoy9S/Rnw36XfU/Hh3Cpy4XEnKVI7Y KjBJNoA6ZD3NSEUprvTWKrmOthx7HaSszU8VxtCxFRNOVYrhpBf54IS4VwefkEmnK2 FJNHpU/VCAIfE4egAGBP8XKCOOnTMTS2dDApolT58aguZ8gGUBXhVu3dAWss4a26yP pzGM3txOZXlk/YG2/SohKo7XpwkffRTE6MMRpg+VJM96RrERXD9y68ZeTTs6oWZ6eW Zh+qCdq/37NBPbQ5qYzv3DQOdpSbQ05xU8epw7XqvZzsGgjVdoEC+hVtQutr7xWTYe F0HLD1lE3qMzA== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 46EB210002E; Thu, 21 Aug 2025 14:58:11 -0400 (EDT) Received: from asado (dyn.144-85-253-250.dsl.vtx.ch [144.85.253.250]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 3ABFA12047F; Thu, 21 Aug 2025 14:58:10 -0400 (EDT) From: Stefan Monnier In-Reply-To: Message-ID: References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Thu, 21 Aug 2025 14:58:07 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP AWL 0.009 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) [ Sorry for the delay reviewing this patch (and others, still waiting). I still have rather spotty access to my keyboard, tho things should slowly return to normal over the next two weeks. ] > From: Spencer Baugh > Date: Wed, 20 Aug 2025 13:23:34 -0400 > Subject: [PATCH] Avoid duplicating strings in pcm--merge-completions > > Make completion-pcm--merge-completions operate only on the text > matched by the wildcards, instead of also the text in between > the wildcards. This improves performance and simplifies the > code by removing the need for the previous mutable variable > "fixed". > > * lisp/minibuffer.el (completion-pcm--merge-completions): > Operate only on text matched by wildcards. (bug#79265) Could we have a regression test for this change? Or is it supposed to make no difference to behavior? > - (let ((re (completion-pcm--pattern->regex pattern 'group)) > + (let ((re (concat > + (completion-pcm--pattern->regex pattern 'group) > + ;; The implicit trailing `any' is greedy. > + "\\([^z-a]*\\)")) > (ccs ())) ;Chopped completions. The comment makes sense only if you know that `completion-pcm--pattern->regex` would use a non-greedy regexp for something like `any`. > - (while (setq next (match-end i)) > - (push (substring str last next) chopped) > - (setq last next) > + (while (setq next (match-string i str)) > + (push next chopped) IIUC this means that when `completion-ignore-case` is t we will keep the capitalization typed by the user rather than adjust it based on the completion candidates. E.g. (let ((completion-ignore-case t)) (completion-pcm--merge-completions '("ABC" "ABD") '("a"))) will complete "a" to "aB" rather than to "AB". See commit 681e0e7c02c4f8ff6d316c8c24001106cb847023 (which should have come with a regression test, obviously). Stefan From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 21 Aug 2025 19:30:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Stefan Monnier Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175580457729523 (code B ref 79265); Thu, 21 Aug 2025 19:30:01 +0000 Received: (at 79265) by debbugs.gnu.org; 21 Aug 2025 19:29:37 +0000 Received: from localhost ([127.0.0.1]:33059 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1upAyb-0007g0-MS for submit@debbugs.gnu.org; Thu, 21 Aug 2025 15:29:37 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:36877) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1upAyV-0007fc-O5 for 79265@debbugs.gnu.org; Thu, 21 Aug 2025 15:29:31 -0400 From: Spencer Baugh In-Reply-To: (Stefan Monnier's message of "Thu, 21 Aug 2025 14:58:07 -0400") References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Thu, 21 Aug 2025 15:29:21 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1755804561; bh=TLXEvW6Tz8CTJZ9VQR0B7IOQBM7yOqrjNMS7f1r7IYg=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=tFzgNN30Yf1WQ5h1/OamGAi6hZgSJtlTawYlKd5WrlXsJF2TTT18AIcP3htFYTnoK kO7WqnehDov5ECPM5ksDKmL1J03VlXtZhU1pjgJCKbJ/KWSxPB9ldeW56fSdwU8bj3 iIykY7B4OpZeL0zjD/PLtnktEQ2I4tiCUzm7MviqS1XrtvHC5Dz3c3QS44ZKZAJAag X28OUeLmGSEFWY1pTmKSaIiMGWsuBCpo50rYvf8bkTYvS6Flo41x+PnwRKk1tMcApE 6RlDFI8OsWxta9d6Fn59glzLw6bYznZZGiEXpX4m03l4VC3SLlzhYLhJBptjEiDOTq /ARSedWGlmKCg== X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Stefan Monnier writes: > [ Sorry for the delay reviewing this patch (and others, still waiting). > I still have rather spotty access to my keyboard, tho things should > slowly return to normal over the next two weeks. ] No worries, thanks for responding to this, to save me from my own buggy change :) >> From: Spencer Baugh >> Date: Wed, 20 Aug 2025 13:23:34 -0400 >> Subject: [PATCH] Avoid duplicating strings in pcm--merge-completions >> >> Make completion-pcm--merge-completions operate only on the text >> matched by the wildcards, instead of also the text in between >> the wildcards. This improves performance and simplifies the >> code by removing the need for the previous mutable variable >> "fixed". >> >> * lisp/minibuffer.el (completion-pcm--merge-completions): >> Operate only on text matched by wildcards. (bug#79265) > > Could we have a regression test for this change? > Or is it supposed to make no difference to behavior? It's supposed to make no difference to behavior... >> - (while (setq next (match-end i)) >> - (push (substring str last next) chopped) >> - (setq last next) >> + (while (setq next (match-string i str)) >> + (push next chopped) > > IIUC this means that when `completion-ignore-case` is t we will keep the > capitalization typed by the user rather than adjust it based on the > completion candidates. > > E.g. > > (let ((completion-ignore-case t)) > (completion-pcm--merge-completions '("ABC" "ABD") '("a"))) > > will complete "a" to "aB" rather than to "AB". > > See commit 681e0e7c02c4f8ff6d316c8c24001106cb847023 (which should have > come with a regression test, obviously). ...except, you're right, of course it does cause this issue. Thanks for catching this. I'll write a test for this and try to find a further change to the code which retains the improvement in clarity while still fixing this issue. It should work to just arbitrarily take the capitalization from the first completion in the list, I think, right? That also will make things more consistent, I think? From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 21 Aug 2025 20:45:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Stefan Monnier Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175580908910760 (code B ref 79265); Thu, 21 Aug 2025 20:45:02 +0000 Received: (at 79265) by debbugs.gnu.org; 21 Aug 2025 20:44:49 +0000 Received: from localhost ([127.0.0.1]:33193 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1upC9Q-0002nT-Pq for submit@debbugs.gnu.org; Thu, 21 Aug 2025 16:44:49 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:57149) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1upC9P-0002nE-2r for 79265@debbugs.gnu.org; Thu, 21 Aug 2025 16:44:47 -0400 From: Spencer Baugh In-Reply-To: (Spencer Baugh's message of "Thu, 21 Aug 2025 15:29:21 -0400") References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Thu, 21 Aug 2025 16:44:40 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1755809080; bh=8rRTb70T4AADPFnMMlgyrKeZtOWriU+FbBYvtXO0iKM=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=J/pyoRkcRUIRuTZFU2l1jBwOX7Qoc5qdK3o/CNgXvT4xlNNhl92hreMH+xAD/Xifc gc9kaT9CR7MHtI2Ml9MP9AyLsX1LjqZtuf3r25gi7hV/ka03Apfslrg9bn2YOG4E6j Al+Eu0yI8KXdcFlCL83h2An0Fs8I5PwHdc8Gjucvt+75vYFMxeI19uFJPNZGqauEf1 udzk4RbGhSWHYSZ94ju/MNg//Dkxe1sMDi5+0ZU+H9Dm+EC4nLwEE3Et9gd0WlfIKy CwJlCpwdE/5cBwb31eEl057spaVRo87vAkau9YNKvmDvPpwbBbGuUQ2hU32NRMsfvV zakL3az/FziyQ== X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) --=-=-= Content-Type: text/plain Spencer Baugh writes: > Stefan Monnier writes: >>> - (while (setq next (match-end i)) >>> - (push (substring str last next) chopped) >>> - (setq last next) >>> + (while (setq next (match-string i str)) >>> + (push next chopped) >> >> IIUC this means that when `completion-ignore-case` is t we will keep the >> capitalization typed by the user rather than adjust it based on the >> completion candidates. >> >> E.g. >> >> (let ((completion-ignore-case t)) >> (completion-pcm--merge-completions '("ABC" "ABD") '("a"))) >> >> will complete "a" to "aB" rather than to "AB". >> >> See commit 681e0e7c02c4f8ff6d316c8c24001106cb847023 (which should have >> come with a regression test, obviously). > > ...except, you're right, of course it does cause this issue. Thanks for catching this. > > I'll write a test for this and try to find a further change to the code > which retains the improvement in clarity while still fixing this issue. > > It should work to just arbitrarily take the capitalization from the > first completion in the list, I think, right? That also will make > things more consistent, I think? No, that's not correct, actually. I need to use try-completion to merge them to get nice behavior. So how about this? This mostly reverts my earlier patch, so it may be easier to review it relative to the old version before my earlier patch. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Use-capitalization-from-completions-in-pcm-try-again.patch >From 85a4b867646d563fc6e05edb2f6782073df77df8 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Thu, 21 Aug 2025 16:42:00 -0400 Subject: [PATCH] Use capitalization from completions in pcm-try, again In b511c38bba5354ff21c697e4d27279bf73e4d3cf I tried to simplify and optimize completion-pcm--merge-completions, but I broke earlier behavior added by 681e0e7c02c4f8ff6d316c8c24001106cb847023 which made completion-pcm-try-completion would change the case of text to match the completions. Support this behavior again and add a test checking this. * lisp/minibuffer.el (completion-pcm--pattern->regex): When GROUP is group-all, also group strings in PATTERN. (completion-pcm--merge-completions): Preserve case from parts of completions matching fixed strings in PATTERN. (bug#79265) * test/lisp/minibuffer-tests.el (completion-pcm-bug4219): Add. --- lisp/minibuffer.el | 55 +++++++++++++++++++++-------------- test/lisp/minibuffer-tests.el | 8 +++++ 2 files changed, 41 insertions(+), 22 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 467ef4591db..67f3f924ffd 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4226,7 +4226,11 @@ completion-pcm--pattern->regex (mapconcat (lambda (x) (cond - ((stringp x) (regexp-quote x)) + ((stringp x) + (let ((re (regexp-quote x))) + (if (eq group 'group-all) + (concat "\\(" re "\\)") + re))) (t (let ((re (if (eq x 'any-delim) (concat completion-pcm--delim-wild-regex "*?") @@ -4625,45 +4629,52 @@ completion-pcm--merge-completions (cond ((null (cdr strs)) (list (car strs))) (t - (let ((re (concat - (completion-pcm--pattern->regex pattern 'group) - ;; The implicit trailing `any' is greedy. - "\\([^z-a]*\\)")) + (let ((re (completion-pcm--pattern->regex pattern 'group-all)) (ccs ())) ;Chopped completions. - ;; First match each string against PATTERN as a regex and extract - ;; the text matched by each wildcard. + ;; First chop each string into the parts corresponding to each + ;; non-constant element of `pattern', using regexp-matching. (let ((case-fold-search completion-ignore-case)) (dolist (str strs) (unless (string-match re str) (error "Internal error: %s doesn't match %s" str re)) (let ((chopped ()) + (last 0) (i 1) next) - (while (setq next (match-string i str)) - (push next chopped) + (while (setq next (match-end i)) + (push (substring str last next) chopped) + (setq last next) (setq i (1+ i))) + ;; Add the text corresponding to the implicit trailing `any'. + (push (substring str last) chopped) (push (nreverse chopped) ccs)))) - ;; Then for each of those wildcards, extract the commonality between them. + ;; Then for each of those non-constant elements, extract the + ;; commonality between them. (let ((res ()) ;; Accumulate each stretch of wildcards, and process them as a unit. (wildcards ())) ;; Make the implicit trailing `any' explicit. (dolist (elem (append pattern '(any))) - (if (stringp elem) - (progn - (push elem res) - (setq wildcards nil)) - (let ((comps ())) + (let ((comps ())) + (dolist (cc (prog1 ccs (setq ccs nil))) + (push (car cc) comps) + (push (cdr cc) ccs)) + ;; Might improve the likelihood to avoid choosing + ;; different capitalizations in different parts. + ;; In practice, it doesn't seem to make any difference. + (setq ccs (nreverse ccs)) + (if (stringp elem) + (let ((shared (try-completion elem comps))) + ;; Use `try-completion' to merge the strings from each + ;; completion, which may differ in case. + (push (cond ((eq shared t) elem) + ((stringp shared) shared) + (t "")) + res) + (setq wildcards nil)) (push elem wildcards) - (dolist (cc (prog1 ccs (setq ccs nil))) - (push (car cc) comps) - (push (cdr cc) ccs)) - ;; Might improve the likelihood to avoid choosing - ;; different capitalizations in different parts. - ;; In practice, it doesn't seem to make any difference. - (setq ccs (nreverse ccs)) (let* ((prefix (try-completion "" comps)) (unique (or (and (eq prefix t) (setq prefix "")) (and (stringp prefix) diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el index c2c37e63012..e5868351619 100644 --- a/test/lisp/minibuffer-tests.el +++ b/test/lisp/minibuffer-tests.el @@ -332,6 +332,14 @@ completion-pcm-test-8 "" '("fooxbar" "fooybar") nil 0) '("foobar" . 3)))) +(ert-deftest completion-pcm-bug4219 () + ;; With `completion-ignore-case', try-completion should change the + ;; case of existing text when the completions have different casing. + (should (equal + (let ((completion-ignore-case t)) + (completion-pcm-try-completion "a" '("ABC" "ABD") nil 1)) + '("AB" . 2)))) + (ert-deftest completion-substring-test-1 () ;; One third of a match! (should (equal -- 2.43.7 --=-=-=-- From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 23 Aug 2025 07:00:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Spencer Baugh Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175593235413676 (code B ref 79265); Sat, 23 Aug 2025 07:00:02 +0000 Received: (at 79265) by debbugs.gnu.org; 23 Aug 2025 06:59:14 +0000 Received: from localhost ([127.0.0.1]:38513 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1upiDZ-0003YW-T5 for submit@debbugs.gnu.org; Sat, 23 Aug 2025 02:59:14 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:53634) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1upiDS-0003Xt-V5 for 79265@debbugs.gnu.org; Sat, 23 Aug 2025 02:59:12 -0400 Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 2D94D1000BC; Sat, 23 Aug 2025 02:59:01 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1755932340; bh=jip6b3o+nCang4A6PQ2bC8goYHN5Dp0tGRtSUFKxAJI=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=HIFzMTSHjJIun2nAWg4K8aK+Lkwb1vMu5IgmEz/H/Or9GYDBZRJHCa5UgF/QnNdEm 1uEmiuWJpkBrwHyKt8sAHmw6PDp6HSqkeT0viWTjboi/WLDu1LnOZKJXths0c4NXmk OyyfogCvItIJY6qlcSw5x+poEE81pMMaHQNwcsqzzolIpeoEa2H7Jq2tDStwiWE46o RoIVpxQuwRGHMmpLCkalKp6ADRDkcFcrI1v13EWPaxE0wRANhYnN+t4kJrxKINKJAV 8dltQ6x8B85y4vVmWsEuibxrequ+6ox84+f8L8EoJk/c1Y0Q/1/kGlT6+aQzZ/ppID hLwdU4LzhUMeQ== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 10E04100029; Sat, 23 Aug 2025 02:59:00 -0400 (EDT) Received: from asado (dyn.144-85-186-084.dsl.vtx.ch [144.85.186.84]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id BD6461203C3; Sat, 23 Aug 2025 02:58:58 -0400 (EDT) From: Stefan Monnier In-Reply-To: Message-ID: References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Sat, 23 Aug 2025 02:58:55 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP AWL 0.009 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > So how about this? This mostly reverts my earlier patch, so it may be > easier to review it relative to the old version before my earlier patch. Now I'm wondering what is the advantage of this new code compared to the old code. > * lisp/minibuffer.el (completion-pcm--pattern->regex): When > GROUP is group-all, also group strings in PATTERN. Why do we need that? > diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el > index c2c37e63012..e5868351619 100644 > --- a/test/lisp/minibuffer-tests.el > +++ b/test/lisp/minibuffer-tests.el > @@ -332,6 +332,14 @@ completion-pcm-test-8 > "" '("fooxbar" "fooybar") nil 0) > '("foobar" . 3)))) > > +(ert-deftest completion-pcm-bug4219 () > + ;; With `completion-ignore-case', try-completion should change the > + ;; case of existing text when the completions have different casing. > + (should (equal > + (let ((completion-ignore-case t)) > + (completion-pcm-try-completion "a" '("ABC" "ABD") nil 1)) > + '("AB" . 2)))) > + > (ert-deftest completion-substring-test-1 () > ;; One third of a match! > (should (equal Thanks, Stefan From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 23 Aug 2025 14:54:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Stefan Monnier Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175596080916249 (code B ref 79265); Sat, 23 Aug 2025 14:54:01 +0000 Received: (at 79265) by debbugs.gnu.org; 23 Aug 2025 14:53:29 +0000 Received: from localhost ([127.0.0.1]:41329 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uppcW-0004E0-A3 for submit@debbugs.gnu.org; Sat, 23 Aug 2025 10:53:28 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:50549) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uppcT-0004Db-QM for 79265@debbugs.gnu.org; Sat, 23 Aug 2025 10:53:26 -0400 From: Spencer Baugh In-Reply-To: (Stefan Monnier's message of "Sat, 23 Aug 2025 02:58:55 -0400") References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Sat, 23 Aug 2025 10:53:20 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1755960800; bh=8+SDhOw/2qfOJRPM4JKEfQzad6eeJH8DnoXLYSrr6rw=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=kKsQ4LJbsHC1ytoPnTUCQAufkfm1/vj4wKLVIEN959TwxRPO2OiZZOgJ/Hwc1iXob eRQpbOGoqRW80m5fMRKjWDY+5nwuZfRkglM+UxHB2Vz468rVIO1+NXe8V8g97d+HSh VLLleaRElFixLyilg/R3eAUxB2g5k4lmYimrlePNtjVZEqCgeh7s2vnUGmER7yf3Oh 6w2/hPuN0PvlFIg/1QTeKFT/QOYLEjGCURC0O7o9lhoI8nYEO6CmX/CvK32fz4m8DR RhgML96qR19VNM2DwLN+pzwj2aSacA7dH6EjqAMu5Jp8qJVkfLYAMoGDfGaSnj9TnF wXqHfHgFvh/Og== X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) Stefan Monnier writes: >> So how about this? This mostly reverts my earlier patch, so it may be >> easier to review it relative to the old version before my earlier patch. > > Now I'm wondering what is the advantage of this new code compared to the > old code. Two things: - It should have somewhat better performance because we're running try-completion on smaller strings and therefore doing less string comparison and work. - It gets rid of the mutable variable FIXED which is mutated across each iteration of the loop over the pattern. IMO this makes the code much easier to follow. Below is the diff from the original version, before my already-installed earlier patch; possibly this is easiest to review. >> * lisp/minibuffer.el (completion-pcm--pattern->regex): When >> GROUP is group-all, also group strings in PATTERN. > > Why do we need that? It makes it possible to extract the parts of the completions that correspond to each part of PATTERN, rather than just the parts corresponding to wildcards. CCS previously contained, for each completion, a list containing one element for each wildcard element of PATTERN. Now CCS contains, for each completion, a list containing one element for each element of PATTERN, both fixed strings and wildcards. (This lets us iterate over PATTERN and CCS with a more uniform loop - there could probably even be some further simplifications, to zip the two lists together one way or another) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index b5060614841..37bfbdc0765 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4187,7 +4207,11 @@ completion-pcm--pattern->regex (mapconcat (lambda (x) (cond - ((stringp x) (regexp-quote x)) + ((stringp x) + (let ((re (regexp-quote x))) + (if (eq group 'group-all) + (concat "\\(" re "\\)") + re))) (t (let ((re (if (eq x 'any-delim) (concat completion-pcm--delim-wild-regex "*?") @@ -4586,7 +4610,7 @@ completion-pcm--merge-completions (cond ((null (cdr strs)) (list (car strs))) (t - (let ((re (completion-pcm--pattern->regex pattern 'group)) + (let ((re (completion-pcm--pattern->regex pattern 'group-all)) (ccs ())) ;Chopped completions. ;; First chop each string into the parts corresponding to each @@ -4610,36 +4634,35 @@ completion-pcm--merge-completions ;; Then for each of those non-constant elements, extract the ;; commonality between them. (let ((res ()) - (fixed "") ;; Accumulate each stretch of wildcards, and process them as a unit. (wildcards ())) ;; Make the implicit trailing `any' explicit. (dolist (elem (append pattern '(any))) - (if (stringp elem) - (progn - (setq fixed (concat fixed elem)) - (setq wildcards nil)) - (let ((comps ())) + (let ((comps ())) + (dolist (cc (prog1 ccs (setq ccs nil))) + (push (car cc) comps) + (push (cdr cc) ccs)) + ;; Might improve the likelihood to avoid choosing + ;; different capitalizations in different parts. + ;; In practice, it doesn't seem to make any difference. + (setq ccs (nreverse ccs)) + (if (stringp elem) + (let ((shared (try-completion elem comps))) + ;; Use `try-completion' to merge the strings from each + ;; completion, which may differ in case. + (push (cond ((eq shared t) elem) + ((stringp shared) shared) + (t "")) + res) + (setq wildcards nil)) (push elem wildcards) - (dolist (cc (prog1 ccs (setq ccs nil))) - (push (car cc) comps) - (push (cdr cc) ccs)) - ;; Might improve the likelihood to avoid choosing - ;; different capitalizations in different parts. - ;; In practice, it doesn't seem to make any difference. - (setq ccs (nreverse ccs)) - ;; FIXED is a prefix of all of COMPS. Try to grow that prefix. - (let* ((prefix (try-completion fixed comps)) - (unique (or (and (eq prefix t) (setq prefix fixed)) + (let* ((prefix (try-completion "" comps)) + (unique (or (and (eq prefix t) (setq prefix "")) (and (stringp prefix) ;; If PREFIX is equal to all of COMPS, ;; then PREFIX is a unique completion. (seq-every-p - ;; PREFIX is still a prefix of all of - ;; COMPS, so if COMP is the same length, - ;; they're equal. - (lambda (comp) - (= (length prefix) (length comp))) + (lambda (comp) (= (length prefix) (length comp))) comps))))) ;; If there's only one completion, `elem' is not useful ;; any more: it can only match the empty string. @@ -4654,7 +4677,7 @@ completion-pcm--merge-completions ;; `prefix' only wants to include the fixed part before the ;; wildcard, not the result of growing that fixed part. (when (seq-some (lambda (elem) (eq elem 'prefix)) wildcards) - (setq prefix fixed)) + (setq prefix "")) (push prefix res) ;; Push all the wildcards in this stretch, to preserve `point' and ;; `star' wildcards before ELEM. @@ -4678,8 +4701,7 @@ completion-pcm--merge-completions (unless (equal suffix "") (push suffix res)))) ;; We pushed these wildcards on RES, so we're done with them. - (setq wildcards nil)) - (setq fixed ""))))) + (setq wildcards nil)))))) ;; We return it in reverse order. res))))) From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sun, 24 Aug 2025 07:28:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Spencer Baugh Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175602045125748 (code B ref 79265); Sun, 24 Aug 2025 07:28:02 +0000 Received: (at 79265) by debbugs.gnu.org; 24 Aug 2025 07:27:31 +0000 Received: from localhost ([127.0.0.1]:43249 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uq58U-0006hE-Lc for submit@debbugs.gnu.org; Sun, 24 Aug 2025 03:27:30 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:16691) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uq58R-0006gv-B0 for 79265@debbugs.gnu.org; Sun, 24 Aug 2025 03:27:28 -0400 Received: from pmg2.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id 9798B80836; Sun, 24 Aug 2025 03:27:21 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1756020436; bh=RzHf6FFfzWpSkUuDoyrBVXMWgAv8iUMiJMKa3gHvxoE=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=oGYQhb2xw5u+kLf1XjiknWovufw9mu4gvIaRDTxhLed3thVn5j2pzETQeNZoTTLRu tHKKgVLMXkvdKqB7ot8wigg+9Kph81p7TokXhjVxm8XY0LDM/3EWhGyijdlzW+pxhS f8gGFUam9qVLiYxx0DRzUAiIlpAj+g647Q+FLKz+Jij5+gJ6h9D8RGqCzheL5LxOUd uzUPtwe5bcz1cIs1zGPyXcp/DlcgFKygMj9kIwEZAdZZzQ7+b+tyZF51ExBTZYhHwK L5zmGpC4p7X4UtvLjHgjWPN57DOU08Ox2KeU42qmovrKv2j187sVWLViLXrU/AG9Q9 R/33VBP2roEbA== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg2.iro.umontreal.ca (Proxmox) with ESMTP id A45BF80916; Sun, 24 Aug 2025 03:27:16 -0400 (EDT) Received: from asado (dyn.144-85-186-084.dsl.vtx.ch [144.85.186.84]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 5EBCC1204ED; Sun, 24 Aug 2025 03:27:15 -0400 (EDT) From: Stefan Monnier In-Reply-To: Message-ID: References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Sun, 24 Aug 2025 03:27:12 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP AWL -0.025 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) > Two things: > > - It should have somewhat better performance because we're running > try-completion on smaller strings and therefore doing less string > comparison and work. Do we have concrete evidence to believe this? > CCS previously contained, for each completion, a list containing one > element for each wildcard element of PATTERN. > > Now CCS contains, for each completion, a list containing one element for > each element of PATTERN, both fixed strings and wildcards. Exactly: that means CCS is more costly to build and AFAICT it also implies correspondingly more calls to `try-completion`. I don't find it obvious that the new code will be more efficient. Barring any clear evidence that one of the two versions is noticeably more efficient than the other, I'd recommend we stick to the code we happen to have. >From where I stand, my impression is that neither version is terribly better nor terribly worse than the other, in terms of clarity, simplicity, and efficiency. Stefan PS: BTW, there's currently a limit of 255 subgroups in regexps, which for `flex` currently implies a limit of 254 chars and with your patch that limit would be halved. From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 25 Aug 2025 18:41:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Stefan Monnier Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175614721123666 (code B ref 79265); Mon, 25 Aug 2025 18:41:02 +0000 Received: (at 79265) by debbugs.gnu.org; 25 Aug 2025 18:40:11 +0000 Received: from localhost ([127.0.0.1]:51202 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1uqc70-00068r-S6 for submit@debbugs.gnu.org; Mon, 25 Aug 2025 14:40:11 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:58115) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1uqc6t-00067u-DV for 79265@debbugs.gnu.org; Mon, 25 Aug 2025 14:40:05 -0400 From: Spencer Baugh In-Reply-To: (Stefan Monnier's message of "Sun, 24 Aug 2025 03:27:12 -0400") References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Mon, 25 Aug 2025 14:39:55 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1756147196; bh=u0K7WG7zO7UbqE89re6xZ6kvkD/2WrqZbNVqcAk18Ds=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=ufLgIOPADJwaGw0276BzLciGk4KXlr4IXyKpMoqNMFPH1PrE/V5NZp8aGvODnIIbR uDDIZ++qgEhXcso2Xo9Utg6g5oIc7SeAI1naE67waI4mX6H070XX3w94sUMg7NpHtk RRNCzq2qUjn+njVA+glGr0Y0DawAiodZdWPrLOqRpoBSLzsLyLGngIsjP69IcUJiRG AW6PVAYfXwRzAzfSWUrsVr7CyXVCrkF5S99nZEkbvn+rfJQTl6WrybZ/JDNxFmxrAo MMOWJ0TYPyp/zGu0bTFj/96grpfETEPwg/DHY3ru4pp+W7d05A56BE3YBzSvSi5iIA s76sNt3yea65g== X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) --=-=-= Content-Type: text/plain Stefan Monnier writes: >> Two things: >> >> - It should have somewhat better performance because we're running >> try-completion on smaller strings and therefore doing less string >> comparison and work. > > Do we have concrete evidence to believe this? No evidence. So nevermind the performance argument, I suppose. >> CCS previously contained, for each completion, a list containing one >> element for each wildcard element of PATTERN. >> >> Now CCS contains, for each completion, a list containing one element for >> each element of PATTERN, both fixed strings and wildcards. > > Exactly: that means CCS is more costly to build and AFAICT it also > implies correspondingly more calls to `try-completion`. > > I don't find it obvious that the new code will be more efficient. > Barring any clear evidence that one of the two versions is noticeably > more efficient than the other, I'd recommend we stick to the code we > happen to have. > > From where I stand, my impression is that neither version is terribly > better nor terribly worse than the other, in terms of clarity, > simplicity, and efficiency. That's fair, that patch alone is not worth much. Attached is a patch (which applies on master) which does most of the rest of the refactoring that I wanted to do; it makes the dolist over the pattern into a simple mapcan over the pattern, with no state preserved between elements of the pattern. IMO, this is much easier to follow (and I think it would make the various bugs I've fixed in the last couple years in merge-completions, easier to catch). What do you think? > PS: BTW, there's currently a limit of 255 subgroups in regexps, which > for `flex` currently implies a limit of 254 chars and with your patch > that limit would be halved. Oh, I didn't realize that. I could definitely lift that limit on flex with my design, though, so that it's not limited to 254 chars. We can just do repeated string-searches instead for each of the fixed elements, which won't have this limitation. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Refactor-completion-pcm-merge-completions.patch >From 54857fe2fb0ed033afcba231f920f8f7fa185333 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Mon, 25 Aug 2025 14:27:19 -0400 Subject: [PATCH] Refactor completion-pcm--merge-completions Further simplify completion-pcm--merge-completions. Also fix a bug I introduced in b511c38bba5354ff21c697e4d27279bf73e4d3cf which made it no longer change the case of text to match the completions. * lisp/minibuffer.el (completion-pcm--pattern->regex): When GROUP is group-all, also group strings in PATTERN. (completion-pcm--merge-completions): Preserve case from parts of completions matching fixed strings in PATTERN. (bug#79265) * test/lisp/minibuffer-tests.el (completion-pcm-bug4219): Add. --- lisp/minibuffer.el | 165 +++++++++++++++++----------------- test/lisp/minibuffer-tests.el | 8 ++ 2 files changed, 90 insertions(+), 83 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 0221af5426e..462280233f1 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4249,7 +4249,11 @@ completion-pcm--pattern->regex (mapconcat (lambda (x) (cond - ((stringp x) (regexp-quote x)) + ((stringp x) + (let ((re (regexp-quote x))) + (if (eq group 'group-all) + (concat "\\(" re "\\)") + re))) (t (let ((re (if (eq x 'any-delim) (concat completion-pcm--delim-wild-regex "*?") @@ -4622,6 +4626,19 @@ completion--common-suffix "Return the common suffix of the strings STRS." (nreverse (try-completion "" (mapcar #'reverse strs)))) +(defun completion-pcm--group-pattern (pattern) + "Group together adjacent wildcards in PATTERN." + (let (ret) + (dolist (elem pattern) + (cond + ((or (stringp elem) (eq elem 'any-delim)) + (push elem ret)) + ((consp (car-safe ret)) + (setf (car ret) (append (car ret) (list elem)))) + (t + (push (list elem) ret)))) + (nreverse ret))) + (defun completion-pcm--merge-completions (strs pattern) "Extract the commonality in STRS, with the help of PATTERN. PATTERN can contain strings and symbols chosen among `star', `any', `point', @@ -4648,93 +4665,75 @@ completion-pcm--merge-completions (cond ((null (cdr strs)) (list (car strs))) (t - (let ((re (concat - (completion-pcm--pattern->regex pattern 'group) - ;; The implicit trailing `any' is greedy. - "\\([^z-a]*\\)")) - (ccs ())) ;Chopped completions. - + (let* ((pattern (completion-pcm--group-pattern (append pattern '(any)))) + (re (concat + ;; Force matching the entire string. + (completion-pcm--pattern->regex pattern 'group-all) (rx eos))) + (pattern-and-comps (mapcar (lambda (elem) (cons elem nil)) pattern))) ;; First match each string against PATTERN as a regex and extract ;; the text matched by each wildcard. (let ((case-fold-search completion-ignore-case)) (dolist (str strs) (unless (string-match re str) (error "Internal error: %s doesn't match %s" str re)) - (let ((chopped ()) - (i 1) - next) - (while (setq next (match-string i str)) - (push next chopped) - (setq i (1+ i))) - (push (nreverse chopped) ccs)))) - - ;; Then for each of those wildcards, extract the commonality between them. - (let ((res ()) - ;; Accumulate each stretch of wildcards, and process them as a unit. - (wildcards ())) - ;; Make the implicit trailing `any' explicit. - (dolist (elem (append pattern '(any))) - (if (stringp elem) - (progn - (push elem res) - (setq wildcards nil)) - (let ((comps ())) - (push elem wildcards) - (dolist (cc (prog1 ccs (setq ccs nil))) - (push (car cc) comps) - (push (cdr cc) ccs)) - ;; Might improve the likelihood to avoid choosing - ;; different capitalizations in different parts. - ;; In practice, it doesn't seem to make any difference. - (setq ccs (nreverse ccs)) - (let* ((prefix (try-completion "" comps)) - (unique (or (and (eq prefix t) (setq prefix "")) - (and (stringp prefix) - ;; If PREFIX is equal to all of COMPS, - ;; then PREFIX is a unique completion. - (seq-every-p - (lambda (comp) (= (length prefix) (length comp))) - comps))))) - ;; If there's only one completion, `elem' is not useful - ;; any more: it can only match the empty string. - ;; FIXME: in some cases, it may be necessary to turn an - ;; `any' into a `star' because the surrounding context has - ;; changed such that string->pattern wouldn't add an `any' - ;; here any more. - (if unique - ;; If the common prefix is unique, it also is a common - ;; suffix, so we should add it for `prefix' elements. - (push prefix res) - ;; `prefix' only wants to include the fixed part before the - ;; wildcard, not the result of growing that fixed part. - (when (seq-some (lambda (elem) (eq elem 'prefix)) wildcards) - (setq prefix "")) - (push prefix res) - ;; Push all the wildcards in this stretch, to preserve `point' and - ;; `star' wildcards before ELEM. - (setq res (append wildcards res)) - ;; Extract common suffix additionally to common prefix. - ;; Don't do it for `any' since it could lead to a merged - ;; completion that doesn't itself match the candidates. - (when (and (seq-some (lambda (elem) (memq elem '(star point prefix))) wildcards) - ;; If prefix is one of the completions, there's no - ;; suffix left to find. - (not (assoc-string prefix comps t))) - (let ((suffix - (completion--common-suffix - (if (zerop (length prefix)) comps - ;; Ignore the chars in the common prefix, so we - ;; don't merge '("abc" "abbc") as "ab*bc". - (let ((skip (length prefix))) - (mapcar (lambda (str) (substring str skip)) - comps)))))) - (cl-assert (stringp suffix)) - (unless (equal suffix "") - (push suffix res)))) - ;; We pushed these wildcards on RES, so we're done with them. - (setq wildcards nil)))))) - ;; We return it in reverse order. - res))))) + (seq-do-indexed + (lambda (elem i) + (push (match-string (1+ i) str) (cdr elem))) + pattern-and-comps))) + + ;; Then for each of those non-constant elements, extract the + ;; commonality between them. + (mapcan + (lambda (cons) + (let ((elem (car cons)) + (comps (cdr cons))) + (if (stringp elem) + (let ((shared (try-completion elem comps))) + ;; Use `try-completion' to merge the strings from each + ;; completion, which may differ in case. + (cond ((eq shared t) (list elem)) + ((stringp shared) (list shared)))) + (let* ((wildcards elem) + (prefix (try-completion "" comps)) + (unique (or (and (eq prefix t) (setq prefix "")) + (and (stringp prefix) + ;; If PREFIX is equal to all of COMPS, + ;; then PREFIX is a unique completion. + (seq-every-p + (lambda (comp) (= (length prefix) (length comp))) + comps))))) + ;; If there's only one completion, `elem' is not useful + ;; any more: it can only match the empty string. + ;; FIXME: in some cases, it may be necessary to turn an + ;; `any' into a `star' because the surrounding context has + ;; changed such that string->pattern wouldn't add an `any' + ;; here any more. + (if unique + ;; If the common prefix is unique, it also is a common + ;; suffix, so we should add it for `prefix' elements. + (list prefix) + (delq nil + (append + ;; `prefix' only wants to include the fixed part before the + ;; wildcard, not the result of growing that fixed part. + (unless (seq-some (lambda (elem) (eq elem 'prefix)) wildcards) + (list prefix)) + wildcards + ;; Extract common suffix additionally to common prefix. + ;; Don't do it for `any' since it could lead to a merged + ;; completion that doesn't itself match the candidates. + (when (and (seq-some (lambda (elem) (memq elem '(star point prefix))) wildcards) + ;; If prefix is one of the completions, there's no + ;; suffix left to find. + (not (assoc-string prefix comps t))) + (list (completion--common-suffix + (if (zerop (length prefix)) comps + ;; Ignore the chars in the common prefix, so we + ;; don't merge '("abc" "abbc") as "ab*bc". + (let ((skip (length prefix))) + (mapcar (lambda (str) (substring str skip)) + comps))))))))))))) + pattern-and-comps))))) (defun completion-pcm--pattern->string (pattern) (mapconcat (lambda (x) (cond @@ -4774,7 +4773,7 @@ completion-pcm--merge-try (equal (completion-pcm--pattern->string pattern) (car all))) t) (t - (let* ((mergedpat (completion-pcm--merge-completions all pattern)) + (let* ((mergedpat (nreverse (completion-pcm--merge-completions all pattern))) ;; `mergedpat' is in reverse order. Place new point (by ;; order of preference) either at the old point, or at ;; the last place where there's something to choose, or diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el index 123ca849f3b..5b2e6f96af0 100644 --- a/test/lisp/minibuffer-tests.el +++ b/test/lisp/minibuffer-tests.el @@ -329,6 +329,14 @@ completion-pcm-test-8 "" '("fooxbar" "fooybar") nil 0) '("foobar" . 3)))) +(ert-deftest completion-pcm-bug4219 () + ;; With `completion-ignore-case', try-completion should change the + ;; case of existing text when the completions have different casing. + (should (equal + (let ((completion-ignore-case t)) + (completion-pcm-try-completion "a" '("ABC" "ABD") nil 1)) + '("AB" . 2)))) + (ert-deftest completion-substring-test-1 () ;; One third of a match! (should (equal -- 2.43.7 --=-=-=-- From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion, [PATCH] Refactor completion-pcm--merge-completions Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Thu, 28 Aug 2025 18:43:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Stefan Monnier Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175640658016858 (code B ref 79265); Thu, 28 Aug 2025 18:43:02 +0000 Received: (at 79265) by debbugs.gnu.org; 28 Aug 2025 18:43:00 +0000 Received: from localhost ([127.0.0.1]:40009 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1urhaM-0004Nc-9e for submit@debbugs.gnu.org; Thu, 28 Aug 2025 14:42:59 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:48603) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1urhaG-0004Lq-PE for 79265@debbugs.gnu.org; Thu, 28 Aug 2025 14:42:55 -0400 From: Spencer Baugh In-Reply-To: (Spencer Baugh's message of "Mon, 25 Aug 2025 14:39:55 -0400") References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Thu, 28 Aug 2025 14:42:46 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1756406566; bh=jVhhPkr3NcNXjaYD6ReaqICxntp/E1nqyuSVM0IKSdc=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=ObGhrIwlbrurzosdZZNuz3Tym9iUrffKzaT+OjuueNXSGqA2wAtoNwBpbtx4mMO2q T8qMeHs3Qy0L7/9G6wQBFsJGe7CHOlkXDYSfKB0GGPe3X7BgkeEetbzmiRo7N+18Du jQ2dCgzRqKukf8ThscxIMowBC6P/SnMMAqI/K7OYepfvNOAxkkpYoFn2jqJ8mWfiAn 3qwZKsiYPVOI0wDlQ+zUxRrv5xgvgb/LtzeVAUyqfCnusp5MvO5cRS6RioPZWtFZJe WN6Jomwsbgq+COJikrxbteBhXel2OChTbymlP2H5Ym+XaWamAvVWvoGfjcSvnQXzaZ wxkDjKpYg/33A== X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) --=-=-= Content-Type: text/plain Spencer Baugh writes: > Stefan Monnier writes: > >>> Two things: >>> >>> - It should have somewhat better performance because we're running >>> try-completion on smaller strings and therefore doing less string >>> comparison and work. >> >> Do we have concrete evidence to believe this? > > No evidence. So nevermind the performance argument, I suppose. > >>> CCS previously contained, for each completion, a list containing one >>> element for each wildcard element of PATTERN. >>> >>> Now CCS contains, for each completion, a list containing one element for >>> each element of PATTERN, both fixed strings and wildcards. >> >> Exactly: that means CCS is more costly to build and AFAICT it also >> implies correspondingly more calls to `try-completion`. >> >> I don't find it obvious that the new code will be more efficient. >> Barring any clear evidence that one of the two versions is noticeably >> more efficient than the other, I'd recommend we stick to the code we >> happen to have. >> >> From where I stand, my impression is that neither version is terribly >> better nor terribly worse than the other, in terms of clarity, >> simplicity, and efficiency. > > That's fair, that patch alone is not worth much. > > Attached is a patch (which applies on master) which does most of the > rest of the refactoring that I wanted to do; it makes the dolist over > the pattern into a simple mapcan over the pattern, with no state > preserved between elements of the pattern. IMO, this is much easier to > follow (and I think it would make the various bugs I've fixed in the > last couple years in merge-completions, easier to catch). What do you > think? This patch has a small bug with any-delim, here's a fixed version, now with a test for that case. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Refactor-completion-pcm-merge-completions.patch >From 8f33ad72f43f40c6613cac5d1466747c7df796c2 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Mon, 25 Aug 2025 14:27:19 -0400 Subject: [PATCH] Refactor completion-pcm--merge-completions Further simplify completion-pcm--merge-completions. Also fix a bug I introduced in b511c38bba5354ff21c697e4d27279bf73e4d3cf which made it no longer change the case of text to match the completions. * lisp/minibuffer.el (completion-pcm--pattern->regex): When GROUP is group-all, also group strings in PATTERN. (completion-pcm--merge-completions): Preserve case from parts of completions matching fixed strings in PATTERN. (bug#79265) * test/lisp/minibuffer-tests.el (completion-pcm-bug4219): Add. --- lisp/minibuffer.el | 165 +++++++++++++++++----------------- test/lisp/minibuffer-tests.el | 15 ++++ 2 files changed, 97 insertions(+), 83 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 0221af5426e..7921d3ddf84 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4249,7 +4249,11 @@ completion-pcm--pattern->regex (mapconcat (lambda (x) (cond - ((stringp x) (regexp-quote x)) + ((stringp x) + (let ((re (regexp-quote x))) + (if (eq group 'group-all) + (concat "\\(" re "\\)") + re))) (t (let ((re (if (eq x 'any-delim) (concat completion-pcm--delim-wild-regex "*?") @@ -4622,6 +4626,19 @@ completion--common-suffix "Return the common suffix of the strings STRS." (nreverse (try-completion "" (mapcar #'reverse strs)))) +(defun completion-pcm--group-pattern (pattern) + "Group together adjacent wildcards in PATTERN." + (let (ret) + (dolist (elem pattern) + (cond + ((or (stringp elem) (eq elem 'any-delim)) + (push elem ret)) + ((consp (car-safe ret)) + (setf (car ret) (append (car ret) (list elem)))) + (t + (push (list elem) ret)))) + (nreverse ret))) + (defun completion-pcm--merge-completions (strs pattern) "Extract the commonality in STRS, with the help of PATTERN. PATTERN can contain strings and symbols chosen among `star', `any', `point', @@ -4648,93 +4665,75 @@ completion-pcm--merge-completions (cond ((null (cdr strs)) (list (car strs))) (t - (let ((re (concat - (completion-pcm--pattern->regex pattern 'group) - ;; The implicit trailing `any' is greedy. - "\\([^z-a]*\\)")) - (ccs ())) ;Chopped completions. - + (let* ((pattern (completion-pcm--group-pattern (append pattern '(any)))) + (re (concat + ;; Force matching the entire string. + (completion-pcm--pattern->regex pattern 'group-all) (rx eos))) + (pattern-and-comps (mapcar (lambda (elem) (cons elem nil)) pattern))) ;; First match each string against PATTERN as a regex and extract ;; the text matched by each wildcard. (let ((case-fold-search completion-ignore-case)) (dolist (str strs) (unless (string-match re str) (error "Internal error: %s doesn't match %s" str re)) - (let ((chopped ()) - (i 1) - next) - (while (setq next (match-string i str)) - (push next chopped) - (setq i (1+ i))) - (push (nreverse chopped) ccs)))) - - ;; Then for each of those wildcards, extract the commonality between them. - (let ((res ()) - ;; Accumulate each stretch of wildcards, and process them as a unit. - (wildcards ())) - ;; Make the implicit trailing `any' explicit. - (dolist (elem (append pattern '(any))) - (if (stringp elem) - (progn - (push elem res) - (setq wildcards nil)) - (let ((comps ())) - (push elem wildcards) - (dolist (cc (prog1 ccs (setq ccs nil))) - (push (car cc) comps) - (push (cdr cc) ccs)) - ;; Might improve the likelihood to avoid choosing - ;; different capitalizations in different parts. - ;; In practice, it doesn't seem to make any difference. - (setq ccs (nreverse ccs)) - (let* ((prefix (try-completion "" comps)) - (unique (or (and (eq prefix t) (setq prefix "")) - (and (stringp prefix) - ;; If PREFIX is equal to all of COMPS, - ;; then PREFIX is a unique completion. - (seq-every-p - (lambda (comp) (= (length prefix) (length comp))) - comps))))) - ;; If there's only one completion, `elem' is not useful - ;; any more: it can only match the empty string. - ;; FIXME: in some cases, it may be necessary to turn an - ;; `any' into a `star' because the surrounding context has - ;; changed such that string->pattern wouldn't add an `any' - ;; here any more. - (if unique - ;; If the common prefix is unique, it also is a common - ;; suffix, so we should add it for `prefix' elements. - (push prefix res) - ;; `prefix' only wants to include the fixed part before the - ;; wildcard, not the result of growing that fixed part. - (when (seq-some (lambda (elem) (eq elem 'prefix)) wildcards) - (setq prefix "")) - (push prefix res) - ;; Push all the wildcards in this stretch, to preserve `point' and - ;; `star' wildcards before ELEM. - (setq res (append wildcards res)) - ;; Extract common suffix additionally to common prefix. - ;; Don't do it for `any' since it could lead to a merged - ;; completion that doesn't itself match the candidates. - (when (and (seq-some (lambda (elem) (memq elem '(star point prefix))) wildcards) - ;; If prefix is one of the completions, there's no - ;; suffix left to find. - (not (assoc-string prefix comps t))) - (let ((suffix - (completion--common-suffix - (if (zerop (length prefix)) comps - ;; Ignore the chars in the common prefix, so we - ;; don't merge '("abc" "abbc") as "ab*bc". - (let ((skip (length prefix))) - (mapcar (lambda (str) (substring str skip)) - comps)))))) - (cl-assert (stringp suffix)) - (unless (equal suffix "") - (push suffix res)))) - ;; We pushed these wildcards on RES, so we're done with them. - (setq wildcards nil)))))) - ;; We return it in reverse order. - res))))) + (seq-do-indexed + (lambda (elem i) + (push (match-string (1+ i) str) (cdr elem))) + pattern-and-comps))) + + ;; Then for each of those non-constant elements, extract the + ;; commonality between them. + (mapcan + (lambda (cons) + (let ((elem (car cons)) + (comps (cdr cons))) + (if (stringp elem) + (let ((shared (try-completion elem comps))) + ;; Use `try-completion' to merge the strings from each + ;; completion, which may differ in case. + (cond ((eq shared t) (list elem)) + ((stringp shared) (list shared)))) + (let* ((wildcards (ensure-list elem)) + (prefix (try-completion "" comps)) + (unique (or (and (eq prefix t) (setq prefix "")) + (and (stringp prefix) + ;; If PREFIX is equal to all of COMPS, + ;; then PREFIX is a unique completion. + (seq-every-p + (lambda (comp) (= (length prefix) (length comp))) + comps))))) + ;; If there's only one completion, `elem' is not useful + ;; any more: it can only match the empty string. + ;; FIXME: in some cases, it may be necessary to turn an + ;; `any' into a `star' because the surrounding context has + ;; changed such that string->pattern wouldn't add an `any' + ;; here any more. + (if unique + ;; If the common prefix is unique, it also is a common + ;; suffix, so we should add it for `prefix' elements. + (list prefix) + (delq nil + (append + ;; `prefix' only wants to include the fixed part before the + ;; wildcard, not the result of growing that fixed part. + (unless (seq-some (lambda (elem) (eq elem 'prefix)) wildcards) + (list prefix)) + wildcards + ;; Extract common suffix additionally to common prefix. + ;; Don't do it for `any' since it could lead to a merged + ;; completion that doesn't itself match the candidates. + (when (and (seq-some (lambda (elem) (memq elem '(star point prefix))) wildcards) + ;; If prefix is one of the completions, there's no + ;; suffix left to find. + (not (assoc-string prefix comps t))) + (list (completion--common-suffix + (if (zerop (length prefix)) comps + ;; Ignore the chars in the common prefix, so we + ;; don't merge '("abc" "abbc") as "ab*bc". + (let ((skip (length prefix))) + (mapcar (lambda (str) (substring str skip)) + comps))))))))))))) + pattern-and-comps))))) (defun completion-pcm--pattern->string (pattern) (mapconcat (lambda (x) (cond @@ -4774,7 +4773,7 @@ completion-pcm--merge-try (equal (completion-pcm--pattern->string pattern) (car all))) t) (t - (let* ((mergedpat (completion-pcm--merge-completions all pattern)) + (let* ((mergedpat (nreverse (completion-pcm--merge-completions all pattern))) ;; `mergedpat' is in reverse order. Place new point (by ;; order of preference) either at the old point, or at ;; the last place where there's something to choose, or diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el index 123ca849f3b..bede9713aa1 100644 --- a/test/lisp/minibuffer-tests.el +++ b/test/lisp/minibuffer-tests.el @@ -329,6 +329,21 @@ completion-pcm-test-8 "" '("fooxbar" "fooybar") nil 0) '("foobar" . 3)))) +(ert-deftest completion-pcm-test-anydelim () + ;; After each delimiter is a special wildcard which matches any + ;; sequence of delimiters. + (should (equal (completion-pcm-try-completion + "-x" '("-_.x" "-__x") nil 2) + '("-_x" . 3)))) + +(ert-deftest completion-pcm-bug4219 () + ;; With `completion-ignore-case', try-completion should change the + ;; case of existing text when the completions have different casing. + (should (equal + (let ((completion-ignore-case t)) + (completion-pcm-try-completion "a" '("ABC" "ABD") nil 1)) + '("AB" . 2)))) + (ert-deftest completion-substring-test-1 () ;; One third of a match! (should (equal -- 2.43.7 --=-=-=-- From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion, [PATCH] Refactor completion-pcm--merge-completions Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 01 Sep 2025 05:42:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Spencer Baugh Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175670530910921 (code B ref 79265); Mon, 01 Sep 2025 05:42:02 +0000 Received: (at 79265) by debbugs.gnu.org; 1 Sep 2025 05:41:49 +0000 Received: from localhost ([127.0.0.1]:56191 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1usxIb-0002q5-6X for submit@debbugs.gnu.org; Mon, 01 Sep 2025 01:41:49 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:35221) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1usxIY-0002pr-Ph for 79265@debbugs.gnu.org; Mon, 01 Sep 2025 01:41:48 -0400 Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 2632910025E; Mon, 1 Sep 2025 01:41:40 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1756705298; bh=chZrYOPAlAYazPhkrUWypSfwYL+POEHHPd8ls9AYsjU=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=bEGw66MPbF/UXRdG44P9nOYmITDcNAIl/hb/2Y4xJ6UJV34f8c9OiuYizMNoDwKut Q3+cckiP7/+VRCWSXZR53druKTqItm3PaalK7qzUMXOmdD1qFBpuF5Kng+wnIZQWT2 O1ZmJRPPR13RCNpVwB/+uT84TmZNjNZfMqBiLKBxuvLbAszhfEwicEKDyglNcrIvUp HxD9ot6fuyhUSq0dZeaw61v/EDcB2b1KcMFSsrsHrwvqHcpgFQuldGwRErGnIon0SV aLmfuoCHuFr6GG31kJW3Ibf7UD7wjYu0We0OHEjseY9YGkYJL1///SBbXS67CMbjZ5 0zHPW4aZNdjAg== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id C297110013E; Mon, 1 Sep 2025 01:41:38 -0400 (EDT) Received: from alfajor (69-165-161-194.dsl.teksavvy.com [69.165.161.194]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 8BC0812063D; Mon, 1 Sep 2025 01:41:38 -0400 (EDT) From: Stefan Monnier In-Reply-To: Message-ID: References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Mon, 01 Sep 2025 01:41:37 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP AWL -0.112 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) >> Attached is a patch (which applies on master) which does most of the >> rest of the refactoring that I wanted to do; it makes the dolist over >> the pattern into a simple mapcan over the pattern, with no state >> preserved between elements of the pattern. IMO, this is much easier to >> follow (and I think it would make the various bugs I've fixed in the >> last couple years in merge-completions, easier to catch). What do you >> think? I still don't understand what this is trying to do. It still seems more costly since it allocates more strings and conses onto `pattern-and-comps` than the previous code did with `ccs` (the "corresponding" variable). Also, can we just revert the last patch (and add the test(s) that we found in the mean time) so as to make it easier to review patches? > --- a/lisp/minibuffer.el > +++ b/lisp/minibuffer.el > @@ -4249,7 +4249,11 @@ completion-pcm--pattern->regex > (mapconcat > (lambda (x) > (cond > - ((stringp x) (regexp-quote x)) > + ((stringp x) > + (let ((re (regexp-quote x))) > + (if (eq group 'group-all) > + (concat "\\(" re "\\)") > + re))) > (t > (let ((re (if (eq x 'any-delim) > (concat completion-pcm--delim-wild-regex "*?") I think we should aim for a patch which doesn't use that many subgroups. > @@ -4622,6 +4626,19 @@ completion--common-suffix > "Return the common suffix of the strings STRS." > (nreverse (try-completion "" (mapcar #'reverse strs)))) > > +(defun completion-pcm--group-pattern (pattern) > + "Group together adjacent wildcards in PATTERN." > + (let (ret) > + (dolist (elem pattern) > + (cond > + ((or (stringp elem) (eq elem 'any-delim)) > + (push elem ret)) > + ((consp (car-safe ret)) > + (setf (car ret) (append (car ret) (list elem)))) > + (t > + (push (list elem) ret)))) > + (nreverse ret))) Why is `any-delim` not considered a "wildcard"? Maybe a better grouping would be to turn ("a" "c" any "b" point) into something like (("a" "c" any) ("b" point)) or (("ac" any) ("b" point)) where each sublist will get its own regexp subgroup and will have its corresponding `try-completion`. > + ;; Then for each of those non-constant elements, extract the > + ;; commonality between them. AFAICT this includes "constant" elements, so either I'm missing something or the comment is off. > @@ -4774,7 +4773,7 @@ completion-pcm--merge-try > (equal (completion-pcm--pattern->string pattern) (car all))) > t) > (t > - (let* ((mergedpat (completion-pcm--merge-completions all pattern)) > + (let* ((mergedpat (nreverse (completion-pcm--merge-completions all pattern))) > ;; `mergedpat' is in reverse order. Place new point (by > ;; order of preference) either at the old point, or at > ;; the last place where there's something to choose, or Shouldn't that require an update to the subsequent comment? Or is it that `completion-pcm--merge-completions` now doesn't return the merged pat in reverse order, so we need an explicit `nreverse` to recover the previous reserve-order merged pat? Stefan From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion, [PATCH] Refactor completion-pcm--merge-completions Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 01 Sep 2025 15:57:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Stefan Monnier Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175674219928360 (code B ref 79265); Mon, 01 Sep 2025 15:57:02 +0000 Received: (at 79265) by debbugs.gnu.org; 1 Sep 2025 15:56:39 +0000 Received: from localhost ([127.0.0.1]:58278 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1ut6ta-0007NL-RF for submit@debbugs.gnu.org; Mon, 01 Sep 2025 11:56:39 -0400 Received: from mxout5.mail.janestreet.com ([64.215.233.18]:41149) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1ut6tY-0007N5-FR for 79265@debbugs.gnu.org; Mon, 01 Sep 2025 11:56:37 -0400 From: Spencer Baugh In-Reply-To: (Stefan Monnier's message of "Mon, 01 Sep 2025 01:41:37 -0400") References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Mon, 01 Sep 2025 11:56:30 -0400 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1756742190; bh=XXgg9wT7PF7f/V9wIMR/Gx8G8jXh/iH4dQ0hnne8svw=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=XCFwM9ln/yNc80JTYpc+8L2WbeOF/uEqcMYwp8YF8STZYiMVjkY1Im5E7J4uFcdeB muTP7s4hE16u/2JjqzCVRsDhcJBFe/yhW9sTKX53mnK2ozZxBNKnvTw5DAiha0g4HR 8WVgOq4xkLFyyRGppRbquDMuT2kIBTFeV4rxFCnLA3sXuk6ZESdgoZq/7hBWnQub/d eVV2LbgfqqFDFdAYkV2T+RZGPwaFDbOe/TiHio3IxxZKKv8sxoXFt2FWKupBrUInvw Zh6d30bSaaLaWYNNumAVIQ/M63cYppho0Hh3Vh7j4nZz4F8dRFU3pECQEf3GboXjD7 Of7ftwoS91s7w== X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) --=-=-= Content-Type: text/plain Stefan Monnier writes: >>> Attached is a patch (which applies on master) which does most of the >>> rest of the refactoring that I wanted to do; it makes the dolist over >>> the pattern into a simple mapcan over the pattern, with no state >>> preserved between elements of the pattern. IMO, this is much easier to >>> follow (and I think it would make the various bugs I've fixed in the >>> last couple years in merge-completions, easier to catch). What do you >>> think? > > I still don't understand what this is trying to do. It still seems more > costly since it allocates more strings and conses onto > `pattern-and-comps` than the previous code did with `ccs` (the > "corresponding" variable). My goal at this point is just to make this code simpler and easier to understand, since I keep finding bugs in it. But that certainly should not be done in a way that makes performance worse, and you're right that it may currently do that. > Also, can we just revert the last patch (and add the test(s) that we > found in the mean time) so as to make it easier to review patches? Certainly, the attached patch does that. (I would have just done the revert directly but I don't have the ability to push to the emacs repo) >> --- a/lisp/minibuffer.el >> +++ b/lisp/minibuffer.el >> @@ -4249,7 +4249,11 @@ completion-pcm--pattern->regex >> (mapconcat >> (lambda (x) >> (cond >> - ((stringp x) (regexp-quote x)) >> + ((stringp x) >> + (let ((re (regexp-quote x))) >> + (if (eq group 'group-all) >> + (concat "\\(" re "\\)") >> + re))) >> (t >> (let ((re (if (eq x 'any-delim) >> (concat completion-pcm--delim-wild-regex "*?") > > I think we should aim for a patch which doesn't use that many subgroups. Well - just as a note, this patch uses only one subgroup for each sequence of wildcards, which reduces the number of subgroups for some kinds of pattern. >> @@ -4622,6 +4626,19 @@ completion--common-suffix >> "Return the common suffix of the strings STRS." >> (nreverse (try-completion "" (mapcar #'reverse strs)))) >> >> +(defun completion-pcm--group-pattern (pattern) >> + "Group together adjacent wildcards in PATTERN." >> + (let (ret) >> + (dolist (elem pattern) >> + (cond >> + ((or (stringp elem) (eq elem 'any-delim)) >> + (push elem ret)) >> + ((consp (car-safe ret)) >> + (setf (car ret) (append (car ret) (list elem)))) >> + (t >> + (push (list elem) ret)))) >> + (nreverse ret))) > > Why is `any-delim` not considered a "wildcard"? Because it's matched by a different regex which only matches delimiters. The idea here is to have one regex subgroup for each stretch of wildcards; i.e., for the pattern '("foo" any star point) we have the regex "foo(.*)" instead of "foo(.*)(.*)(.*)". But any-delim needs its own regex subgroup since its matched by a different regex. > Maybe a better grouping would be to turn > > ("a" "c" any "b" point) > > into something like > > (("a" "c" any) ("b" point)) > > or > > (("ac" any) ("b" point)) > > where each sublist will get its own regexp subgroup and will have its > corresponding `try-completion`. That's a great idea, and it matches up with things I was already thinking of doing. I'll try that in the next version of this patch. >> + ;; Then for each of those non-constant elements, extract the >> + ;; commonality between them. > > AFAICT this includes "constant" elements, so either I'm missing > something or the comment is off. > >> @@ -4774,7 +4773,7 @@ completion-pcm--merge-try >> (equal (completion-pcm--pattern->string pattern) (car all))) >> t) >> (t >> - (let* ((mergedpat (completion-pcm--merge-completions all pattern)) >> + (let* ((mergedpat (nreverse (completion-pcm--merge-completions all pattern))) >> ;; `mergedpat' is in reverse order. Place new point (by >> ;; order of preference) either at the old point, or at >> ;; the last place where there's something to choose, or > > Shouldn't that require an update to the subsequent comment? > Or is it that `completion-pcm--merge-completions` now doesn't return the > merged pat in reverse order, so we need an explicit `nreverse` to > recover the previous reserve-order merged pat? Yes, the return value of completion-pcm--merge-completions is no longer in reverse order. The attached patch just reverts my original broken change and adds tests. After landing that, I'll open a new bug if I want to further work on a patch like this. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Revert-Avoid-duplicating-strings-in-pcm-merge-comple.patch >From 2bbb3e31467449be25286af5f03078c3fbc5ac8b Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Mon, 1 Sep 2025 11:43:25 -0400 Subject: [PATCH] Revert "Avoid duplicating strings in pcm--merge-completions" Revert "Avoid duplicating strings in pcm--merge-completions", commit b511c38bba5354ff21c697e4d27279bf73e4d3cf. It broke existing behavior, now covered by tests adding in this commit. * lisp/minibuffer.el (completion-pcm--merge-completions): * test/lisp/minibuffer-tests.el (completion-pcm-test-anydelim): (completion-pcm-bug4219): --- lisp/minibuffer.el | 39 +++++++++++++++++++++-------------- test/lisp/minibuffer-tests.el | 15 ++++++++++++++ 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 1c3c4bd0681..6617aee2d5a 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -4639,35 +4639,38 @@ completion-pcm--merge-completions (cond ((null (cdr strs)) (list (car strs))) (t - (let ((re (concat - (completion-pcm--pattern->regex pattern 'group) - ;; The implicit trailing `any' is greedy. - "\\([^z-a]*\\)")) + (let ((re (completion-pcm--pattern->regex pattern 'group)) (ccs ())) ;Chopped completions. - ;; First match each string against PATTERN as a regex and extract - ;; the text matched by each wildcard. + ;; First chop each string into the parts corresponding to each + ;; non-constant element of `pattern', using regexp-matching. (let ((case-fold-search completion-ignore-case)) (dolist (str strs) (unless (string-match re str) (error "Internal error: %s doesn't match %s" str re)) (let ((chopped ()) + (last 0) (i 1) next) - (while (setq next (match-string i str)) - (push next chopped) + (while (setq next (match-end i)) + (push (substring str last next) chopped) + (setq last next) (setq i (1+ i))) + ;; Add the text corresponding to the implicit trailing `any'. + (push (substring str last) chopped) (push (nreverse chopped) ccs)))) - ;; Then for each of those wildcards, extract the commonality between them. + ;; Then for each of those non-constant elements, extract the + ;; commonality between them. (let ((res ()) + (fixed "") ;; Accumulate each stretch of wildcards, and process them as a unit. (wildcards ())) ;; Make the implicit trailing `any' explicit. (dolist (elem (append pattern '(any))) (if (stringp elem) (progn - (push elem res) + (setq fixed (concat fixed elem)) (setq wildcards nil)) (let ((comps ())) (push elem wildcards) @@ -4678,13 +4681,18 @@ completion-pcm--merge-completions ;; different capitalizations in different parts. ;; In practice, it doesn't seem to make any difference. (setq ccs (nreverse ccs)) - (let* ((prefix (try-completion "" comps)) - (unique (or (and (eq prefix t) (setq prefix "")) + ;; FIXED is a prefix of all of COMPS. Try to grow that prefix. + (let* ((prefix (try-completion fixed comps)) + (unique (or (and (eq prefix t) (setq prefix fixed)) (and (stringp prefix) ;; If PREFIX is equal to all of COMPS, ;; then PREFIX is a unique completion. (seq-every-p - (lambda (comp) (= (length prefix) (length comp))) + ;; PREFIX is still a prefix of all of + ;; COMPS, so if COMP is the same length, + ;; they're equal. + (lambda (comp) + (= (length prefix) (length comp))) comps))))) ;; If there's only one completion, `elem' is not useful ;; any more: it can only match the empty string. @@ -4699,7 +4707,7 @@ completion-pcm--merge-completions ;; `prefix' only wants to include the fixed part before the ;; wildcard, not the result of growing that fixed part. (when (seq-some (lambda (elem) (eq elem 'prefix)) wildcards) - (setq prefix "")) + (setq prefix fixed)) (push prefix res) ;; Push all the wildcards in this stretch, to preserve `point' and ;; `star' wildcards before ELEM. @@ -4723,7 +4731,8 @@ completion-pcm--merge-completions (unless (equal suffix "") (push suffix res)))) ;; We pushed these wildcards on RES, so we're done with them. - (setq wildcards nil)))))) + (setq wildcards nil)) + (setq fixed ""))))) ;; We return it in reverse order. res))))) diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el index 99753f31330..de1a98c8189 100644 --- a/test/lisp/minibuffer-tests.el +++ b/test/lisp/minibuffer-tests.el @@ -332,6 +332,21 @@ completion-pcm-test-8 "" '("fooxbar" "fooybar") nil 0) '("foobar" . 3)))) +(ert-deftest completion-pcm-test-anydelim () + ;; After each delimiter is a special wildcard which matches any + ;; sequence of delimiters. + (should (equal (completion-pcm-try-completion + "-x" '("-_.x" "-__x") nil 2) + '("-_x" . 3)))) + +(ert-deftest completion-pcm-bug4219 () + ;; With `completion-ignore-case', try-completion should change the + ;; case of existing text when the completions have different casing. + (should (equal + (let ((completion-ignore-case t)) + (completion-pcm-try-completion "a" '("ABC" "ABD") nil 1)) + '("AB" . 2)))) + (ert-deftest completion-substring-test-1 () ;; One third of a match! (should (equal -- 2.43.7 --=-=-=-- From unknown Thu Sep 11 09:00:07 2025 X-Loop: help-debbugs@gnu.org Subject: bug#79265: [PATCH] Treat point more consistently in PCM completion, [PATCH] Treat point more consistently in PCM completion, [PATCH] Refactor completion-pcm--merge-completions Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 01 Sep 2025 21:26:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 79265 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch To: Spencer Baugh Cc: Dmitry Gutov , 79265@debbugs.gnu.org Received: via spool by 79265-submit@debbugs.gnu.org id=B79265.175676190229223 (code B ref 79265); Mon, 01 Sep 2025 21:26:02 +0000 Received: (at 79265) by debbugs.gnu.org; 1 Sep 2025 21:25:02 +0000 Received: from localhost ([127.0.0.1]:58688 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1utC1N-0007b5-Gy for submit@debbugs.gnu.org; Mon, 01 Sep 2025 17:25:01 -0400 Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:52935) by debbugs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.84_2) (envelope-from ) id 1utC1K-0007aq-SN for 79265@debbugs.gnu.org; Mon, 01 Sep 2025 17:24:59 -0400 Received: from pmg3.iro.umontreal.ca (localhost [127.0.0.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id 30EC744106B; Mon, 1 Sep 2025 17:24:52 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1756761886; bh=NQzUnKAAtttCXBvy6m0BQ3AOAuR0Hv7aA4wUh9rfM50=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=fZWJlfD5qukAPU2PQC0GYd1+TBeo10UtfraSe61tV/CK2yc+68/CBtMPFZHeQGBkh BMhqoH/LTaKuHcpHBP6WJHgRXeNBDTq0Nar+zI2iI0w6ERoI9Y2QXnOestPeVLxHwz De+cRY/NgzAiVx6gIxO0wM9CE5ZlIW6I1cUYWStUmvr6U9t1U/jJpMbeeaYlR536m6 WTgXlrXIR0J3wKbdWeuFo5+D2cybEQuPwjysXDwnInAerT3xi0wDPacZlF9q5FxNYI G9VSw/j2S6R1FSNZ7wsvmrRWTlxXbhO1jgCKthqR+8JGeOJtFxIBwA1fKoBdmjccXK GiYsAULg75VlQ== Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg3.iro.umontreal.ca (Proxmox) with ESMTP id B7A1A440F9D; Mon, 1 Sep 2025 17:24:46 -0400 (EDT) Received: from alfajor (69-165-161-194.dsl.teksavvy.com [69.165.161.194]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 787081202C7; Mon, 1 Sep 2025 17:24:46 -0400 (EDT) From: Stefan Monnier In-Reply-To: Message-ID: References: <494e7ecc-42e0-4d3d-a4b4-a043f650fc94@gutov.dev> <60d07537-9a46-4db6-855f-70632780004d@gutov.dev> Date: Mon, 01 Sep 2025 17:24:45 -0400 User-Agent: Gnus/5.13 (Gnus v5.13) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-SPAM-INFO: Spam detection results: 0 ALL_TRUSTED -1 Passed through trusted hosts only via SMTP AWL -0.332 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DKIM_SIGNED 0.1 Message has a DKIM or DK signature, not necessarily valid DKIM_VALID -0.1 Message has at least one valid DKIM or DK signature DKIM_VALID_AU -0.1 Message has a valid DKIM or DK signature from author's domain DKIM_VALID_EF -0.1 Message has a valid DKIM or DK signature from envelope-from domain X-SPAM-LEVEL: X-Spam-Score: -2.3 (--) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: debbugs-submit-bounces@debbugs.gnu.org Sender: "Debbugs-submit" X-Spam-Score: -3.3 (---) >> Also, can we just revert the last patch (and add the test(s) that we >> found in the mean time) so as to make it easier to review patches? > > Certainly, the attached patch does that. > > (I would have just done the revert directly but I don't have the ability > to push to the emacs repo) Sorry, forgot about that. Pushed, thanks. >>> +(defun completion-pcm--group-pattern (pattern) >>> + "Group together adjacent wildcards in PATTERN." >>> + (let (ret) >>> + (dolist (elem pattern) >>> + (cond >>> + ((or (stringp elem) (eq elem 'any-delim)) >>> + (push elem ret)) >>> + ((consp (car-safe ret)) >>> + (setf (car ret) (append (car ret) (list elem)))) >>> + (t >>> + (push (list elem) ret)))) >>> + (nreverse ret))) >> >> Why is `any-delim` not considered a "wildcard"? > > Because it's matched by a different regex which only matches > delimiters. The idea here is to have one regex subgroup for each > stretch of wildcards; i.e., for the pattern '("foo" any star point) we > have the regex "foo(.*)" instead of "foo(.*)(.*)(.*)". But any-delim > needs its own regex subgroup since its matched by a different regex. Ah, I see, thanks. I was thinking in terms of "needs a subgroup and a `try-completion`", which `any-delim` does need. But yeah, it's also good to collapse any/star/point (also because it can avoid some pathological regexp matching performance issues, tho I haven't bumped into them). > Yes, the return value of completion-pcm--merge-completions is no > longer in reverse order. Thanks, makes a lot more sense now. =F0=9F=99=82 Stefan