GNU bug report logs - #64351
probabe bug associated with “completion-regexp-list”

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 64351 in the body.
You can then email your comments to 64351 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: Shynur Xie <one.last.kiss <at> outlook.com>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: probabe bug associated with “completion-regexp-list”
Date: Thu, 29 Jun 2023 18:53:28 +0000

Evaluate this:

    (setq completion-regexp-list '("^[^-]"))

Then type “C-h v -l TAB”, an error will be thrown.

--
shynur

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Shynur Xie <one.last.kiss <at> outlook.com>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 64351 <at> debbugs.gnu.org
Subject: Re: bug#64351: probabe bug associated with
 “completion-regexp-list”
Date: Fri, 30 Jun 2023 09:21:02 +0300

> From: Shynur Xie <one.last.kiss <at> outlook.com>
> Date: Thu, 29 Jun 2023 18:53:28 +0000

(Please always state at least in which version of Emacs on what
platform did the problem occurred.  It is best to use the dedicated
command report-emacs-bug, which will collect the important details for
you.  This avoids the need to guess whether the issue is relevant to
Emacs 29 and trying several different versions to see if they
reproduce the problem.)

> Evaluate this:
> 
>     (setq completion-regexp-list '("^[^-]"))
> 
> Then type “C-h v -l TAB”, an error will be thrown.

Stefan, is the below the right fix?  If it is, do you think it is safe
enough for the release branch?

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 4aa1ab3..3e30b68 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4027,7 +4027,8 @@ completion-pcm--merge-completions
               (setq ccs (nreverse ccs))
               (let* ((prefix (try-completion fixed comps))
                      (unique (or (and (eq prefix t) (setq prefix fixed))
-                                 (eq t (try-completion prefix comps)))))
+                                 (and (stringp prefix)
+                                      (eq t (try-completion prefix comps))))))
                 (unless (or (eq elem 'prefix)
                             (equal prefix ""))
                   (push prefix res))

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Shynur Xie <one.last.kiss <at> outlook.com>, 64351 <at> debbugs.gnu.org
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 08:51:18 -0400

> Stefan, is the below the right fix?  If it is, do you think it is safe
> enough for the release branch?

I haven't had time to investigate if it's really The Right Fix, but it's
definitely a safe fix, yes.


        Stefan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: one.last.kiss <at> outlook.com, 64351 <at> debbugs.gnu.org
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 16:15:43 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Shynur Xie <one.last.kiss <at> outlook.com>,  64351 <at> debbugs.gnu.org
> Date: Fri, 30 Jun 2023 08:51:18 -0400
> 
> > Stefan, is the below the right fix?  If it is, do you think it is safe
> > enough for the release branch?
> 
> I haven't had time to investigate if it's really The Right Fix, but it's
> definitely a safe fix, yes.

Thanks.

The alternative would be to treat the nil return value from
try-completion here:

              (let* ((prefix (try-completion fixed comps)) <<<<<<<<<<<<<<<
                     (unique (or (and (eq prefix t) (setq prefix fixed))
                                 (and (stringp prefix)
                                      (eq t (try-completion prefix comps))))))
                (unless (or (eq elem 'prefix)
                            (equal prefix ""))
                  (push prefix res))

as if it were "".  Would that be better?

With the change I proposed, "C-h v -l TAB" deletes "-l" that the user
typed -- is that the expected result when there are no completions?

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

From: Shynur Xie <one.last.kiss <at> outlook.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: "64351 <at> debbugs.gnu.org" <64351 <at> debbugs.gnu.org>
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 14:17:59 +0000

Hi Eli, there’s another issue about “completion-regexp-list”.

I launched Emacs with “-Q” and evaluate:

    (advice-add 'describe-variable :around ;“C-h v”
                (lambda (advice-added-function &rest arguments)
                  (let ((completion-regexp-list '("^\\([^-]*$\\|\\([^-]+\\(-[^-]+\\)-?\\)\\)$")))
                    (apply advice-added-function arguments))))

to filter out symbols “*--*”.

Then type “C-h v advice TAB”, I can still see some variables whose names are
“*--*” (e.g., “advice--bytecodes”).  Though “string-match-p” says that
name doesn’t match:

    ELISP> (string-match-p "^\\([^-]*$\\|\\([^-]+\\(-[^-]+\\)-?\\)\\)$" "advice--bytecodes")
    nil

____________________

Emacs 28.2 on MS-Windows

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

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Shynur Xie <one.last.kiss <at> outlook.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "64351 <at> debbugs.gnu.org" <64351 <at> debbugs.gnu.org>
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 16:27:34 +0200

On Jun 30 2023, Shynur Xie wrote:

> I launched Emacs with “-Q” and evaluate:
>
>     (advice-add 'describe-variable :around ;“C-h v”
>                 (lambda (advice-added-function &rest arguments)
>                   (let ((completion-regexp-list '("^\\([^-]*$\\|\\([^-]+\\(-[^-]+\\)-?\\)\\)$")))
>                     (apply advice-added-function arguments))))
>
> to filter out symbols “*--*”.
>
> Then type “C-h v advice TAB”,

Interactive completion runs before the function is called, inside
call-interactively.  You need to advice the interactive spec.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

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

From: Shynur Xie <one.last.kiss <at> outlook.com>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "64351 <at> debbugs.gnu.org" <64351 <at> debbugs.gnu.org>
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 15:23:45 +0000

> Andreas> To: Shynur
> Andreas> Interactive completion runs before the function is called,
> Andreas> inside call-interactively.  You need to advice the
> Andreas> interactive spec.

Thanks!

This is a tricky issue to me, because I only want to filter when
calling “describe-variable”.  I guess I have to give up this idea...

No further question now.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: Shynur Xie <one.last.kiss <at> outlook.com>, 64351 <at> debbugs.gnu.org
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 14:33:49 -0400

>> Evaluate this:
>>     (setq completion-regexp-list '("^[^-]"))

Bad idea.
Admittedly, `completion-regexp-list` is documented poorly enough that
it's not clear what usage is valid and what isn't, but a lot of
completion code will be broken if you set it globally to anything else
than nil.

>> Then type “C-h v -l TAB”, an error will be thrown.
> Stefan, is the below the right fix?  If it is, do you think it is safe
> enough for the release branch?
>
> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index 4aa1ab3..3e30b68 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -4027,7 +4027,8 @@ completion-pcm--merge-completions
>                (setq ccs (nreverse ccs))
>                (let* ((prefix (try-completion fixed comps))
>                       (unique (or (and (eq prefix t) (setq prefix fixed))
> -                                 (eq t (try-completion prefix comps)))))
> +                                 (and (stringp prefix)
> +                                      (eq t (try-completion prefix comps))))))
>                  (unless (or (eq elem 'prefix)
>                              (equal prefix ""))
>                    (push prefix res))

I'd argue that the better fix is to let-bind `completion-regexp-list` to
nil around the `minibuffer.el` code which uses
`try/all/test-completion` to protect against those kinds of misuses.
Or alternatively to say "if it hurts, don't do that".


        Stefan

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

From: Shynur Xie <one.last.kiss <at> outlook.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>, Eli Zaretskii <eliz <at> gnu.org>
Cc: "64351 <at> debbugs.gnu.org" <64351 <at> debbugs.gnu.org>
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 18:45:04 +0000

> Shynur> Evaluate this:
> Shynur>     (setq completion-regexp-list '("^[^-]"))
> Stefan> Bad idea.

I have to say that it is not a bad idea to write this _meaningless_
and _short_ expression.  I just want to find a short enough expression
to show the issue.  If I post a _long_ code snippet and say “Hey,
there’s a bug”, nobody wants to see it. :-)

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Shynur Xie <one.last.kiss <at> outlook.com>
Cc: Eli Zaretskii <eliz <at> gnu.org>,
 "64351 <at> debbugs.gnu.org" <64351 <at> debbugs.gnu.org>
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 15:02:07 -0400

>> Shynur> Evaluate this:
>> Shynur>     (setq completion-regexp-list '("^[^-]"))
>> Stefan> Bad idea.
>
> I have to say that it is not a bad idea to write this _meaningless_
> and _short_ expression.  I just want to find a short enough expression
> to show the issue.

FWIW, the main problem is setting this var while minibuffer.el functions
are called (so setting globally is definitely a problem).
This var is usually expected to be let-bound next to your own calls to
`try/all/test-completion`.

But as I say, this is actually not documented, it's just what I have
found to be general assumptions is various packages that implement
completion UIs (including the one I wrote :-)

> If I post a _long_ code snippet and say “Hey,
> there’s a bug”, nobody wants to see it. :-)

I know what you men.  I'd be interested to hear about your
use-case, tho.  Maybe I can provide a good alternative.


        Stefan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: one.last.kiss <at> outlook.com, 64351 <at> debbugs.gnu.org
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 22:08:29 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Shynur Xie <one.last.kiss <at> outlook.com>,  64351 <at> debbugs.gnu.org
> Date: Fri, 30 Jun 2023 14:33:49 -0400
> 
> >> Evaluate this:
> >>     (setq completion-regexp-list '("^[^-]"))
> 
> Bad idea.
> Admittedly, `completion-regexp-list` is documented poorly enough that
> it's not clear what usage is valid and what isn't, but a lot of
> completion code will be broken if you set it globally to anything else
> than nil.

Would you mind telling more about valid uses of this?  I'd like to
improve the documentation.

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

From: Shynur Xie <one.last.kiss <at> outlook.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: "64351 <at> debbugs.gnu.org" <64351 <at> debbugs.gnu.org>
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 19:36:44 +0000

> Stefan> I'd be interested to hear about your use-case, tho.

I tried to filter out internal variables “*--*” when typing “C-h v”
several days ago, so I wrote this:

(advice-add 'describe-variable :around ;“C-h v”
            (lambda (advice-added-function &rest arguments)
              (let ((completion-regexp-list
                     '("^\\([^-]*$\\|\\([^-]+\\(-[^-]+\\)-?\\)\\)$")))
                (apply advice-added-function arguments))))

It didn’t work.  I wanted to find out why, so to save time, I set its
value globally:

(setq completion-regexp-list
                      '("^\\([^-]*$\\|\\([^-]+\\(-[^-]+\\)-?\\)\\)$"))

Somehow, Emacs threw an error.  I guess this is a bug, so I tried to
narrow it down and then reported it.

> Stefan> Maybe I can provide a good alternative.

I think it would be nice if Emacs can provides an option, say,
“completions-filter-function” (or “completions-help-filter-function”
only for functions “describe-*”), which receives 2 arguments -- TYPE
and COMPLETION.  If I evaluate:

(setq completions-filter-function
        (lambda (type completion)
          (pcase type
            ('variable
             (not (string-match-p "--" (symbol-name completion)))))))

I can filter out “*--*” variables.  It’s also easier to add other
rules than the original “completion-regexp-list”.  Using regexp to
filter is not very convenient, say if I want to filter out
“*-internal”, I have to write this regexp:

"\\([^l]\\|[^a]l\\|[^n]al\\|[^r]nal\\|[^e]rnal\\|[^t]ernal\\|[^n]ternal\\|[^i]nternal\\|[^-]internal\\)$"

I don’t know whether “completions-filter-function” is a good idea, I’m
just making a suggestion.  Thanks.

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

From: Shynur Xie <one.last.kiss <at> outlook.com>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: "64351 <at> debbugs.gnu.org" <64351 <at> debbugs.gnu.org>
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 19:52:45 +0000

> Shynur> To: Stefan
> Shynur> (setq completions-filter-function
> Shynur>        (lambda (type completion)
> Shynur>          (pcase type
> Shynur>            ('variable
> Shynur>             (not (string-match-p "--" (symbol-name completion)))))))

Should be

(pcase type
  (...)
  (_ t))

Sorry for this typo.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Shynur Xie <one.last.kiss <at> outlook.com>
Cc: "64351 <at> debbugs.gnu.org" <64351 <at> debbugs.gnu.org>
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Fri, 30 Jun 2023 19:24:00 -0400

> (setq completions-filter-function
>         (lambda (type completion)
>           (pcase type
>             ('variable
>              (not (string-match-p "--" (symbol-name completion)))))))

Are you looking for `minibuffer-completion-predicate`, maybe?


        Stefan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: one.last.kiss <at> outlook.com, 64351 <at> debbugs.gnu.org
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Sat, 01 Jul 2023 11:21:07 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: Shynur Xie <one.last.kiss <at> outlook.com>,  64351 <at> debbugs.gnu.org
> Date: Fri, 30 Jun 2023 14:33:49 -0400
> 
> > Stefan, is the below the right fix?  If it is, do you think it is safe
> > enough for the release branch?
> >
> > diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> > index 4aa1ab3..3e30b68 100644
> > --- a/lisp/minibuffer.el
> > +++ b/lisp/minibuffer.el
> > @@ -4027,7 +4027,8 @@ completion-pcm--merge-completions
> >                (setq ccs (nreverse ccs))
> >                (let* ((prefix (try-completion fixed comps))
> >                       (unique (or (and (eq prefix t) (setq prefix fixed))
> > -                                 (eq t (try-completion prefix comps)))))
> > +                                 (and (stringp prefix)
> > +                                      (eq t (try-completion prefix comps))))))
> >                  (unless (or (eq elem 'prefix)
> >                              (equal prefix ""))
> >                    (push prefix res))
> 
> I'd argue that the better fix is to let-bind `completion-regexp-list` to
> nil around the `minibuffer.el` code which uses
> `try/all/test-completion` to protect against those kinds of misuses.

You are saying that you don't like my proposed patch for the emacs-29
branch, and would even prefer to leave it unfixed?

The binding you propose might be appropriate for master, but not for
the release branch.

> Or alternatively to say "if it hurts, don't do that".

That could be okay, if we also say how to use this variable so it
doesn't hurt.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: one.last.kiss <at> outlook.com, 64351 <at> debbugs.gnu.org
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Sat, 01 Jul 2023 10:19:50 -0400

>> Bad idea.
>> Admittedly, `completion-regexp-list` is documented poorly enough that
>> it's not clear what usage is valid and what isn't, but a lot of
>> completion code will be broken if you set it globally to anything else
>> than nil.
>
> Would you mind telling more about valid uses of this?
> I'd like to improve the documentation.

Here's what I know: `completion-regexp-list` used to be virtually unused
in Emacs's code base, so I don't know what was its intended use.

But when completion UIs started to offer other styles than just
prefix-based completion, they also started to use
`try/all/test-completion` more "internally" (i.e. for intermediate
computations rather than to compute the final list of completions
displayed to the user) and they did it without paying attention to
`completion-regexp-list`, i.e. in ways which tended to just misbehave
when `completion-regexp-list` wasn't nil.

So, in practice nowadays, the only reliable way to use
`completion-regexp-list` is to think of it as an optional argument to
`try/all/test-completion`, but one that has to be provided via a nearby
let-binding rather than by listing it explicitly at the call site.

> You are saying that you don't like my proposed patch for the emacs-29
> branch, and would even prefer to leave it unfixed?

Your patch is harmless, IMO, so it's OK to install it.

> The binding you propose might be appropriate for master, but not for
> the release branch.

Agreed.

> > Or alternatively to say "if it hurts, don't do that".
> That could be okay, if we also say how to use this variable so it
> doesn't hurt.

My opinion is that we should document that variable as something that's
not meant to be used/set by users, but only one that should be set by
the code that calls `try/all/test-completion`.


        Stefan

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: one.last.kiss <at> outlook.com, 64351-done <at> debbugs.gnu.org
Subject: Re: bug#64351: probabe bug associated with “completion-regexp-list”
Date: Sun, 02 Jul 2023 09:18:54 +0300

> From: Stefan Monnier <monnier <at> iro.umontreal.ca>
> Cc: one.last.kiss <at> outlook.com,  64351 <at> debbugs.gnu.org
> Date: Sat, 01 Jul 2023 10:19:50 -0400
> 
> > Would you mind telling more about valid uses of this?
> > I'd like to improve the documentation.
> 
> Here's what I know: `completion-regexp-list` used to be virtually unused
> in Emacs's code base, so I don't know what was its intended use.
> 
> But when completion UIs started to offer other styles than just
> prefix-based completion, they also started to use
> `try/all/test-completion` more "internally" (i.e. for intermediate
> computations rather than to compute the final list of completions
> displayed to the user) and they did it without paying attention to
> `completion-regexp-list`, i.e. in ways which tended to just misbehave
> when `completion-regexp-list` wasn't nil.
> 
> So, in practice nowadays, the only reliable way to use
> `completion-regexp-list` is to think of it as an optional argument to
> `try/all/test-completion`, but one that has to be provided via a nearby
> let-binding rather than by listing it explicitly at the call site.

Thanks, I added something along these lines to the documentation.

> > You are saying that you don't like my proposed patch for the emacs-29
> > branch, and would even prefer to leave it unfixed?
> 
> Your patch is harmless, IMO, so it's OK to install it.

Installed on the emacs-29 branch, and closing the bug.

Previous Next

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