bug#23533: 24.5; test-completion with completion-regexp-list

bug#23533: 24.5; test-completion with completion-regexp-list

[ynyaaa]

Calling `test-completion' with non-empty `completion-regexp-list'
may cause an error.

(let ((completion-regexp-list '("")))
 (test-completion "XXX" '(("XXX"))))
error-> Wrong type argument: stringp, ("XXX")




In GNU Emacs 24.5.1 (i686-pc-mingw32)
 of 2015-04-11 on LEG570
Windowing system distributor `Microsoft Corp.', version 6.0.6002
Configured using:
 `configure --prefix=/c/usr --host=i686-pc-mingw32'

Important settings:
  value of $LANG: JPN
  locale-coding-system: cp932

Major mode: Fundamental

Minor modes in effect:
  tooltip-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
  blink-cursor-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Recent messages:

Load-path shadows:
None found.

Features:
(advice rect debug help-mode pp shadow sort gnus-util mail-extr emacsbug
message format-spec rfc822 mml easymenu mml-sec mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader sendmail
rfc2047 rfc2045 ietf-drums mm-util help-fns mail-prsvr mail-utils
time-date japan-util tooltip electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel dos-w32 ls-lisp w32-common-fns disp-table w32-win
w32-vars tool-bar dnd fontset image regexp-opt fringe tabulated-list
newcomment lisp-mode prog-mode register page menu-bar rfn-eshadow timer
select scroll-bar mouse jit-lock font-lock syntax facemenu font-core
frame cham georgian utf-8-lang misc-lang vietnamese tibetan thai
tai-viet lao korean japanese hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese case-table epa-hook jka-cmpr-hook help
simple abbrev minibuffer nadvice loaddefs button faces cus-face macroexp
files text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote make-network-process
w32notify w32 multi-tty emacs)

Memory information:
((conses 8 79804 16234)
 (symbols 32 17797 0)
 (miscs 32 58 258)
 (strings 16 11770 3224)
 (string-bytes 1 298070)
 (vectors 8 10732)
 (vector-slots 4 468507 13202)
 (floats 8 62 414)
 (intervals 28 270 193)
 (buffers 508 20))

bug#23533: 24.5; test-completion with completion-regexp-list

[Noam Postavsky]

found 23533 25.0.95
tag 23533 + confirmed patch
quit

Yup, test-completion doesn't handle testing an alist against
completion-regexp-list. Easy to fix:

diff --git i/src/minibuf.c w/src/minibuf.c
index d85a7a9..74be5d8 100644
--- i/src/minibuf.c
+++ w/src/minibuf.c
@@ -1665,6 +1665,8 @@ DEFUN ("test-completion", Ftest_completion,
Stest_completion, 2, 3, 0,
       tem = Fassoc_string (string, collection, completion_ignore_case
? Qt : Qnil);
       if (NILP (tem))
     return Qnil;
+      else if (CONSP (tem))
+        tem = XCDR (tem);
     }
   else if (VECTORP (collection))
     {

bug#23533: 24.5; test-completion with completion-regexp-list

[Noam Postavsky]

On Fri, Jun 17, 2016 at 12:06 AM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> +      else if (CONSP (tem))
> +        tem = XCDR (tem);

Of course that should be XCAR, not XCDR.

Should this go to master or emacs-25? The fix is simple (though I
managed to get it wrong once) and localized; on the other hand, it's
not a critical bug.

bug#23533: 24.5; test-completion with completion-regexp-list

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Fri, 17 Jun 2016 21:01:01 -0400
> Cc: ynyaaa@gmail.com
> 
> On Fri, Jun 17, 2016 at 12:06 AM, Noam Postavsky
> <npostavs@users.sourceforge.net> wrote:
> > +      else if (CONSP (tem))
> > +        tem = XCDR (tem);
> 
> Of course that should be XCAR, not XCDR.
> 
> Should this go to master or emacs-25? The fix is simple (though I
> managed to get it wrong once) and localized; on the other hand, it's
> not a critical bug.

I'm not sure this is a bug.  But if we want to allow this, the fix
should go to master.

Thanks.

bug#23533: 24.5; test-completion with completion-regexp-list

[Noam Postavsky]

On Sat, Jun 18, 2016 at 4:08 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> I'm not sure this is a bug.

I  think it's pretty clearly a bug, because the docs explain that
test-completion takes the same arguments as all-completions and
try-completions, which do handle an alist COLLECTION with a non-nil
completion-regexp-list.

bug#23533: 24.5; test-completion with completion-regexp-list

[Eli Zaretskii]

> From: Noam Postavsky <npostavs@users.sourceforge.net>
> Date: Sat, 18 Jun 2016 13:14:59 -0400
> Cc: 23533@debbugs.gnu.org, ynyaaa@gmail.com
> 
> On Sat, Jun 18, 2016 at 4:08 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> > I'm not sure this is a bug.
> 
> I  think it's pretty clearly a bug, because the docs explain that
> test-completion takes the same arguments as all-completions and
> try-completions, which do handle an alist COLLECTION with a non-nil
> completion-regexp-list.

When code contradicts the documentation, it's not immediately clear
that the code should be fixed.

Anyway, I don't consider myself an expert on completion, so I feel
uneasy about makings this change without any of the experts voicing
their opinions on the change.

bug#23533: 24.5; test-completion with completion-regexp-list

[Noam Postavsky]

On Sat, Jun 18, 2016 at 1:29 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Noam Postavsky <npostavs@users.sourceforge.net>
>> Date: Sat, 18 Jun 2016 13:14:59 -0400
>> Cc: 23533@debbugs.gnu.org, ynyaaa@gmail.com
>>
>> On Sat, Jun 18, 2016 at 4:08 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> > I'm not sure this is a bug.
>>
>> I  think it's pretty clearly a bug, because the docs explain that
>> test-completion takes the same arguments as all-completions and
>> try-completions, which do handle an alist COLLECTION with a non-nil
>> completion-regexp-list.
>
> When code contradicts the documentation, it's not immediately clear
> that the code should be fixed.

Independently of the documentation, I also think it makes no sense to
have test-completion be inconsistent with all-completions and
try-completion.

>
> Anyway, I don't consider myself an expert on completion, so I feel
> uneasy about makings this change without any of the experts voicing
> their opinions on the change.

Okay, I will wait another week before pushing to master in case anyone
has some objections/comments.

bug#23533: 24.5; test-completion with completion-regexp-list

[Noam Postavsky]

Version: 25.2

On Sat, Jun 18, 2016 at 1:40 PM, Noam Postavsky
<npostavs@users.sourceforge.net> wrote:
> Okay, I will wait another week before pushing to master in case anyone
> has some objections/comments.

Pushed as dd98ee89