* bug#72819: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM
@ 2024-08-26 14:17 Spencer Baugh
2024-08-27 10:59 ` Eli Zaretskii
2024-09-07 7:32 ` Eli Zaretskii
0 siblings, 2 replies; 6+ messages in thread
From: Spencer Baugh @ 2024-08-26 14:17 UTC (permalink / raw)
To: 72819; +Cc: monnier
[-- Attachment #1: Type: text/plain, Size: 1216 bytes --]
Tags: patch
In 63a48252306a631dc07d62d19311433c7877bd27 I fixed a bug with
the PCM implementation of substring completion, relating to the
handling of PCM wildcards.
However, this fix was incomplete. This change completes the fix by
also including a fixed string if it appears before a `prefix'
wildcard, even if try-completion doesn't discover that fixed string
grows to a unique completion.
I discovered this bug while working on enhancements to PCM completion
related to completion-pcm-leading-wildcard.
In GNU Emacs 29.2.50 (build 12, x86_64-pc-linux-gnu, X toolkit, cairo
version 1.15.12, Xaw scroll bars) of 2024-08-23 built on
igm-qws-u22796a
Repository revision: 253525ba4598f487e451b93a4ab1680ef212a62d
Repository branch: emacs-29
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-native-compilation=aot --with-tree-sitter
PKG_CONFIG_PATH=/usr/local/home/garnish/libtree-sitter/0.22.6-1/lib/pkgconfig/'
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Correctly-include-fixed-strings-before-a-prefix-wild.patch --]
[-- Type: text/patch, Size: 4228 bytes --]
From 6d7a8f5e491e4ba4eb6e0026278b4df3d70a4a83 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Mon, 26 Aug 2024 10:12:51 -0400
Subject: [PATCH] Correctly include fixed strings before a prefix wildcard in
PCM
In 63a48252306a631dc07d62d19311433c7877bd27 I fixed a bug with
the PCM implementation of substring completion, relating to the
handling of PCM wildcards.
However, this fix was incomplete. This change completes the fix by
also including a fixed string if it appears before a `prefix'
wildcard, even if try-completion doesn't discover that fixed string
grows to a unique completion.
I discovered this bug while working on enhancements to PCM completion
related to completion-pcm-leading-wildcard.
* lisp/minibuffer.el (completion-pcm--merge-completions):
* test/lisp/minibuffer-tests.el (completion-substring-test-5):
---
lisp/minibuffer.el | 15 +++++++++------
test/lisp/minibuffer-tests.el | 17 ++++++++++++-----
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 6fae62b3904..1efe71f10e3 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4418,18 +4418,21 @@ completion-pcm--merge-completions
(unique (or (and (eq prefix t) (setq prefix fixed))
(and (stringp prefix)
(eq t (try-completion prefix comps))))))
- ;; if the common prefix is unique, it also is a common
- ;; suffix, so we should add it for `prefix' elements
- (unless (or (and (eq elem 'prefix) (not unique))
- (equal prefix ""))
- (push prefix res))
;; 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.
- (unless unique
+ (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 (eq elem 'prefix)
+ (setq prefix fixed))
+ (push prefix res)
(push elem res)
;; Extract common suffix additionally to common prefix.
;; Don't do it for `any' since it could lead to a merged
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index df36bce4634..38c2b8c4552 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -306,13 +306,20 @@ completion-substring-test-4
6)))
(ert-deftest completion-substring-test-5 ()
- ;; merge-completions needs to work correctly when
+ ;; Normally a `prefix' wildcard ignores the common prefix to its
+ ;; left, since it only grows the common suffix; but if that common
+ ;; prefix is also a common suffix, it should be included.
(should (equal
- (completion-pcm--merge-completions '("ab" "sab") '(prefix "b"))
- '("b" "a" prefix)))
+ (completion-pcm--merge-try '(prefix "b") '("ab" "sab") "" "")
+ '("ab" . 2)))
(should (equal
- (completion-pcm--merge-completions '("ab" "ab") '(prefix "b"))
- '("b" "a")))
+ (completion-pcm--merge-try '(prefix "b") '("ab" "ab") "" "")
+ '("ab" . 2)))
+ ;; When there's a fixed string before `prefix', that fixed string
+ ;; should always be included.
+ (should (equal
+ (completion-pcm--merge-try '("a" prefix "b") '("axb" "ayb") "" "")
+ '("ab" . 2)))
;; substring completion should successfully complete the entire string
(should (equal
(completion-substring-try-completion "b" '("ab" "ab") nil 0)
--
2.39.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#72819: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM
2024-08-26 14:17 bug#72819: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM Spencer Baugh
@ 2024-08-27 10:59 ` Eli Zaretskii
2024-09-10 16:31 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-09-07 7:32 ` Eli Zaretskii
1 sibling, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2024-08-27 10:59 UTC (permalink / raw)
To: Spencer Baugh; +Cc: 72819, monnier
> Cc: monnier@iro.umontreal.ca
> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Mon, 26 Aug 2024 10:17:05 -0400
>
> In 63a48252306a631dc07d62d19311433c7877bd27 I fixed a bug with
Git tells me "bad object 63a48252306a631dc07d62d19311433c7877bd27", so
I couldn't figure out which commit are you referring to.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#72819: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM
2024-08-26 14:17 bug#72819: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM Spencer Baugh
2024-08-27 10:59 ` Eli Zaretskii
@ 2024-09-07 7:32 ` Eli Zaretskii
2024-09-08 11:15 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
1 sibling, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2024-09-07 7:32 UTC (permalink / raw)
To: Spencer Baugh, monnier; +Cc: 72819
Ping! Stefan, any comments?
> Cc: monnier@iro.umontreal.ca
> From: Spencer Baugh <sbaugh@janestreet.com>
> Date: Mon, 26 Aug 2024 10:17:05 -0400
>
> Tags: patch
>
>
> In 63a48252306a631dc07d62d19311433c7877bd27 I fixed a bug with
> the PCM implementation of substring completion, relating to the
> handling of PCM wildcards.
>
> However, this fix was incomplete. This change completes the fix by
> also including a fixed string if it appears before a `prefix'
> wildcard, even if try-completion doesn't discover that fixed string
> grows to a unique completion.
>
> I discovered this bug while working on enhancements to PCM completion
> related to completion-pcm-leading-wildcard.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#72819: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM
2024-09-07 7:32 ` Eli Zaretskii
@ 2024-09-08 11:15 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
0 siblings, 0 replies; 6+ messages in thread
From: Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-09-08 11:15 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: Spencer Baugh, 72819
> Ping! Stefan, any comments?
Looks good to me.
[ Maybe we could appoint Spencer as maintainer of that code :-) ]
Stefan
>> Cc: monnier@iro.umontreal.ca
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Mon, 26 Aug 2024 10:17:05 -0400
>>
>> Tags: patch
>>
>>
>> In 63a48252306a631dc07d62d19311433c7877bd27 I fixed a bug with
>> the PCM implementation of substring completion, relating to the
>> handling of PCM wildcards.
>>
>> However, this fix was incomplete. This change completes the fix by
>> also including a fixed string if it appears before a `prefix'
>> wildcard, even if try-completion doesn't discover that fixed string
>> grows to a unique completion.
>>
>> I discovered this bug while working on enhancements to PCM completion
>> related to completion-pcm-leading-wildcard.
^ permalink raw reply [flat|nested] 6+ messages in thread
* bug#72819: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM
2024-08-27 10:59 ` Eli Zaretskii
@ 2024-09-10 16:31 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-09-14 9:27 ` Eli Zaretskii
0 siblings, 1 reply; 6+ messages in thread
From: Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors @ 2024-09-10 16:31 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: 72819, monnier
[-- Attachment #1: Type: text/plain, Size: 425 bytes --]
Eli Zaretskii <eliz@gnu.org> writes:
>> Cc: monnier@iro.umontreal.ca
>> From: Spencer Baugh <sbaugh@janestreet.com>
>> Date: Mon, 26 Aug 2024 10:17:05 -0400
>>
>> In 63a48252306a631dc07d62d19311433c7877bd27 I fixed a bug with
>
> Git tells me "bad object 63a48252306a631dc07d62d19311433c7877bd27", so
> I couldn't figure out which commit are you referring to.
Oops, here's the patch again with a corrected commit message:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Correctly-include-fixed-strings-before-a-prefix-wild.patch --]
[-- Type: text/x-patch, Size: 4317 bytes --]
From 1c9df731bacadd4ef762675af9903ccb0897a3ae Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@janestreet.com>
Date: Mon, 26 Aug 2024 10:12:51 -0400
Subject: [PATCH] Correctly include fixed strings before a prefix wildcard in
PCM
In 03ac16ece40ba3e3ba805d6a61cc457d84bf3792 I fixed a bug with the PCM
implementation of substring completion, relating to the handling of
PCM wildcards.
However, this fix was incomplete. This change completes the fix by
also including a fixed string if it appears before a `prefix'
wildcard, even if try-completion doesn't discover that fixed string
grows to a unique completion.
I discovered this bug while working on enhancements to PCM completion
related to completion-pcm-leading-wildcard.
* lisp/minibuffer.el (completion-pcm--merge-completions): Include
fixed strings before 'prefix wildcard. (bug#72819)
* test/lisp/minibuffer-tests.el (completion-substring-test-5): Add a
test for this behavior.
---
lisp/minibuffer.el | 15 +++++++++------
test/lisp/minibuffer-tests.el | 17 ++++++++++++-----
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 6fae62b3904..1efe71f10e3 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4418,18 +4418,21 @@ completion-pcm--merge-completions
(unique (or (and (eq prefix t) (setq prefix fixed))
(and (stringp prefix)
(eq t (try-completion prefix comps))))))
- ;; if the common prefix is unique, it also is a common
- ;; suffix, so we should add it for `prefix' elements
- (unless (or (and (eq elem 'prefix) (not unique))
- (equal prefix ""))
- (push prefix res))
;; 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.
- (unless unique
+ (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 (eq elem 'prefix)
+ (setq prefix fixed))
+ (push prefix res)
(push elem res)
;; Extract common suffix additionally to common prefix.
;; Don't do it for `any' since it could lead to a merged
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index df36bce4634..38c2b8c4552 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -306,13 +306,20 @@ completion-substring-test-4
6)))
(ert-deftest completion-substring-test-5 ()
- ;; merge-completions needs to work correctly when
+ ;; Normally a `prefix' wildcard ignores the common prefix to its
+ ;; left, since it only grows the common suffix; but if that common
+ ;; prefix is also a common suffix, it should be included.
(should (equal
- (completion-pcm--merge-completions '("ab" "sab") '(prefix "b"))
- '("b" "a" prefix)))
+ (completion-pcm--merge-try '(prefix "b") '("ab" "sab") "" "")
+ '("ab" . 2)))
(should (equal
- (completion-pcm--merge-completions '("ab" "ab") '(prefix "b"))
- '("b" "a")))
+ (completion-pcm--merge-try '(prefix "b") '("ab" "ab") "" "")
+ '("ab" . 2)))
+ ;; When there's a fixed string before `prefix', that fixed string
+ ;; should always be included.
+ (should (equal
+ (completion-pcm--merge-try '("a" prefix "b") '("axb" "ayb") "" "")
+ '("ab" . 2)))
;; substring completion should successfully complete the entire string
(should (equal
(completion-substring-try-completion "b" '("ab" "ab") nil 0)
--
2.39.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* bug#72819: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM
2024-09-10 16:31 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
@ 2024-09-14 9:27 ` Eli Zaretskii
0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2024-09-14 9:27 UTC (permalink / raw)
To: Spencer Baugh; +Cc: 72819-done, monnier
> From: Spencer Baugh <sbaugh@janestreet.com>
> Cc: 72819@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Tue, 10 Sep 2024 12:31:13 -0400
>
> Eli Zaretskii <eliz@gnu.org> writes:
> >> Cc: monnier@iro.umontreal.ca
> >> From: Spencer Baugh <sbaugh@janestreet.com>
> >> Date: Mon, 26 Aug 2024 10:17:05 -0400
> >>
> >> In 63a48252306a631dc07d62d19311433c7877bd27 I fixed a bug with
> >
> > Git tells me "bad object 63a48252306a631dc07d62d19311433c7877bd27", so
> > I couldn't figure out which commit are you referring to.
>
> Oops, here's the patch again with a corrected commit message:
Thanks, installed, and closing the bug.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-09-14 9:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-26 14:17 bug#72819: [PATCH] Correctly include fixed strings before a prefix wildcard in PCM Spencer Baugh
2024-08-27 10:59 ` Eli Zaretskii
2024-09-10 16:31 ` Spencer Baugh via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-09-14 9:27 ` Eli Zaretskii
2024-09-07 7:32 ` Eli Zaretskii
2024-09-08 11:15 ` Stefan Monnier via Bug reports for GNU Emacs, the Swiss army knife of text editors
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).