From: Ryan Thompson <rct@thompsonclan.org>
To: Dmitry Gutov <dgutov@yandex.ru>, npostavs@users.sourceforge.net
Cc: 27841@debbugs.gnu.org
Subject: bug#27841: 25.2; Patch for completion-table-with-predicate
Date: Sat, 05 Aug 2017 06:22:39 +0000 [thread overview]
Message-ID: <CAHCt_aZS4N_+jMKDV97PSGt00HwzuvEDTwgGwpiU0k6noX6_Mg@mail.gmail.com> (raw)
In-Reply-To: <10ad76ff-8e89-d8f2-1278-3fd04845b243@yandex.ru>
[-- Attachment #1.1: Type: text/plain, Size: 1705 bytes --]
Ok, here is the revised patch. I added a test and reformatted the commit
message in (hopefully) ChangeLog format. The test includes a few basic
tests of "completion-table-with-predicate" including one which is fixed by
this patch (noted in the test comments).
I've dropped the second patch entirely, because I realized it was
inappropriately optimizing for a rare case (i.e. PRED1 being nil).
Also, a few other related things: First, I noticed that while the test file
name is minibuffer-tests.el, the lisp header and provide statement both
contain "completion-tests.el" instead. Presumably this file was renamed at
some point without updating its contents.
Second, I notice that there is not much in this file, in particular nothing
testing interactive completion in the minibuffer. I recently wrote a
package for simulating interactive input to a command in order to implement
tests for my ido-comrpleting-read+ package, and it might be useful for
adding more tests here. You can view it here:
https://github.com/DarwinAwardWinner/with-simulated-input and view example
usage in the ido-cr+ test suite here:
https://github.com/DarwinAwardWinner/ido-completing-read-plus/blob/master/tests/test-ido-completing-read%2B.el
Let me know if there is interest in using such an approach to add automated
tests for some of the core interactive Emacs functions like
"completing-read".
On Fri, Aug 4, 2017 at 4:11 PM Dmitry Gutov <dgutov@yandex.ru> wrote:
> On 8/5/17 2:00 AM, Ryan Thompson wrote:
> > Would you also like me
> > to provide an example where the old code does the wrong thing?
>
> Adding a test case or two to test/lisp/minibuffer-tests.el would be
> ideal (in the same patch that fixes it).
>
[-- Attachment #1.2: Type: text/html, Size: 2312 bytes --]
[-- Attachment #2: 0001-Fix-a-bug-in-completion-table-with-predicate.patch --]
[-- Type: application/octet-stream, Size: 2901 bytes --]
From b5313defb1132021bd86d5d385e8ea6d2d92a663 Mon Sep 17 00:00:00 2001
From: "Ryan C. Thompson" <rct@thompsonclan.org>
Date: Wed, 26 Jul 2017 11:09:42 -0700
Subject: [PATCH] Fix a bug in completion-table-with-predicate
* ../lisp/minibuffer.el (completion-table-with-predicate): Don't
act as if strict is non-nil when pred2 is nil (Bug#27841).
* ../test/lisp/minibuffer-tests.el
(completion-table-with-predicate-test): Add a test for Bug#27841.
---
lisp/minibuffer.el | 2 +-
test/lisp/minibuffer-tests.el | 34 ++++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index e5b1029c01..bb8cf21ad2 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -392,7 +392,7 @@ obeys predicates."
(and (funcall pred1 x) (funcall pred2 x)))))
;; If completion failed and we're not applying pred1 strictly, try
;; again without pred1.
- (and (not strict) pred1 pred2
+ (and (not strict) pred1
(complete-with-action action table string pred2))))))
(defun completion-table-in-turn (&rest tables)
diff --git a/test/lisp/minibuffer-tests.el b/test/lisp/minibuffer-tests.el
index 7c5fcb4838..b209ae1c22 100644
--- a/test/lisp/minibuffer-tests.el
+++ b/test/lisp/minibuffer-tests.el
@@ -42,5 +42,39 @@
(should (equal (buffer-string)
"test: "))))))
+(ert-deftest completion-table-with-predicate-test ()
+ (cl-letf* ((full-collection
+ '("apple" ; Has A
+ "beet" ; Has B
+ "banana" ; Has A & B
+ "cherry" ; Has neither
+ ))
+ ((symbol-function 'no-A)
+ (lambda (x) (not (string-match-p "a" x))))
+ ((symbol-function 'no-B)
+ (lambda (x) (not (string-match-p "b" x)))))
+ (should
+ (member "cherry"
+ (completion-table-with-predicate
+ full-collection 'no-A t "" 'no-B t)))
+ (should-not
+ (member "banana"
+ (completion-table-with-predicate
+ full-collection 'no-A t "" 'no-B t)))
+ ;; "apple" should still match when strict is nil
+ (should (eq t (try-completion
+ "apple"
+ (apply-partially
+ 'completion-table-with-predicate
+ full-collection 'no-A nil)
+ 'no-B)))
+ ;; "apple" should still match when strict is nil and pred2 is nil
+ ;; (Bug#27841)
+ (should (eq t (try-completion
+ "apple"
+ (apply-partially
+ 'completion-table-with-predicate
+ full-collection 'no-A nil))))))
+
(provide 'completion-tests)
;;; completion-tests.el ends here
--
2.13.4
next prev parent reply other threads:[~2017-08-05 6:22 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-26 18:36 bug#27841: 25.2; Patch for completion-table-with-predicate Ryan
2017-08-04 22:55 ` npostavs
2017-08-04 23:00 ` Ryan Thompson
2017-08-04 23:10 ` npostavs
2017-08-04 23:11 ` Dmitry Gutov
2017-08-05 6:22 ` Ryan Thompson [this message]
2017-08-08 1:27 ` npostavs
2017-11-08 2:30 ` Noam Postavsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAHCt_aZS4N_+jMKDV97PSGt00HwzuvEDTwgGwpiU0k6noX6_Mg@mail.gmail.com \
--to=rct@thompsonclan.org \
--cc=27841@debbugs.gnu.org \
--cc=dgutov@yandex.ru \
--cc=npostavs@users.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.