bug#24966: 26.0.50;

bug#24966: 26.0.50;

[Mark Oteiza]

Hi,

dd98ee8992c3246861e44 appears to be the culprit for breaking the
following from -Q:

1. C-x C-f /tmp/foo.tex
2. M-x reftex-mode
3. Insert the following: \label{foobar}
4. M-x reftex-goto-label

and, when attempting to complete, a wrong-type-argument error occurs.

Debugger entered--Lisp error: (wrong-type-argument listp "argh")
  #[(x) "\b@;\207" [x] 1]("argh")
  test-completion("argh" ((xr nil "\\\\\\\\\\\\") (index-tags) (is-multi nil) (bibview-cache) (master-dir . "/tmp/") (label-numbers) (bof "/tmp/lol.tex") ("argh" "s" " " "/tmp/lol.tex" nil) (eof "/tmp/lol.tex")) #[(x) "\b@;\207" [x] 1])
  completion--do-completion(8 8)
  completion--in-region-1(8 8)
  #[1028 "\x01\x19\n\203!\304!\203\0\202\0\305!\305\306\"\x03F\307\310!\210\311\x04\"*\207" [minibuffer-completion-predicate minibuffer-completion-table completion-in-region-mode-predicate completion-in-region--data markerp copy-marker t completion-in-region-mode 1 completion--in-region-1] 8 "\n\n(fn START END COLLECTION PREDICATE)"](8 8 ((xr nil "\\\\\\\\\\\\") (index-tags) (is-multi nil) (bibview-cache) (master-dir . "/tmp/") (label-numbers) (bof "/tmp/lol.tex") ("argh" "s" " " "/tmp/lol.tex" nil) (eof "/tmp/lol.tex")) #[(x) "\b@;\207" [x] 1])
  apply(#[1028 "\x01\x19\n\203!\304!\203\0\202\0\305!\305\306\"\x03F\307\310!\210\311\x04\"*\207" [minibuffer-completion-predicate minibuffer-completion-table completion-in-region-mode-predicate completion-in-region--data markerp copy-marker t completion-in-region-mode 1 completion--in-region-1] 8 "\n\n(fn START END COLLECTION PREDICATE)"] (8 8 ((xr nil "\\\\\\\\\\\\") (index-tags) (is-multi nil) (bibview-cache) (master-dir . "/tmp/") (label-numbers) (bof "/tmp/lol.tex") ("argh" "s" " " "/tmp/lol.tex" nil) (eof "/tmp/lol.tex")) #[(x) "\b@;\207" [x] 1]))
  #[771 ":\2030\x02@\301=\203\0\300\242\302\x05A\"\303#\207\304@\305\306\307\310\311\312\300!\313\"\314\315%\x06A\x06##\207\304\316\"\207" [(#0) t append nil apply apply-partially make-byte-code 642 "\300\242\x03#\207" vconcat vector [] 7 "\n\n(fn FUNS GLOBAL &rest ARGS)" #[1028 "\x01\x19\n\203!\304!\203\0\202\0\305!\305\306\"\x03F\307\310!\210\311\x04\"*\207" [minibuffer-completion-predicate minibuffer-completion-table completion-in-region-mode-predicate completion-in-region--data markerp copy-marker t completion-in-region-mode 1 completion--in-region-1] 8 "\n\n(fn START END COLLECTION PREDICATE)"]] 12 "\n\n(fn FUNS GLOBAL ARGS)"](nil nil (8 8 ((xr nil "\\\\\\\\\\\\") (index-tags) (is-multi nil) (bibview-cache) (master-dir . "/tmp/") (label-numbers) (bof "/tmp/lol.tex") ("argh" "s" " " "/tmp/lol.tex" nil) (eof "/tmp/lol.tex")) #[(x) "\b@;\207" [x] 1]))
  completion--in-region(8 8 ((xr nil "\\\\\\\\\\\\") (index-tags) (is-multi nil) (bibview-cache) (master-dir . "/tmp/") (label-numbers) (bof "/tmp/lol.tex") ("argh" "s" " " "/tmp/lol.tex" nil) (eof "/tmp/lol.tex")) #[(x) "\b@;\207" [x] 1])
  completion-in-region(8 8 ((xr nil "\\\\\\\\\\\\") (index-tags) (is-multi nil) (bibview-cache) (master-dir . "/tmp/") (label-numbers) (bof "/tmp/lol.tex") ("argh" "s" " " "/tmp/lol.tex" nil) (eof "/tmp/lol.tex")) #[(x) "\b@;\207" [x] 1])
  minibuffer-complete()
  funcall-interactively(minibuffer-complete)
  call-interactively(minibuffer-complete nil nil)
  command-execute(minibuffer-complete)
  read-from-minibuffer("Label: " nil (keymap (10 . minibuffer-complete-and-exit) (13 . minibuffer-complete-and-exit) keymap (menu-bar keymap (minibuf "Minibuf" keymap (tab menu-item "Complete" minibuffer-complete :help "Complete as far as possible") (space menu-item "Complete Word" minibuffer-complete-word :help "Complete at most one word") (63 menu-item "List Completions" minibuffer-completion-help :help "Display all possible completions") "Minibuf")) (27 keymap (118 . switch-to-completions)) (prior . switch-to-completions) (63 . minibuffer-completion-help) (32 . minibuffer-complete-word) (9 . minibuffer-complete) keymap (menu-bar keymap (minibuf "Minibuf" keymap (previous menu-item "Previous History Item" previous-history-element :help "Put previous minibuffer history element in the minibuffer") (next menu-item "Next History Item" next-history-element :help "Put next minibuffer history element in the minibuffer") (isearch-backward menu-item "Isearch History Backward" isearch-backward :help "Incrementally search minibuffer history backward") (isearch-forward menu-item "Isearch History Forward" isearch-forward :help "Incrementally search minibuffer history forward") (return menu-item "Enter" exit-minibuffer :key-sequence "" :help "Terminate input and exit minibuffer") (quit menu-item "Quit" abort-recursive-edit :help "Abort input and exit minibuffer") "Minibuf")) (10 . exit-minibuffer) (13 . exit-minibuffer) (7 . abort-recursive-edit) (C-tab . file-cache-minibuffer-complete) (9 . self-insert-command) (XF86Back . previous-history-element) (up . previous-line-or-history-element) (prior . previous-history-element) (XF86Forward . next-history-element) (down . next-line-or-history-element) (next . next-history-element) (27 keymap (114 . previous-matching-history-element) (115 . next-matching-history-element) (112 . previous-history-element) (110 . next-history-element))) nil nil nil nil)
  completing-read-default("Label: " ((xr nil "\\\\\\\\\\\\") (index-tags) (is-multi nil) (bibview-cache) (master-dir . "/tmp/") (label-numbers) (bof "/tmp/lol.tex") ("argh" "s" " " "/tmp/lol.tex" nil) (eof "/tmp/lol.tex")) #[(x) "\b@;\207" [x] 1] t nil nil nil nil)
  completing-read("Label: " ((xr nil "\\\\\\\\\\\\") (index-tags) (is-multi nil) (bibview-cache) (master-dir . "/tmp/") (label-numbers) (bof "/tmp/lol.tex") ("argh" "s" " " "/tmp/lol.tex" nil) (eof "/tmp/lol.tex")) #[(x) "\b@;\207" [x] 1] t nil nil nil)
  reftex-goto-label(nil)
  funcall-interactively(reftex-goto-label nil)
  call-interactively(reftex-goto-label record nil)
  command-execute(reftex-goto-label record)
  execute-extended-command(nil "reftex-goto-label" nil)
  funcall-interactively(execute-extended-command nil "reftex-goto-label" nil)
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)


In GNU Emacs 26.0.50.2 (x86_64-unknown-linux-gnu, X toolkit, Xaw scroll bars)
 of 2016-11-18 built on logos
Repository revision: 6cdd1c333034b308e74e70c4fd10399fbb5329b9

bug#24966: 26.0.50;

[npostavs]

unarchive 23533
# It was marked for 25.2 before master was renumbered to 26
notfixed 23533 25.2
fixed 23533 26.0.50
retitle 24966 26.0.50; test-completion with alist COLLECTION calls PREDICATE incorrectly
tags 24966 confirmed
quit

Mark Oteiza <mvoteiza@udel.edu> writes:

> Hi,
>
> dd98ee8992c3246861e44 appears to be the culprit for breaking the
> following from -Q:

Yup, I didn't fix test-completion correctly.

>
> 1. C-x C-f /tmp/foo.tex
> 2. M-x reftex-mode
> 3. Insert the following: \label{foobar}
> 4. M-x reftex-goto-label
>
> and, when attempting to complete, a wrong-type-argument error occurs.
>
> Debugger entered--Lisp error: (wrong-type-argument listp "argh")
>   #[(x) "\b@;\207" [x] 1]("argh")
>   test-completion("argh" ((xr nil "\\\\\\\\\\\\") (index-tags) (is-multi nil) (bibview-cache) (master-dir . "/tmp/") (label-numbers) (bof "/tmp/lol.tex") ("argh" "s" " " "/tmp/lol.tex" nil) (eof "/tmp/lol.tex")) #[(x) "\b@;\207" [x] 1])

I tentatively suggest the patch below, but I want to add some tests
before commiting anything.

diff --git i/src/minibuf.c w/src/minibuf.c
index 57eea05..fe187b3 100644
--- i/src/minibuf.c
+++ w/src/minibuf.c
@@ -1686,8 +1686,6 @@ 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 = XCAR (tem);
     }
   else if (VECTORP (collection))
     {
@@ -1765,14 +1763,16 @@ DEFUN ("test-completion", Ftest_completion, Stest_completion, 2, 3, 0,
   /* Reject this element if it fails to match all the regexps.  */
   if (CONSP (Vcompletion_regexp_list))
     {
+      Lisp_Object temstr = (CONSP (tem)) ? XCAR (tem) : tem;
+      if (SYMBOLP (temstr))
+        temstr = string;
+
       ptrdiff_t count = SPECPDL_INDEX ();
       specbind (Qcase_fold_search, completion_ignore_case ? Qt : Qnil);
       for (regexps = Vcompletion_regexp_list; CONSP (regexps);
 	   regexps = XCDR (regexps))
 	{
-	  if (NILP (Fstring_match (XCAR (regexps),
-				   SYMBOLP (tem) ? string : tem,
-				   Qnil)))
+	  if (NILP (Fstring_match (XCAR (regexps), temstr, Qnil)))
 	    return unbind_to (count, Qnil);
 	}
       unbind_to (count, Qnil);

bug#24966: 26.0.50;

[Mark Oteiza]

On 20/11/16 at 11:24pm, npostavs@users.sourceforge.net wrote:
> unarchive 23533
> # It was marked for 25.2 before master was renumbered to 26
> notfixed 23533 25.2
> fixed 23533 26.0.50
> retitle 24966 26.0.50; test-completion with alist COLLECTION calls PREDICATE incorrectly
> tags 24966 confirmed
> quit
> 
> Mark Oteiza <mvoteiza@udel.edu> writes:
> 
> > Hi,
> >
> > dd98ee8992c3246861e44 appears to be the culprit for breaking the
> > following from -Q:
> 
> Yup, I didn't fix test-completion correctly.
> 
> >
> > 1. C-x C-f /tmp/foo.tex
> > 2. M-x reftex-mode
> > 3. Insert the following: \label{foobar}
> > 4. M-x reftex-goto-label
> >
> > and, when attempting to complete, a wrong-type-argument error occurs.
> >
> > Debugger entered--Lisp error: (wrong-type-argument listp "argh")
> >   #[(x) "\b@;\207" [x] 1]("argh")
> >   test-completion("argh" ((xr nil "\\\\\\\\\\\\") (index-tags) (is-multi nil) (bibview-cache) (master-dir . "/tmp/") (label-numbers) (bof "/tmp/lol.tex") ("argh" "s" " " "/tmp/lol.tex" nil) (eof "/tmp/lol.tex")) #[(x) "\b@;\207" [x] 1])
> 
> I tentatively suggest the patch below, but I want to add some tests
> before commiting anything.
> 
> diff --git i/src/minibuf.c w/src/minibuf.c
> index 57eea05..fe187b3 100644
> --- i/src/minibuf.c
> +++ w/src/minibuf.c
> @@ -1686,8 +1686,6 @@ 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 = XCAR (tem);
>      }
>    else if (VECTORP (collection))
>      {
> @@ -1765,14 +1763,16 @@ DEFUN ("test-completion", Ftest_completion, Stest_completion, 2, 3, 0,
>    /* Reject this element if it fails to match all the regexps.  */
>    if (CONSP (Vcompletion_regexp_list))
>      {
> +      Lisp_Object temstr = (CONSP (tem)) ? XCAR (tem) : tem;
> +      if (SYMBOLP (temstr))
> +        temstr = string;
> +
>        ptrdiff_t count = SPECPDL_INDEX ();
>        specbind (Qcase_fold_search, completion_ignore_case ? Qt : Qnil);
>        for (regexps = Vcompletion_regexp_list; CONSP (regexps);
>  	   regexps = XCDR (regexps))
>  	{
> -	  if (NILP (Fstring_match (XCAR (regexps),
> -				   SYMBOLP (tem) ? string : tem,
> -				   Qnil)))
> +	  if (NILP (Fstring_match (XCAR (regexps), temstr, Qnil)))
>  	    return unbind_to (count, Qnil);
>  	}
>        unbind_to (count, Qnil);
> 

Thanks, this does indeed fix the recipe.

bug#24966: 26.0.50; test-completion with alist COLLECTION calls PREDICATE incorrectly

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 472 bytes --]

tags 24966 patch
quit

npostavs@users.sourceforge.net writes:
>
> I tentatively suggest the patch below, but I want to add some tests
> before commiting anything.

While adding tests, I found another inconsistency: when given a
hashtable with symbol keys, test-completion passes the symbol-name to
PREDICATE, while all-completions and try-completion pass the original
symbol key.  Here are two patches, the first for this bug, and the
second for the other inconsistency.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 20004 bytes --]

From ba3f1477e66536fd759c4f7128bfbca009532b14 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 27 Nov 2016 10:04:48 -0500
Subject: [PATCH v1 1/2] Give test-completion's PREDICATE full alist entry

Since 2016-06-26 "Fix test-completion with completion-regexp-list", when
calling test-completion with an alist collection, the predicate was
recieving the string value instead of the alist entry (Bug#24966).

* src/minibuf.c (Ftest_completion): Don't modify the found element, just
test STRING against `completion-regexp-list'.
* test/src/minibuf-tests.el: New tests for `try-completion',
`all-completions', and `test-completion'.
---
 src/minibuf.c             |   8 +-
 test/src/minibuf-tests.el | 406 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 409 insertions(+), 5 deletions(-)
 create mode 100644 test/src/minibuf-tests.el

diff --git a/src/minibuf.c b/src/minibuf.c
index 57eea05..6c694cb 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1686,8 +1686,6 @@ 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 = XCAR (tem);
     }
   else if (VECTORP (collection))
     {
@@ -1770,9 +1768,9 @@ DEFUN ("test-completion", Ftest_completion, Stest_completion, 2, 3, 0,
       for (regexps = Vcompletion_regexp_list; CONSP (regexps);
 	   regexps = XCDR (regexps))
 	{
-	  if (NILP (Fstring_match (XCAR (regexps),
-				   SYMBOLP (tem) ? string : tem,
-				   Qnil)))
+          /* We can test against STRING, because if we got here, then
+             the element is equivalent to it.  */
+          if (NILP (Fstring_match (XCAR (regexps), string, Qnil)))
 	    return unbind_to (count, Qnil);
 	}
       unbind_to (count, Qnil);
diff --git a/test/src/minibuf-tests.el b/test/src/minibuf-tests.el
new file mode 100644
index 0000000..98b8614
--- /dev/null
+++ b/test/src/minibuf-tests.el
@@ -0,0 +1,406 @@
+;;; minibuf-tests.el --- tests for minibuf.c functions -*- lexical-binding: t -*-
+
+;; Copyright (C) 2016 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+
+\f
+;;; Support functions for `try-completion', `all-completion', and
+;;; `test-completion' tests.
+
+(defun minibuf-tests--strings-to-symbol-list (list)
+  (mapcar #'intern list))
+(defun minibuf-tests--strings-to-symbol-alist (list)
+  (let ((num 0))
+    (mapcar (lambda (str) (cons (intern str) (cl-incf num))) list)))
+(defun minibuf-tests--strings-to-string-alist (list)
+  (let ((num 0))
+    (mapcar (lambda (str) (cons str (cl-incf num))) list)))
+(defun minibuf-tests--strings-to-obarray (list)
+  (let ((ob (make-vector 7 0)))
+    (mapc (lambda (str) (intern str ob)) list)
+    ob))
+(defun minibuf-tests--strings-to-string-hashtable (list)
+  (let ((ht (make-hash-table :test #'equal))
+        (num 0))
+    (mapc (lambda (str) (puthash str (cl-incf num) ht)) list)
+    ht))
+(defun minibuf-tests--strings-to-symbol-hashtable (list)
+  (let ((ht (make-hash-table :test #'equal))
+        (num 0))
+    (mapc (lambda (str) (puthash (intern str) (cl-incf num) ht)) list)
+    ht))
+
+;;; Functions that produce a predicate (for *-completion functions)
+;;; which always returns non-nil for a given collection.
+
+(defun minibuf-tests--memq-of-collection (collection)
+  (lambda (elt) (memq elt collection)))
+(defun minibuf-tests--part-of-obarray (ob)
+  (lambda (sym) (eq (intern-soft (symbol-name sym) ob) sym)))
+(defun minibuf-tests--part-of-hashtable (table)
+  (lambda (k v) (equal (gethash k table) v)))
+
+\f
+;;; Testing functions that are agnostic to type of COLLECTION.
+
+(defun minibuf-tests--try-completion (xform-collection)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (should (equal (try-completion "a" abcdef) "abc"))
+    (should (equal (try-completion "a" +abba) "ab"))
+    (should (equal (try-completion "abc" +abba) t))
+    (should (equal (try-completion "abcd" +abba) nil))))
+
+(defun minibuf-tests--try-completion-pred (xform-collection collection-member)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (abcdef-member (funcall collection-member abcdef))
+         (+abba  (funcall xform-collection '("abc" "abba" "def")))
+         (+abba-member (funcall collection-member +abba)))
+    (should (equal (try-completion "a" abcdef abcdef-member) "abc"))
+    (should (equal (try-completion "a" +abba +abba-member) "ab"))
+    (should (equal (try-completion "abc" +abba +abba-member) t))
+    (should (equal (try-completion "abcd" +abba +abba-member) nil))
+    (should-not (try-completion "a" abcdef #'ignore))
+    (should-not (try-completion "a" +abba #'ignore))
+    (should-not (try-completion "abc" +abba #'ignore))
+    (should-not (try-completion "abcd" +abba #'ignore))))
+
+(defun minibuf-tests--try-completion-regexp (xform-collection)
+  (let ((abcdef (funcall xform-collection '("abc" "def")))
+        (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (let ((completion-regexp-list '(".")))
+      (should (equal (try-completion "a" abcdef) "abc"))
+      (should (equal (try-completion "a" +abba) "ab"))
+      (should (equal (try-completion "abc" +abba) t))
+      (should (equal (try-completion "abcd" +abba) nil)))
+    (let ((completion-regexp-list '("X")))
+      (should-not (try-completion "a" abcdef))
+      (should-not (try-completion "a" +abba))
+      (should-not (try-completion "abc" +abba))
+      (should-not (try-completion "abcd" +abba)))))
+
+(defun minibuf-tests--all-completions (xform-collection)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (should (equal (all-completions "a" abcdef) '("abc")))
+    (should (equal (all-completions "a" +abba) '("abc" "abba")))
+    (should (equal (all-completions "abc" +abba) '("abc")))
+    (should (equal (all-completions "abcd" +abba) nil))))
+
+(defun minibuf-tests--all-completions-pred (xform-collection collection-member)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (abcdef-member (funcall collection-member abcdef))
+         (+abba  (funcall xform-collection '("abc" "abba" "def")))
+         (+abba-member (funcall collection-member +abba)))
+    (should (equal (all-completions "a" abcdef abcdef-member) '("abc")))
+    (should (equal (all-completions "a" +abba +abba-member) '("abc" "abba")))
+    (should (equal (all-completions "abc" +abba +abba-member) '("abc")))
+    (should (equal (all-completions "abcd" +abba +abba-member) nil))
+    (should-not (all-completions "a" abcdef #'ignore))
+    (should-not (all-completions "a" +abba #'ignore))
+    (should-not (all-completions "abc" +abba #'ignore))
+    (should-not (all-completions "abcd" +abba #'ignore))))
+
+(defun minibuf-tests--all-completions-regexp (xform-collection)
+  (let ((abcdef (funcall xform-collection '("abc" "def")))
+        (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (let ((completion-regexp-list '(".")))
+      (should (equal (all-completions "a" abcdef) '("abc")))
+      (should (equal (all-completions "a" +abba) '("abc" "abba")))
+      (should (equal (all-completions "abc" +abba) '("abc")))
+      (should (equal (all-completions "abcd" +abba) nil)))
+    (let ((completion-regexp-list '("X")))
+      (should-not (all-completions "a" abcdef))
+      (should-not (all-completions "a" +abba))
+      (should-not (all-completions "abc" +abba))
+      (should-not (all-completions "abcd" +abba)))))
+
+(defun minibuf-tests--test-completion (xform-collection)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (should (test-completion "abc" abcdef))
+    (should (test-completion "def" +abba))
+    (should (test-completion "abba" +abba))
+    (should-not (test-completion "abcd" +abba))))
+
+(defun minibuf-tests--test-completion-pred (xform-collection collection-member)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (abcdef-member (funcall collection-member abcdef))
+         (+abba  (funcall xform-collection '("abc" "abba" "def")))
+         (+abba-member (funcall collection-member +abba)))
+    (should (test-completion "abc" abcdef abcdef-member))
+    (should (test-completion "def" +abba +abba-member))
+    (should (test-completion "abba" +abba +abba-member))
+    (should-not (test-completion "abcd" +abba +abba-member))
+    (should-not (test-completion "abc" abcdef #'ignore))
+    (should-not (test-completion "def" +abba #'ignore))
+    (should-not (test-completion "abba" +abba #'ignore))
+    (should-not (test-completion "abcd" +abba #'ignore))))
+
+(defun minibuf-tests--test-completion-regexp (xform-collection)
+  (let ((abcdef (funcall xform-collection '("abc" "def")))
+        (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (let ((completion-regexp-list '(".")))
+      (should (test-completion "abc" abcdef))
+      (should (test-completion "def" +abba))
+      (should (test-completion "abba" +abba))
+      (should-not (test-completion "abcd" +abba)))
+    (let ((completion-regexp-list '("X")))
+      (should-not (test-completion "abc" abcdef))
+      (should-not (test-completion "def" +abba))
+      (should-not (test-completion "abba" +abba))
+      (should-not (test-completion "abcd" +abba)))))
+
+\f
+;;; Tests for `try-completion'.
+(ert-deftest try-completion-string-list ()
+  (minibuf-tests--try-completion #'identity))
+(ert-deftest try-completion-string-list-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'identity #'minibuf-tests--memq-of-collection))
+(ert-deftest try-completion-string-list-completion-regexp ()
+  (minibuf-tests--try-completion-regexp #'identity))
+
+(ert-deftest try-completion-symbol-list ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-symbol-list))
+(ert-deftest try-completion-symbol-list-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-symbol-list
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest try-completion-symbol-list-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-symbol-list))
+
+(ert-deftest try-completion-symbol-alist ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-symbol-alist))
+(ert-deftest try-completion-symbol-alist-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-symbol-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest try-completion-symbol-alist-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-symbol-alist))
+
+(ert-deftest try-completion-string-alist ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-string-alist))
+(ert-deftest try-completion-string-alist-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-string-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest try-completion-string-alist-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-string-alist))
+
+(ert-deftest try-completion-obarray ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-obarray))
+(ert-deftest try-completion-obarray-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-obarray
+   #'minibuf-tests--part-of-obarray))
+(ert-deftest try-completion-obarray-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-obarray))
+
+(ert-deftest try-completion-string-hashtable ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-string-hashtable))
+(ert-deftest try-completion-string-hashtable-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-string-hashtable
+   #'minibuf-tests--part-of-hashtable))
+(ert-deftest try-completion-string-hashtable-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-string-hashtable))
+
+(ert-deftest try-completion-symbol-hashtable ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-symbol-hashtable))
+(ert-deftest try-completion-symbol-hashtable-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-symbol-hashtable
+   #'minibuf-tests--part-of-hashtable))
+(ert-deftest try-completion-symbol-hashtable-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-symbol-hashtable))
+
+\f
+;;; Tests for `all-completions'.
+
+(ert-deftest all-completions-string-list ()
+  (minibuf-tests--all-completions #'identity))
+(ert-deftest all-completions-string-list-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'identity #'minibuf-tests--memq-of-collection))
+(ert-deftest all-completions-string-list-completion-regexp ()
+  (minibuf-tests--all-completions-regexp #'identity))
+
+(ert-deftest all-completions-symbol-list ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-symbol-list))
+(ert-deftest all-completions-symbol-list-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-symbol-list
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest all-completions-symbol-list-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-symbol-list))
+
+(ert-deftest all-completions-symbol-alist ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-symbol-alist))
+(ert-deftest all-completions-symbol-alist-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-symbol-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest all-completions-symbol-alist-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-symbol-alist))
+
+(ert-deftest all-completions-string-alist ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-string-alist))
+(ert-deftest all-completions-string-alist-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-string-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest all-completions-string-alist-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-string-alist))
+
+(ert-deftest all-completions-obarray ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-obarray))
+(ert-deftest all-completions-obarray-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-obarray
+   #'minibuf-tests--part-of-obarray))
+(ert-deftest all-completions-obarray-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-obarray))
+
+(ert-deftest all-completions-string-hashtable ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-string-hashtable))
+(ert-deftest all-completions-string-hashtable-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-string-hashtable
+   #'minibuf-tests--part-of-hashtable))
+(ert-deftest all-completions-string-hashtable-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-string-hashtable))
+
+(ert-deftest all-completions-symbol-hashtable ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-symbol-hashtable))
+(ert-deftest all-completions-symbol-hashtable-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-symbol-hashtable
+   #'minibuf-tests--part-of-hashtable))
+(ert-deftest all-completions-symbol-hashtable-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-symbol-hashtable))
+
+\f
+;;; Tests for `test-completion'.
+
+(ert-deftest test-completion-string-list ()
+  (minibuf-tests--test-completion #'identity))
+(ert-deftest test-completion-string-list-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'identity #'minibuf-tests--memq-of-collection))
+(ert-deftest test-completion-string-list-completion-regexp ()
+  (minibuf-tests--test-completion-regexp #'identity))
+
+(ert-deftest test-completion-symbol-list ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-symbol-list))
+(ert-deftest test-completion-symbol-list-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-symbol-list
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest test-completion-symbol-list-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-symbol-list))
+
+(ert-deftest test-completion-symbol-alist ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-symbol-alist))
+(ert-deftest test-completion-symbol-alist-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-symbol-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest test-completion-symbol-alist-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-symbol-alist))
+
+(ert-deftest test-completion-string-alist ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-string-alist))
+(ert-deftest test-completion-string-alist-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-string-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest test-completion-string-alist-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-string-alist))
+
+(ert-deftest test-completion-obarray ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-obarray))
+(ert-deftest test-completion-obarray-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-obarray
+   #'minibuf-tests--part-of-obarray))
+(ert-deftest test-completion-obarray-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-obarray))
+
+(ert-deftest test-completion-string-hashtable ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-string-hashtable))
+(ert-deftest test-completion-string-hashtable-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-string-hashtable
+   #'minibuf-tests--part-of-hashtable))
+(ert-deftest test-completion-string-hashtable-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-string-hashtable))
+
+(ert-deftest test-completion-symbol-hashtable ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-symbol-hashtable))
+(ert-deftest test-completion-symbol-hashtable-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-symbol-hashtable
+   ;; The predicate recieves a string as the key in this case.
+   (lambda (table)
+     (let ((in-table (minibuf-tests--part-of-hashtable table)))
+       (lambda (k v) (funcall in-table (intern k) v))))))
+(ert-deftest test-completion-symbol-hashtable-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-symbol-hashtable))
+
+\f
+;;; minibuf-tests.el ends here
-- 
2.9.3


