GNU bug report logs - #24676
25.1; `completion-pcm-all-completions' should not reverse order of candidates

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 24676 in the body.
You can then email your comments to 24676 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: Drew Adams <drew.adams <at> oracle.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 25.1; `completion-pcm-all-completions' should not reverse order of
 candidates
Date: Wed, 12 Oct 2016 09:56:32 -0700 (PDT)

Because of this part, at the end of the function,
`completion-pcm-all-completions' reverses the order of the candidates.

(let ((poss ()))
  (dolist (c compl)
    (when (string-match-p regex c) (push c poss))) ; <==== reverses
  poss)

It should not do this.  Users should be able to depend on whatever order
the candidates are already in (as returned by `all-completions').

For example, for buffer candidates, the default order from
`all-completions' is last-display-time order.

The fix is trivial:

(let ((poss ()))
  (dolist (c compl)
    (when (string-match-p regex c) (push c poss)))
  (nreverse poss))


In GNU Emacs 25.1.1 (x86_64-w64-mingw32)
 of 2016-09-17
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --without-compress-install CFLAGS=-static'

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 24676 <at> debbugs.gnu.org
Subject: Re: bug#24676: 25.1;
 `completion-pcm-all-completions' should not reverse order of
 candidates
Date: Mon, 17 Oct 2016 10:19:54 +0200

Hello,

> Because of this part, at the end of the function,
> `completion-pcm-all-completions' reverses the order of the candidates.
>
> (let ((poss ()))
>   (dolist (c compl)
>     (when (string-match-p regex c) (push c poss))) ; <==== reverses
>   poss)
>
> It should not do this.  Users should be able to depend on whatever order
> the candidates are already in (as returned by `all-completions').
>
> For example, for buffer candidates, the default order from
> `all-completions' is last-display-time order.
>
> The fix is trivial:
>
> (let ((poss ()))
>   (dolist (c compl)
>     (when (string-match-p regex c) (push c poss)))
>   (nreverse poss))

Ah, this is the reason for the candidate list reversion I see.  Yes,
this should IMO not happen.

Stefan, if you get the time, can you please have a look at whether the
suggested fix looks ok to you, and commit it if applicable?


Thanks,

Michael.

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

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 24676 <at> debbugs.gnu.org
Subject: Re: bug#24676: 25.1;
 `completion-pcm-all-completions' should not reverse order of
 candidates
Date: Mon, 17 Oct 2016 12:29:50 -0400

> Because of this part, at the end of the function,
> `completion-pcm-all-completions' reverses the order of the candidates.

completion-pcm-all-completions doesn't have any such code.
After searching a bit I decided you meant completion-pcm--all-completions.

> (let ((poss ()))
>   (dolist (c compl)
>     (when (string-match-p regex c) (push c poss)))
>   (nreverse poss))

Looks good, please install,


        Stefan

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 24676 <at> debbugs.gnu.org, Drew Adams <drew.adams <at> oracle.com>
Subject: Re: bug#24676: 25.1;
 `completion-pcm-all-completions' should not reverse order of
 candidates
Date: Tue, 18 Oct 2016 10:11:38 +0200

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

> completion-pcm-all-completions doesn't have any such code.  After
> searching a bit I decided you meant completion-pcm--all-completions.

Oh yes, I found that too, but forgot to mention.

>
> > (let ((poss ()))
> >   (dolist (c compl)
> >     (when (string-match-p regex c) (push c poss)))
> >   (nreverse poss))
>
> Looks good, please install,

This doesn't seem to meet any of the criteria in

    http://lists.gnu.org/archive/html/emacs-devel/2016-10/msg00007.html

so I think I'll have to commit to master.


Michael.

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

From: Drew Adams <drew.adams <at> oracle.com>
To: Michael Heerdegen <michael_heerdegen <at> web.de>, Stefan Monnier
 <monnier <at> iro.umontreal.ca>
Cc: 24676 <at> debbugs.gnu.org
Subject: RE: bug#24676: 25.1; `completion-pcm-all-completions' should not
 reverse order of candidates
Date: Tue, 18 Oct 2016 06:56:34 -0700 (PDT)

> > > (let ((poss ()))
> > >   (dolist (c compl)
> > >     (when (string-match-p regex c) (push c poss)))
> > >   (nreverse poss))
> >
> > Looks good, please install,
> 
> This doesn't seem to meet any of the criteria in
>     http://lists.gnu.org/archive/html/emacs-devel/2016-10/msg00007.html
> so I think I'll have to commit to master.

Thanks for fixing this, Michael.

Unfortunately, there is no way for my code that compensates for this bug to now know whether/when it has been fixed.  No simple test such as `fboundp' or an Emacs version.

Of course, if you happened to decide to rename the function, there would be no problem...

And it's already a bit much that there is also a function `completion-pcm-all-completions' that does not even call `completion-pcm--all-completions', and that has a completely different signature.  And the "internal" function has a nice doc string, but the "external" one has no doc string...

Perhaps you would consider taking this opportunity to find a better name for this "internal" function?  Especially since it is "internal", probably few 3rd-party libraries would be affected by a name change.

In my case, I would not be referencing this "internal" function at all, except that I needed to work around this bug.  If you did choose to rename it, I would not need to make any code changes, because I already protect this change with `(when (fboundp 'completion-pcm--all-completions)...)'.

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

From: Michael Heerdegen <michael_heerdegen <at> web.de>
To: Drew Adams <drew.adams <at> oracle.com>
Cc: 24676 <at> debbugs.gnu.org, Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#24676: 25.1;
 `completion-pcm-all-completions' should not reverse order of
 candidates
Date: Tue, 18 Oct 2016 16:22:31 +0200

Drew Adams <drew.adams <at> oracle.com> writes:

> Unfortunately, there is no way for my code that compensates for this
> bug to now know whether/when it has been fixed.  No simple test such
> as `fboundp' or an Emacs version.
>
> Of course, if you happened to decide to rename the function, there
> would be no problem...

Hmm, that sounds reasonable.  If nobody objects, I can do that, sure.


Michael.

Previous Next

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