GNU bug report logs - #67210
30.0.50; completing-read with REQUIRE-MATCH=t can sometimes return a non-match

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 67210 in the body.
You can then email your comments to 67210 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 30.0.50; completing-read with REQUIRE-MATCH=t can sometimes return
 a non-match
Date: Wed, 15 Nov 2023 15:25:15 -0500

1. emacs -Q
2. (completing-read ":" '("foo") nil t '("foobar" . 3))
3. RET
4. See "foobar", i.e. "foobar" was accepted as input despite not being
   in the collection we're completing over.

I believe this is because
(completion-try-completion "foobar" '("foo") nil 3)
returns t, which completion--do-completion interprets as meaning that
the input string is an "Exact and unique match."

That t is returned because
(completion-emacs22-try-completion "foobar" '("foo") nil 3)
returns t.

So perhaps completion--do-completion is wrong in assuming that a t
return value from completion-try-completion indicates an "Exact and
unique match."?

Or perhaps completion-emacs22-try-completion is violating the spec of
completion-try-completion, and should be returning nil in this case?


In GNU Emacs 30.0.50 (build 5, x86_64-pc-linux-gnu, GTK+ Version
 3.22.30, cairo version 1.15.12) of 2023-11-15 built on igm-qws-u22796a
Repository revision: 1a1f47e4a1fb70e6810f9eabd0f1826b71a2bcb0
Repository branch: project-history
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Rocky Linux 8.8 (Green Obsidian)

Configured using:
 'configure --enable-checking=yes,glyphs --enable-check-lisp-object-type
 'CFLAGS=-O0 -g3' -C --with-gif=ifavailable'

Configured features:
CAIRO DBUS FREETYPE GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG JSON
LIBSELINUX LIBSYSTEMD LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG RSVG
SECCOMP SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS X11 XDBE XIM
XINPUT2 XPM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tool-bar-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
None found.

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util text-property-search time-date subr-x mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
cl-loaddefs cl-lib sendmail rfc2047 rfc2045 ietf-drums mm-util
mail-prsvr mail-utils rmc iso-transl tooltip cconv eldoc paren electric
uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/x-win x-win term/common-win x-dnd touch-screen tool-bar dnd fontset
image regexp-opt fringe tabulated-list replace newcomment text-mode
lisp-mode prog-mode register page tab-bar menu-bar rfn-eshadow isearch
easymenu timer select scroll-bar mouse jit-lock font-lock syntax
font-core term/tty-colors frame minibuffer nadvice seq simple cl-generic
indonesian philippine cham georgian utf-8-lang misc-lang vietnamese
tibetan thai tai-viet lao korean japanese eucjp-ms cp51932 hebrew greek
romanian slovak czech european ethiopic indian cyrillic chinese
composite emoji-zwj charscript charprop case-table epa-hook
jka-cmpr-hook help abbrev obarray oclosure cl-preloaded button loaddefs
theme-loaddefs faces cus-face macroexp files window text-properties
overlay sha1 md5 base64 format env code-pages mule custom widget keymap
hashtable-print-readable backquote threads dbusbind inotify
dynamic-setting system-font-setting font-render-setting cairo gtk
x-toolkit xinput2 x multi-tty move-toolbar make-network-process emacs)

Memory information:
((conses 16 64205 13453) (symbols 48 9538 0) (strings 32 22015 1421)
 (string-bytes 1 665593) (vectors 16 10318)
 (vector-slots 8 156970 17260) (floats 8 37 13) (intervals 56 253 0)
 (buffers 984 10))

Message #8 received at 67210 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>, "67210 <at> debbugs.gnu.org"
 <67210 <at> debbugs.gnu.org>
Subject: RE: [External] : bug#67210: 30.0.50; completing-read with
 REQUIRE-MATCH=t can sometimes return a non-match
Date: Thu, 16 Nov 2023 01:29:19 +0000

Good question!  Dunno whether it's ever come
up before.
___

FWIW, this is the behavior back at least as
far as Emacs 20.  It may _always_ have been
the behavior for `completing-read'; dunno.
__

(FWIW2, it's NOT the Icicles behavior, with
Icicle mode turned on.  In that case the
initial input doesn't match any candidates,
and since REQUIRE is non-nil RET won't exit
the minibuffer.

I don't recall why I chose that the behavior,
or even whether I did so consciously, but it
does make sense to me.)
__

FWIW3.  ("foobar" . 3) is a reasonable INIT
value in that example only in the sense that
a user _could_ hit `C-k' and then `RET', to
use the input "foo".

Why might that be done?  Far-fetched, no
doubt, but trailing text after point (which
is after "foo") could perhaps serve some
other purpose in some context, e.g., as a
tip or emphasis or instructions or ...

But even in such a context, I can't see why
the input of "foobar" would be accepted.
(But see FWIW4, next, for an alternative POV.)
___

FWIW4, I can see an argument being made that
when you use the INIT value you're no longer
completing - regardless of how you might edit
that text - so args REQUIRE and COLLECTION
have no significance.

That's not the way I'd like to look at it, but
I can imagine that it might be the original
rationale, or at least it might be argued today.

Message #11 received at 67210 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Spencer Baugh <sbaugh <at> janestreet.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 67210 <at> debbugs.gnu.org
Subject: Re: bug#67210: 30.0.50;
 completing-read with REQUIRE-MATCH=t can sometimes return a non-match
Date: Thu, 16 Nov 2023 07:38:40 +0200

> From: Spencer Baugh <sbaugh <at> janestreet.com>
> Date: Wed, 15 Nov 2023 15:25:15 -0500
> 
> 
> 1. emacs -Q
> 2. (completing-read ":" '("foo") nil t '("foobar" . 3))
> 3. RET
> 4. See "foobar", i.e. "foobar" was accepted as input despite not being
>    in the collection we're completing over.
> 
> I believe this is because
> (completion-try-completion "foobar" '("foo") nil 3)
> returns t, which completion--do-completion interprets as meaning that
> the input string is an "Exact and unique match."
> 
> That t is returned because
> (completion-emacs22-try-completion "foobar" '("foo") nil 3)
> returns t.
> 
> So perhaps completion--do-completion is wrong in assuming that a t
> return value from completion-try-completion indicates an "Exact and
> unique match."?
> 
> Or perhaps completion-emacs22-try-completion is violating the spec of
> completion-try-completion, and should be returning nil in this case?

Adding Stefan to the discussion, as he probably has an opinion about
this.

Message #14 received at 67210 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: "67210 <at> debbugs.gnu.org" <67210 <at> debbugs.gnu.org>
Subject: Re: bug#67210: 30.0.50; completing-read with REQUIRE-MATCH=t can
 sometimes return a non-match
Date: Thu, 16 Nov 2023 09:47:17 -0500

Drew Adams <drew.adams <at> oracle.com> writes:
> FWIW3.  ("foobar" . 3) is a reasonable INIT
> value in that example only in the sense that
> a user _could_ hit `C-k' and then `RET', to
> use the input "foo".
>
> Why might that be done?  Far-fetched, no
> doubt, but trailing text after point (which
> is after "foo") could perhaps serve some
> other purpose in some context, e.g., as a
> tip or emphasis or instructions or ...
>
> But even in such a context, I can't see why
> the input of "foobar" would be accepted.
> (But see FWIW4, next, for an alternative POV.)
> ___
>
> FWIW4, I can see an argument being made that
> when you use the INIT value you're no longer
> completing - regardless of how you might edit
> that text - so args REQUIRE and COLLECTION
> have no significance.
>
> That's not the way I'd like to look at it, but
> I can imagine that it might be the original
> rationale, or at least it might be argued today.

To be clear, this happens even without setting INIT, I just included
that to make the reproduction clearer.  Sorry for the ambguity.

i.e. doing

1. (completing-read ":" '("foo") nil t)
2. type in "foobar"
3. move point back to after "foo"
4. RET

also returns "foobar"

Message #17 received at 67210 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Spencer Baugh <sbaugh <at> janestreet.com>, 67210 <at> debbugs.gnu.org
Subject: Re: bug#67210: 30.0.50; completing-read with REQUIRE-MATCH=t can
 sometimes return a non-match
Date: Thu, 16 Nov 2023 10:04:51 -0500

>> Or perhaps completion-emacs22-try-completion is violating the spec of
>> completion-try-completion, and should be returning nil in this case?

The "emacs22" style disregards the text after point to compute the
completions, so "foo" would be a valid return value as well, but indeed
t seems wrong because that should apply to the complete input.


        Stefan

Message #20 received at 67210 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 67210 <at> debbugs.gnu.org
Subject: Re: bug#67210: 30.0.50; completing-read with REQUIRE-MATCH=t can
 sometimes return a non-match
Date: Thu, 16 Nov 2023 11:36:57 -0500

[Message part 1 (text/plain, inline)]

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:
>>> Or perhaps completion-emacs22-try-completion is violating the spec of
>>> completion-try-completion, and should be returning nil in this case?
>
> The "emacs22" style disregards the text after point to compute the
> completions, so "foo" would be a valid return value as well, but indeed
> t seems wrong because that should apply to the complete input.

I think "foo" (or more specifically '("foo" . 3)) might be a bit
unexpected, because then if you pressed TAB with the input foo|bar, the
bar would be deleted.  And emacs22 completion usually doesn't delete the
text after point, it just ignores it.

So '("foobar" . 3) seems better.  Open to counterarguments of course.

A patch which implements this is below.

[0001-Return-t-from-completion-emacs22-try-completion-only.patch (text/x-patch, inline)]

From e96e154f1ab6c0a743b88cfd7a9b59e14726b07a Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Thu, 16 Nov 2023 11:34:08 -0500
Subject: [PATCH] Return t from completion-emacs22-try-completion only for
 completions

The emacs22 completion style ignores the text after point when
computing completions.  However, it still needs to take into account
the entire string it's given, to avoid returning incorrect values.

Previously, completion-emacs22-try-completion would return t if the
text before point was an exact completion.  But this is effectively
saying that the entire input string was an exact completion, which may
not be correct.  This would cause completing-read with REQUIRE-MATCH=t
to return a non-completion.

Now, completion-emacs22-try-completion only returns t if the entire
input string is an exact completion.

* lisp/minibuffer.el (completion-emacs22-try-completion): Return t
only if the entire input string is an exact completion.  (Bug#67210)
---
 lisp/minibuffer.el | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 07a284134d6..e9a349d39ea 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3526,8 +3526,15 @@ completion-emacs21-all-completions
 (defun completion-emacs22-try-completion (string table pred point)
   (let ((suffix (substring string point))
         (completion (try-completion (substring string 0 point) table pred)))
-    (if (not (stringp completion))
-        completion
+    (cond
+     ((eq completion t)
+      ;; The prefix is an exact and unique completion, but STRING
+      ;; might not be a completion.
+      (if (test-completion string table)
+          t
+        (cons string point)))
+     ((not (stringp completion)) completion)
+     (t
       ;; Merge a trailing / in completion with a / after point.
       ;; We used to only do it for word completion, but it seems to make
       ;; sense for all completions.
@@ -3541,7 +3548,7 @@ completion-emacs22-try-completion
                (eq ?/ (aref suffix 0)))
           ;; This leaves point after the / .
           (setq suffix (substring suffix 1)))
-      (cons (concat completion suffix) (length completion)))))
+      (cons (concat completion suffix) (length completion))))))
 
 (defun completion-emacs22-all-completions (string table pred point)
   (let ((beforepoint (substring string 0 point)))
-- 
2.39.3

Message #23 received at 67210 <at> debbugs.gnu.org (full text, mbox):

From: Drew Adams <drew.adams <at> oracle.com>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: "67210 <at> debbugs.gnu.org" <67210 <at> debbugs.gnu.org>
Subject: RE: [External] : Re: bug#67210: 30.0.50; completing-read with
 REQUIRE-MATCH=t can  sometimes return a non-match
Date: Thu, 16 Nov 2023 17:13:32 +0000

> To be clear, this happens even without setting INIT, I just included
> that to make the reproduction clearer.  Sorry for the ambguity.
> 
> i.e. doing
> 1. (completing-read ":" '("foo") nil t)
> 2. type in "foobar"
> 3. move point back to after "foo"
> 4. RET
> also returns "foobar"

Yes.

It doesn't in Icicle mode:

* The `bar' part is highlighted as not matching any candidates.
* `RET' doesn't exit the minibuffer.

Message #26 received at 67210 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 67210 <at> debbugs.gnu.org
Subject: Re: bug#67210: 30.0.50; completing-read with REQUIRE-MATCH=t can
 sometimes return a non-match
Date: Thu, 16 Nov 2023 13:24:42 -0500

>>>> Or perhaps completion-emacs22-try-completion is violating the spec of
>>>> completion-try-completion, and should be returning nil in this case?
>>
>> The "emacs22" style disregards the text after point to compute the
>> completions, so "foo" would be a valid return value as well, but indeed
>> t seems wrong because that should apply to the complete input.
>
> I think "foo" (or more specifically '("foo" . 3)) might be a bit
> unexpected, because then if you pressed TAB with the input foo|bar, the
> bar would be deleted.  And emacs22 completion usually doesn't delete the
> text after point, it just ignores it.
>
> So '("foobar" . 3) seems better.  Open to counterarguments of course.

Oh, right you are.


        Stefan

Message #29 received at 67210 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 67210 <at> debbugs.gnu.org
Subject: Re: bug#67210: 30.0.50; completing-read with REQUIRE-MATCH=t can
 sometimes return a non-match
Date: Fri, 17 Nov 2023 11:06:09 -0500

>  (defun completion-emacs22-try-completion (string table pred point)
>    (let ((suffix (substring string point))
>          (completion (try-completion (substring string 0 point) table pred)))
> -    (if (not (stringp completion))
> -        completion
> +    (cond
> +     ((eq completion t)
> +      ;; The prefix is an exact and unique completion, but STRING
> +      ;; might not be a completion.
> +      (if (test-completion string table)
> +          t
> +        (cons string point)))

I think we should test (equal "" suffix) instead.
The cases where (equal "" suffix) is false but (test-completion string table)
are sufficiently weird that returning t seems risky and I can't think of
any reason why it's worthwhile to take this risk.


        Stefan

Message #32 received at 67210 <at> debbugs.gnu.org (full text, mbox):

From: Spencer Baugh <sbaugh <at> janestreet.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 67210 <at> debbugs.gnu.org
Subject: Re: bug#67210: 30.0.50; completing-read with REQUIRE-MATCH=t can
 sometimes return a non-match
Date: Fri, 17 Nov 2023 12:18:23 -0500

[Message part 1 (text/plain, inline)]

Stefan Monnier <monnier <at> iro.umontreal.ca> writes:

>>  (defun completion-emacs22-try-completion (string table pred point)
>>    (let ((suffix (substring string point))
>>          (completion (try-completion (substring string 0 point) table pred)))
>> -    (if (not (stringp completion))
>> -        completion
>> +    (cond
>> +     ((eq completion t)
>> +      ;; The prefix is an exact and unique completion, but STRING
>> +      ;; might not be a completion.
>> +      (if (test-completion string table)
>> +          t
>> +        (cons string point)))
>
> I think we should test (equal "" suffix) instead.
> The cases where (equal "" suffix) is false but (test-completion string table)
> are sufficiently weird that returning t seems risky and I can't think of
> any reason why it's worthwhile to take this risk.

True.  I was thinking about the case where STRING is a completion, so it
would be nice to just return t.  But that would be caught by other
completion styles before emacs22 anyway.

Fixed.

[0001-Return-t-from-completion-emacs22-try-completion-only.patch (text/x-patch, inline)]

From b196a456430ff6ddc4705bc012a14f615b360e37 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh <at> janestreet.com>
Date: Thu, 16 Nov 2023 11:34:08 -0500
Subject: [PATCH] Return t from completion-emacs22-try-completion only for
 completions

The emacs22 completion style ignores the text after point when
computing completions.  However, it still needs to take into account
the entire string it's given, to avoid returning incorrect values.

Previously, completion-emacs22-try-completion would return t if the
text before point was an exact completion.  But this is effectively
saying that the entire input string was an exact completion, which may
not be correct.  This would cause completing-read with REQUIRE-MATCH=t
to return a non-completion.

Now, completion-emacs22-try-completion only returns t if the entire
input string is an exact completion.

* lisp/minibuffer.el (completion-emacs22-try-completion): Return t
only if the entire input string is an exact completion.  (Bug#67210)
---
 lisp/minibuffer.el | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 07a284134d6..5bba202603a 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3526,8 +3526,13 @@ completion-emacs21-all-completions
 (defun completion-emacs22-try-completion (string table pred point)
   (let ((suffix (substring string point))
         (completion (try-completion (substring string 0 point) table pred)))
-    (if (not (stringp completion))
-        completion
+    (cond
+     ((eq completion t)
+      (if (equal "" suffix)
+          t
+        (cons string point)))
+     ((not (stringp completion)) completion)
+     (t
       ;; Merge a trailing / in completion with a / after point.
       ;; We used to only do it for word completion, but it seems to make
       ;; sense for all completions.
@@ -3541,7 +3546,7 @@ completion-emacs22-try-completion
                (eq ?/ (aref suffix 0)))
           ;; This leaves point after the / .
           (setq suffix (substring suffix 1)))
-      (cons (concat completion suffix) (length completion)))))
+      (cons (concat completion suffix) (length completion))))))
 
 (defun completion-emacs22-all-completions (string table pred point)
   (let ((beforepoint (substring string 0 point)))
-- 
2.39.3

Message #37 received at 67210-done <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Spencer Baugh <sbaugh <at> janestreet.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>, 67210-done <at> debbugs.gnu.org
Subject: Re: bug#67210: 30.0.50; completing-read with REQUIRE-MATCH=t can
 sometimes return a non-match
Date: Fri, 17 Nov 2023 12:35:44 -0500

> True.  I was thinking about the case where STRING is a completion, so it
> would be nice to just return t.  But that would be caught by other
> completion styles before emacs22 anyway.

It would also be a strange completion table.

> Fixed.

Thanks, pushed,


        Stefan

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.