[-- Attachment #3: patch --]
[-- Type: text/plain, Size: 2894 bytes --]

From 6b587c804c6a98b30de984e58e56b9ba3794810a Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 27 Nov 2016 14:41:02 -0500
Subject: [PATCH v1 2/2] Give test-completion's PREDICATE the hashtable key

For hashtable entries with symbol keys, `test-completion' would convert
the key to a string before calling PREDICATE, unlike `try-completion'
and `all-completions'.

* src/minibuf.c (Ftest_completion): Pass original key from hashtable.
---
 src/minibuf.c             | 23 ++++++++++++-----------
 test/src/minibuf-tests.el |  5 +----
 2 files changed, 13 insertions(+), 15 deletions(-)

diff --git a/src/minibuf.c b/src/minibuf.c
index 6c694cb..7c5af34 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1736,26 +1736,27 @@ DEFUN ("test-completion", Ftest_completion, Stest_completion, 2, 3, 0,
   else if (HASH_TABLE_P (collection))
     {
       struct Lisp_Hash_Table *h = XHASH_TABLE (collection);
-      Lisp_Object key = Qnil;
       i = hash_lookup (h, string, NULL);
       if (i >= 0)
+        {
           tem = HASH_KEY (h, i);
+          goto found_matching_key;
+        }
       else
 	for (i = 0; i < HASH_TABLE_SIZE (h); ++i)
-	  if (!NILP (HASH_HASH (h, i))
-	      && (key = HASH_KEY (h, i),
-		  SYMBOLP (key) ? key = Fsymbol_name (key) : key,
-		  STRINGP (key))
-	      && EQ (Fcompare_strings (string, make_number (0), Qnil,
-				       key, make_number (0) , Qnil,
+          {
+            if (NILP (HASH_HASH (h, i))) continue;
+            tem = HASH_KEY (h, i);
+            Lisp_Object strkey = (SYMBOLP (tem) ? Fsymbol_name (tem) : tem);
+            if (!STRINGP (strkey)) continue;
+            if (EQ (Fcompare_strings (string, Qnil, Qnil,
+                                      strkey, Qnil, Qnil,
                                       completion_ignore_case ? Qt : Qnil),
                     Qt))
-	    {
-	      tem = key;
-	      break;
+              goto found_matching_key;
           }
-      if (!STRINGP (tem))
       return Qnil;
+    found_matching_key: ;
     }
   else
     return call3 (collection, string, predicate, Qlambda);
diff --git a/test/src/minibuf-tests.el b/test/src/minibuf-tests.el
index 98b8614..82ac037 100644
--- a/test/src/minibuf-tests.el
+++ b/test/src/minibuf-tests.el
@@ -394,10 +394,7 @@ minibuf-tests--test-completion-regexp
 (ert-deftest test-completion-symbol-hashtable-predicate ()
   (minibuf-tests--test-completion-pred
    #'minibuf-tests--strings-to-symbol-hashtable
-   ;; The predicate recieves a string as the key in this case.
-   (lambda (table)
-     (let ((in-table (minibuf-tests--part-of-hashtable table)))
-       (lambda (k v) (funcall in-table (intern k) v))))))
+   #'minibuf-tests--part-of-hashtable))
 (ert-deftest test-completion-symbol-hashtable-completion-regexp ()
   (minibuf-tests--test-completion-regexp
    #'minibuf-tests--strings-to-symbol-hashtable))
-- 
2.9.3

bug#24966: 26.0.50; test-completion with alist COLLECTION calls PREDICATE incorrectly

[Mark Oteiza]

On 27/11/16 at 03:24pm, npostavs@users.sourceforge.net wrote:
> tags 24966 patch
> quit
> 
> npostavs@users.sourceforge.net writes:
> >
> > I tentatively suggest the patch below, but I want to add some tests
> > before commiting anything.
> 
> While adding tests, I found another inconsistency: when given a
> hashtable with symbol keys, test-completion passes the symbol-name to
> PREDICATE, while all-completions and try-completion pass the original
> symbol key.  Here are two patches, the first for this bug, and the
> second for the other inconsistency.

The first hunk of 2/2 isn't applying here.

bug#24966: 26.0.50; test-completion with alist COLLECTION calls PREDICATE incorrectly

[Noam Postavsky]

On Mon, Nov 28, 2016 at 3:59 PM, Mark Oteiza <mvoteiza@udel.edu> wrote:
>>
>> npostavs@users.sourceforge.net writes:
>> >
>> > I tentatively suggest the patch below, but I want to add some tests
>> > before commiting anything.
>>
>> While adding tests, I found another inconsistency: when given a
>> hashtable with symbol keys, test-completion passes the symbol-name to
>> PREDICATE, while all-completions and try-completion pass the original
>> symbol key.  Here are two patches, the first for this bug, and the
>> second for the other inconsistency.
>
> The first hunk of 2/2 isn't applying here.

Oh, that's probably because I generated it with -w, so some whitespace
changes are missing in 1/2. I'll post a full patch tonight.

bug#24966: 26.0.50; test-completion with alist COLLECTION calls PREDICATE incorrectly

[npostavs]

[-- Attachment #1: Type: text/plain, Size: 866 bytes --]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> On Mon, Nov 28, 2016 at 3:59 PM, Mark Oteiza <mvoteiza@udel.edu> wrote:
>>>
>>> npostavs@users.sourceforge.net writes:
>>> >
>>> > I tentatively suggest the patch below, but I want to add some tests
>>> > before commiting anything.
>>>
>>> While adding tests, I found another inconsistency: when given a
>>> hashtable with symbol keys, test-completion passes the symbol-name to
>>> PREDICATE, while all-completions and try-completion pass the original
>>> symbol key.  Here are two patches, the first for this bug, and the
>>> second for the other inconsistency.
>>
>> The first hunk of 2/2 isn't applying here.
>
> Oh, that's probably because I generated it with -w, so some whitespace
> changes are missing in 1/2. I'll post a full patch tonight.

Here are the same patches with full whitespace changes.


[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 20004 bytes --]

From ba3f1477e66536fd759c4f7128bfbca009532b14 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 27 Nov 2016 10:04:48 -0500
Subject: [PATCH v1 1/2] Give test-completion's PREDICATE full alist entry

Since 2016-06-26 "Fix test-completion with completion-regexp-list", when
calling test-completion with an alist collection, the predicate was
recieving the string value instead of the alist entry (Bug#24966).

* src/minibuf.c (Ftest_completion): Don't modify the found element, just
test STRING against `completion-regexp-list'.
* test/src/minibuf-tests.el: New tests for `try-completion',
`all-completions', and `test-completion'.
---
 src/minibuf.c             |   8 +-
 test/src/minibuf-tests.el | 406 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 409 insertions(+), 5 deletions(-)
 create mode 100644 test/src/minibuf-tests.el

diff --git a/src/minibuf.c b/src/minibuf.c
index 57eea05..6c694cb 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1686,8 +1686,6 @@ 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 = XCAR (tem);
     }
   else if (VECTORP (collection))
     {
@@ -1770,9 +1768,9 @@ DEFUN ("test-completion", Ftest_completion, Stest_completion, 2, 3, 0,
       for (regexps = Vcompletion_regexp_list; CONSP (regexps);
 	   regexps = XCDR (regexps))
 	{
-	  if (NILP (Fstring_match (XCAR (regexps),
-				   SYMBOLP (tem) ? string : tem,
-				   Qnil)))
+          /* We can test against STRING, because if we got here, then
+             the element is equivalent to it.  */
+          if (NILP (Fstring_match (XCAR (regexps), string, Qnil)))
 	    return unbind_to (count, Qnil);
 	}
       unbind_to (count, Qnil);
diff --git a/test/src/minibuf-tests.el b/test/src/minibuf-tests.el
new file mode 100644
index 0000000..98b8614
--- /dev/null
+++ b/test/src/minibuf-tests.el
@@ -0,0 +1,406 @@
+;;; minibuf-tests.el --- tests for minibuf.c functions -*- lexical-binding: t -*-
+
+;; Copyright (C) 2016 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+
+\f
+;;; Support functions for `try-completion', `all-completion', and
+;;; `test-completion' tests.
+
+(defun minibuf-tests--strings-to-symbol-list (list)
+  (mapcar #'intern list))
+(defun minibuf-tests--strings-to-symbol-alist (list)
+  (let ((num 0))
+    (mapcar (lambda (str) (cons (intern str) (cl-incf num))) list)))
+(defun minibuf-tests--strings-to-string-alist (list)
+  (let ((num 0))
+    (mapcar (lambda (str) (cons str (cl-incf num))) list)))
+(defun minibuf-tests--strings-to-obarray (list)
+  (let ((ob (make-vector 7 0)))
+    (mapc (lambda (str) (intern str ob)) list)
+    ob))
+(defun minibuf-tests--strings-to-string-hashtable (list)
+  (let ((ht (make-hash-table :test #'equal))
+        (num 0))
+    (mapc (lambda (str) (puthash str (cl-incf num) ht)) list)
+    ht))
+(defun minibuf-tests--strings-to-symbol-hashtable (list)
+  (let ((ht (make-hash-table :test #'equal))
+        (num 0))
+    (mapc (lambda (str) (puthash (intern str) (cl-incf num) ht)) list)
+    ht))
+
+;;; Functions that produce a predicate (for *-completion functions)
+;;; which always returns non-nil for a given collection.
+
+(defun minibuf-tests--memq-of-collection (collection)
+  (lambda (elt) (memq elt collection)))
+(defun minibuf-tests--part-of-obarray (ob)
+  (lambda (sym) (eq (intern-soft (symbol-name sym) ob) sym)))
+(defun minibuf-tests--part-of-hashtable (table)
+  (lambda (k v) (equal (gethash k table) v)))
+
+\f
+;;; Testing functions that are agnostic to type of COLLECTION.
+
+(defun minibuf-tests--try-completion (xform-collection)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (should (equal (try-completion "a" abcdef) "abc"))
+    (should (equal (try-completion "a" +abba) "ab"))
+    (should (equal (try-completion "abc" +abba) t))
+    (should (equal (try-completion "abcd" +abba) nil))))
+
+(defun minibuf-tests--try-completion-pred (xform-collection collection-member)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (abcdef-member (funcall collection-member abcdef))
+         (+abba  (funcall xform-collection '("abc" "abba" "def")))
+         (+abba-member (funcall collection-member +abba)))
+    (should (equal (try-completion "a" abcdef abcdef-member) "abc"))
+    (should (equal (try-completion "a" +abba +abba-member) "ab"))
+    (should (equal (try-completion "abc" +abba +abba-member) t))
+    (should (equal (try-completion "abcd" +abba +abba-member) nil))
+    (should-not (try-completion "a" abcdef #'ignore))
+    (should-not (try-completion "a" +abba #'ignore))
+    (should-not (try-completion "abc" +abba #'ignore))
+    (should-not (try-completion "abcd" +abba #'ignore))))
+
+(defun minibuf-tests--try-completion-regexp (xform-collection)
+  (let ((abcdef (funcall xform-collection '("abc" "def")))
+        (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (let ((completion-regexp-list '(".")))
+      (should (equal (try-completion "a" abcdef) "abc"))
+      (should (equal (try-completion "a" +abba) "ab"))
+      (should (equal (try-completion "abc" +abba) t))
+      (should (equal (try-completion "abcd" +abba) nil)))
+    (let ((completion-regexp-list '("X")))
+      (should-not (try-completion "a" abcdef))
+      (should-not (try-completion "a" +abba))
+      (should-not (try-completion "abc" +abba))
+      (should-not (try-completion "abcd" +abba)))))
+
+(defun minibuf-tests--all-completions (xform-collection)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (should (equal (all-completions "a" abcdef) '("abc")))
+    (should (equal (all-completions "a" +abba) '("abc" "abba")))
+    (should (equal (all-completions "abc" +abba) '("abc")))
+    (should (equal (all-completions "abcd" +abba) nil))))
+
+(defun minibuf-tests--all-completions-pred (xform-collection collection-member)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (abcdef-member (funcall collection-member abcdef))
+         (+abba  (funcall xform-collection '("abc" "abba" "def")))
+         (+abba-member (funcall collection-member +abba)))
+    (should (equal (all-completions "a" abcdef abcdef-member) '("abc")))
+    (should (equal (all-completions "a" +abba +abba-member) '("abc" "abba")))
+    (should (equal (all-completions "abc" +abba +abba-member) '("abc")))
+    (should (equal (all-completions "abcd" +abba +abba-member) nil))
+    (should-not (all-completions "a" abcdef #'ignore))
+    (should-not (all-completions "a" +abba #'ignore))
+    (should-not (all-completions "abc" +abba #'ignore))
+    (should-not (all-completions "abcd" +abba #'ignore))))
+
+(defun minibuf-tests--all-completions-regexp (xform-collection)
+  (let ((abcdef (funcall xform-collection '("abc" "def")))
+        (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (let ((completion-regexp-list '(".")))
+      (should (equal (all-completions "a" abcdef) '("abc")))
+      (should (equal (all-completions "a" +abba) '("abc" "abba")))
+      (should (equal (all-completions "abc" +abba) '("abc")))
+      (should (equal (all-completions "abcd" +abba) nil)))
+    (let ((completion-regexp-list '("X")))
+      (should-not (all-completions "a" abcdef))
+      (should-not (all-completions "a" +abba))
+      (should-not (all-completions "abc" +abba))
+      (should-not (all-completions "abcd" +abba)))))
+
+(defun minibuf-tests--test-completion (xform-collection)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (should (test-completion "abc" abcdef))
+    (should (test-completion "def" +abba))
+    (should (test-completion "abba" +abba))
+    (should-not (test-completion "abcd" +abba))))
+
+(defun minibuf-tests--test-completion-pred (xform-collection collection-member)
+  (let* ((abcdef (funcall xform-collection '("abc" "def")))
+         (abcdef-member (funcall collection-member abcdef))
+         (+abba  (funcall xform-collection '("abc" "abba" "def")))
+         (+abba-member (funcall collection-member +abba)))
+    (should (test-completion "abc" abcdef abcdef-member))
+    (should (test-completion "def" +abba +abba-member))
+    (should (test-completion "abba" +abba +abba-member))
+    (should-not (test-completion "abcd" +abba +abba-member))
+    (should-not (test-completion "abc" abcdef #'ignore))
+    (should-not (test-completion "def" +abba #'ignore))
+    (should-not (test-completion "abba" +abba #'ignore))
+    (should-not (test-completion "abcd" +abba #'ignore))))
+
+(defun minibuf-tests--test-completion-regexp (xform-collection)
+  (let ((abcdef (funcall xform-collection '("abc" "def")))
+        (+abba  (funcall xform-collection '("abc" "abba" "def"))))
+    (let ((completion-regexp-list '(".")))
+      (should (test-completion "abc" abcdef))
+      (should (test-completion "def" +abba))
+      (should (test-completion "abba" +abba))
+      (should-not (test-completion "abcd" +abba)))
+    (let ((completion-regexp-list '("X")))
+      (should-not (test-completion "abc" abcdef))
+      (should-not (test-completion "def" +abba))
+      (should-not (test-completion "abba" +abba))
+      (should-not (test-completion "abcd" +abba)))))
+
+\f
+;;; Tests for `try-completion'.
+(ert-deftest try-completion-string-list ()
+  (minibuf-tests--try-completion #'identity))
+(ert-deftest try-completion-string-list-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'identity #'minibuf-tests--memq-of-collection))
+(ert-deftest try-completion-string-list-completion-regexp ()
+  (minibuf-tests--try-completion-regexp #'identity))
+
+(ert-deftest try-completion-symbol-list ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-symbol-list))
+(ert-deftest try-completion-symbol-list-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-symbol-list
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest try-completion-symbol-list-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-symbol-list))
+
+(ert-deftest try-completion-symbol-alist ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-symbol-alist))
+(ert-deftest try-completion-symbol-alist-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-symbol-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest try-completion-symbol-alist-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-symbol-alist))
+
+(ert-deftest try-completion-string-alist ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-string-alist))
+(ert-deftest try-completion-string-alist-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-string-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest try-completion-string-alist-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-string-alist))
+
+(ert-deftest try-completion-obarray ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-obarray))
+(ert-deftest try-completion-obarray-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-obarray
+   #'minibuf-tests--part-of-obarray))
+(ert-deftest try-completion-obarray-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-obarray))
+
+(ert-deftest try-completion-string-hashtable ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-string-hashtable))
+(ert-deftest try-completion-string-hashtable-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-string-hashtable
+   #'minibuf-tests--part-of-hashtable))
+(ert-deftest try-completion-string-hashtable-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-string-hashtable))
+
+(ert-deftest try-completion-symbol-hashtable ()
+  (minibuf-tests--try-completion
+   #'minibuf-tests--strings-to-symbol-hashtable))
+(ert-deftest try-completion-symbol-hashtable-predicate ()
+  (minibuf-tests--try-completion-pred
+   #'minibuf-tests--strings-to-symbol-hashtable
+   #'minibuf-tests--part-of-hashtable))
+(ert-deftest try-completion-symbol-hashtable-completion-regexp ()
+  (minibuf-tests--try-completion-regexp
+   #'minibuf-tests--strings-to-symbol-hashtable))
+
+\f
+;;; Tests for `all-completions'.
+
+(ert-deftest all-completions-string-list ()
+  (minibuf-tests--all-completions #'identity))
+(ert-deftest all-completions-string-list-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'identity #'minibuf-tests--memq-of-collection))
+(ert-deftest all-completions-string-list-completion-regexp ()
+  (minibuf-tests--all-completions-regexp #'identity))
+
+(ert-deftest all-completions-symbol-list ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-symbol-list))
+(ert-deftest all-completions-symbol-list-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-symbol-list
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest all-completions-symbol-list-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-symbol-list))
+
+(ert-deftest all-completions-symbol-alist ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-symbol-alist))
+(ert-deftest all-completions-symbol-alist-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-symbol-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest all-completions-symbol-alist-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-symbol-alist))
+
+(ert-deftest all-completions-string-alist ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-string-alist))
+(ert-deftest all-completions-string-alist-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-string-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest all-completions-string-alist-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-string-alist))
+
+(ert-deftest all-completions-obarray ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-obarray))
+(ert-deftest all-completions-obarray-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-obarray
+   #'minibuf-tests--part-of-obarray))
+(ert-deftest all-completions-obarray-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-obarray))
+
+(ert-deftest all-completions-string-hashtable ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-string-hashtable))
+(ert-deftest all-completions-string-hashtable-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-string-hashtable
+   #'minibuf-tests--part-of-hashtable))
+(ert-deftest all-completions-string-hashtable-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-string-hashtable))
+
+(ert-deftest all-completions-symbol-hashtable ()
+  (minibuf-tests--all-completions
+   #'minibuf-tests--strings-to-symbol-hashtable))
+(ert-deftest all-completions-symbol-hashtable-predicate ()
+  (minibuf-tests--all-completions-pred
+   #'minibuf-tests--strings-to-symbol-hashtable
+   #'minibuf-tests--part-of-hashtable))
+(ert-deftest all-completions-symbol-hashtable-completion-regexp ()
+  (minibuf-tests--all-completions-regexp
+   #'minibuf-tests--strings-to-symbol-hashtable))
+
+\f
+;;; Tests for `test-completion'.
+
+(ert-deftest test-completion-string-list ()
+  (minibuf-tests--test-completion #'identity))
+(ert-deftest test-completion-string-list-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'identity #'minibuf-tests--memq-of-collection))
+(ert-deftest test-completion-string-list-completion-regexp ()
+  (minibuf-tests--test-completion-regexp #'identity))
+
+(ert-deftest test-completion-symbol-list ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-symbol-list))
+(ert-deftest test-completion-symbol-list-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-symbol-list
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest test-completion-symbol-list-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-symbol-list))
+
+(ert-deftest test-completion-symbol-alist ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-symbol-alist))
+(ert-deftest test-completion-symbol-alist-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-symbol-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest test-completion-symbol-alist-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-symbol-alist))
+
+(ert-deftest test-completion-string-alist ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-string-alist))
+(ert-deftest test-completion-string-alist-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-string-alist
+   #'minibuf-tests--memq-of-collection))
+(ert-deftest test-completion-string-alist-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-string-alist))
+
+(ert-deftest test-completion-obarray ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-obarray))
+(ert-deftest test-completion-obarray-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-obarray
+   #'minibuf-tests--part-of-obarray))
+(ert-deftest test-completion-obarray-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-obarray))
+
+(ert-deftest test-completion-string-hashtable ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-string-hashtable))
+(ert-deftest test-completion-string-hashtable-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-string-hashtable
+   #'minibuf-tests--part-of-hashtable))
+(ert-deftest test-completion-string-hashtable-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-string-hashtable))
+
+(ert-deftest test-completion-symbol-hashtable ()
+  (minibuf-tests--test-completion
+   #'minibuf-tests--strings-to-symbol-hashtable))
+(ert-deftest test-completion-symbol-hashtable-predicate ()
+  (minibuf-tests--test-completion-pred
+   #'minibuf-tests--strings-to-symbol-hashtable
+   ;; The predicate recieves a string as the key in this case.
+   (lambda (table)
+     (let ((in-table (minibuf-tests--part-of-hashtable table)))
+       (lambda (k v) (funcall in-table (intern k) v))))))
+(ert-deftest test-completion-symbol-hashtable-completion-regexp ()
+  (minibuf-tests--test-completion-regexp
+   #'minibuf-tests--strings-to-symbol-hashtable))
+
+\f
+;;; minibuf-tests.el ends here
-- 
2.9.3


[-- Attachment #3: patch --]
[-- Type: text/plain, Size: 3014 bytes --]

From 6b587c804c6a98b30de984e58e56b9ba3794810a Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sun, 27 Nov 2016 14:41:02 -0500
Subject: [PATCH v1 2/2] Give test-completion's PREDICATE the hashtable key

For hashtable entries with symbol keys, `test-completion' would convert
the key to a string before calling PREDICATE, unlike `try-completion'
and `all-completions'.

* src/minibuf.c (Ftest_completion): Pass original key from hashtable.
---
 src/minibuf.c             | 33 +++++++++++++++++----------------
 test/src/minibuf-tests.el |  5 +----
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/src/minibuf.c b/src/minibuf.c
index 6c694cb..7c5af34 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1736,26 +1736,27 @@ DEFUN ("test-completion", Ftest_completion, Stest_completion, 2, 3, 0,
   else if (HASH_TABLE_P (collection))
     {
       struct Lisp_Hash_Table *h = XHASH_TABLE (collection);
-      Lisp_Object key = Qnil;
       i = hash_lookup (h, string, NULL);
       if (i >= 0)
-	tem = HASH_KEY (h, i);
+        {
+          tem = HASH_KEY (h, i);
+          goto found_matching_key;
+        }
       else
 	for (i = 0; i < HASH_TABLE_SIZE (h); ++i)
-	  if (!NILP (HASH_HASH (h, i))
-	      && (key = HASH_KEY (h, i),
-		  SYMBOLP (key) ? key = Fsymbol_name (key) : key,
-		  STRINGP (key))
-	      && EQ (Fcompare_strings (string, make_number (0), Qnil,
-				       key, make_number (0) , Qnil,
-				       completion_ignore_case ? Qt : Qnil),
-		     Qt))
-	    {
-	      tem = key;
-	      break;
-	    }
-      if (!STRINGP (tem))
-	return Qnil;
+          {
+            if (NILP (HASH_HASH (h, i))) continue;
+            tem = HASH_KEY (h, i);
+            Lisp_Object strkey = (SYMBOLP (tem) ? Fsymbol_name (tem) : tem);
+            if (!STRINGP (strkey)) continue;
+            if (EQ (Fcompare_strings (string, Qnil, Qnil,
+                                      strkey, Qnil, Qnil,
+                                      completion_ignore_case ? Qt : Qnil),
+                    Qt))
+              goto found_matching_key;
+          }
+      return Qnil;
+    found_matching_key: ;
     }
   else
     return call3 (collection, string, predicate, Qlambda);
diff --git a/test/src/minibuf-tests.el b/test/src/minibuf-tests.el
index 98b8614..82ac037 100644
--- a/test/src/minibuf-tests.el
+++ b/test/src/minibuf-tests.el
@@ -394,10 +394,7 @@ minibuf-tests--test-completion-regexp
 (ert-deftest test-completion-symbol-hashtable-predicate ()
   (minibuf-tests--test-completion-pred
    #'minibuf-tests--strings-to-symbol-hashtable
-   ;; The predicate recieves a string as the key in this case.
-   (lambda (table)
-     (let ((in-table (minibuf-tests--part-of-hashtable table)))
-       (lambda (k v) (funcall in-table (intern k) v))))))
+   #'minibuf-tests--part-of-hashtable))
 (ert-deftest test-completion-symbol-hashtable-completion-regexp ()
   (minibuf-tests--test-completion-regexp
    #'minibuf-tests--strings-to-symbol-hashtable))
-- 
2.9.3

bug#24966: 26.0.50; test-completion with alist COLLECTION calls PREDICATE incorrectly

[Mark Oteiza]

On 28/11/16 at 10:31pm, npostavs@users.sourceforge.net wrote:
> Noam Postavsky <npostavs@users.sourceforge.net> writes:
> 
> > On Mon, Nov 28, 2016 at 3:59 PM, Mark Oteiza <mvoteiza@udel.edu> wrote:
> >>>
> >>> npostavs@users.sourceforge.net writes:
> >>> >
> >>> > I tentatively suggest the patch below, but I want to add some tests
> >>> > before commiting anything.
> >>>
> >>> While adding tests, I found another inconsistency: when given a
> >>> hashtable with symbol keys, test-completion passes the symbol-name to
> >>> PREDICATE, while all-completions and try-completion pass the original
> >>> symbol key.  Here are two patches, the first for this bug, and the
> >>> second for the other inconsistency.
> >>
> >> The first hunk of 2/2 isn't applying here.
> >
> > Oh, that's probably because I generated it with -w, so some whitespace
> > changes are missing in 1/2. I'll post a full patch tonight.
> 
> Here are the same patches with full whitespace changes.

Thanks, all is well here with these patches

bug#24966: 26.0.50; test-completion with alist COLLECTION calls PREDICATE incorrectly

[Mark Oteiza]

On 29/11/16 at 09:04pm, Mark Oteiza wrote:
> On 28/11/16 at 10:31pm, npostavs@users.sourceforge.net wrote:
> > Noam Postavsky <npostavs@users.sourceforge.net> writes:
> > 
> > > On Mon, Nov 28, 2016 at 3:59 PM, Mark Oteiza <mvoteiza@udel.edu> wrote:
> > >>>
> > >>> npostavs@users.sourceforge.net writes:
> > >>> >
> > >>> > I tentatively suggest the patch below, but I want to add some tests
> > >>> > before commiting anything.
> > >>>
> > >>> While adding tests, I found another inconsistency: when given a
> > >>> hashtable with symbol keys, test-completion passes the symbol-name to
> > >>> PREDICATE, while all-completions and try-completion pass the original
> > >>> symbol key.  Here are two patches, the first for this bug, and the
> > >>> second for the other inconsistency.
> > >>
> > >> The first hunk of 2/2 isn't applying here.
> > >
> > > Oh, that's probably because I generated it with -w, so some whitespace
> > > changes are missing in 1/2. I'll post a full patch tonight.
> > 
> > Here are the same patches with full whitespace changes.
> 
> Thanks, all is well here with these patches

Anything holding back applying these?

bug#24966: 26.0.50; test-completion with alist COLLECTION calls PREDICATE incorrectly

[npostavs]

tags 24966 fixed
close 24966 
quit

Mark Oteiza <mvoteiza@udel.edu> writes:

> On 29/11/16 at 09:04pm, Mark Oteiza wrote:
>> > >>>
>> > >>> npostavs@users.sourceforge.net writes:
>> > >>>
>> > >>> While adding tests, I found another inconsistency: when given a
>> > >>> hashtable with symbol keys, test-completion passes the symbol-name to
>> > >>> PREDICATE, while all-completions and try-completion pass the original
>> > >>> symbol key.  Here are two patches, the first for this bug, and the
>> > >>> second for the other inconsistency.
>> 
>> Thanks, all is well here with these patches
>
> Anything holding back applying these?

I was waiting to see if there were any comments, especially about the
second patch, but it's been long enough by now.  Pushed as 60fe6301,
2a3420d9.