Subject: Re: bug#36981: 26.2; request: add searching by package name to list-packages

From: Federico Tedin <[email-masked]>
Date: Mon, 09 Sep 2019 22:54:56 +0200

Hi Štěpán, thanks for your very detailed feedback.

>> +  (if (or keywords (and packages (listp packages)))
>                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> AKA "(consp packages)", though maybe you find your version more
> descriptive.

Thanks, didn't know they were equivalent.

>> +(defun package-menu-search (name)
>> +  "Filter the *Packages* buffer.
>> +Show only those items whose name matches NAME. If NAME is nil or an
>> +empty string, show all packages.
>
> If I was reading it as a user, I would appreciate it if the doc string
> was more specific regarding the NAME argument, e.g. said "matches the
> regular expression NAME" or something to that effect.

> Also, I think "empty string" is usually prefixed with "the", not "an",
> as there is only one such thing, e.g. (eq "" "") => t. 

I've changed the docstring as you described, I think it's clearer now.

>> +To restore the full package list, type `q'."
>> +  (interactive
>> +   (list (read-from-minibuffer "Package name: ")
>> +         current-prefix-arg))
>             ^^^^^^^^^^^^^^^^^^
>
> Is this a remnant of some previous WIP version? The function now takes a
> single argument, right?

Correct! I forgot to remove it. I was experimenting with making 'C-u s'
search packages by name, but only considering packages that were already
listed. This way it was possible to combine the keyword search and name
search. I decided against it in the end because I'm not sure if it is a
necessary addition.

I'm attaching a new patch with some changes.

- Fede

>From e1a8d4ab5b03d5800e5c42c61cd2471e45f2a9f5 Mon Sep 17 00:00:00 2001
From: Federico Tedin <[email-masked]>
Date: Mon, 9 Sep 2019 22:48:36 +0200
Subject: [PATCH 1/1] Search packages by name in list-packages (Bug#36981)

* lisp/emacs-lisp/package.el (package-menu-search): Added a new
function that allows filtering packages by name.
(package-menu--generate): Show full packages list with 'q' if current
list has been filtered.
(package-menu-mode-map): Bind 's' to package-menu-search.
* test/lisp/emacs-lisp/package-tests.el (package-test-list-search):
Added a test for package-menu-search.
* doc/emacs/package.texi: Document usage of 'package-menu-search'.
* etc/NEWS: Announce changes.
---
 doc/emacs/package.texi                |  5 +++++
 etc/NEWS                              |  4 ++++
 lisp/emacs-lisp/package.el            | 28 +++++++++++++++++++++++++--
 test/lisp/emacs-lisp/package-tests.el | 11 ++++++++++-
 4 files changed, 45 insertions(+), 3 deletions(-)

diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
index 4b33f250c4..84de3a5103 100644
--- a/doc/emacs/package.texi
+++ b/doc/emacs/package.texi
@@ -142,6 +142,11 @@ Package Menu
 that relate to that keyword.  To restore the full package list,
 type @kbd{q}.
 
+@item s
+Search packages by name (@code{package-menu-search}).  This prompts
+for a string, then shows only the packages whose names match a regexp
+with that value.  To restore the full package list, type @kbd{q}.
+
 @item H
 Permanently hide packages that match a regexp
 (@code{package-menu-hide-package}).
diff --git a/etc/NEWS b/etc/NEWS
index 87666740df..6554dc1f79 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -968,6 +968,10 @@ early init file.
 
 *** New function 'package-activate-all'.
 
+*** New function 'package-menu-search'.
+Allows users to search packages by name on the packages list. By
+default, it is bound to 's'.
+
 ---
 *** Imenu support has been added to 'package-menu-mode'.
 
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ef0c5171de..d1ff7b964d 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2621,6 +2621,7 @@ package-menu-mode-map
     (define-key map "U" 'package-menu-mark-upgrades)
     (define-key map "r" 'package-menu-refresh)
     (define-key map "f" 'package-menu-filter)
+    (define-key map "s" 'package-menu-search)
     (define-key map "~" 'package-menu-mark-obsolete-for-deletion)
     (define-key map "x" 'package-menu-execute)
     (define-key map "h" 'package-menu-quick-help)
@@ -2652,7 +2653,8 @@ package-menu-mode-map
     ["Unmark" package-menu-mark-unmark :help "Clear any marks on a package and move to the next line"]
 
     "--"
-    ["Filter Package List" package-menu-filter :help "Filter package selection (q to go back)"]
+    ["Filter Packages by Keywords" package-menu-filter :help "Filter packages by keywords (q to go back)"]
+    ["Filter Packages by Name" package-menu-search :help "Filter packages by name (q to go back)"]
     ["Hide by Regexp" package-menu-hide-package :help "Permanently hide all packages matching a regexp"]
     ["Display Older Versions" package-menu-toggle-hiding
      :style toggle :selected (not package-menu--hide-packages)
@@ -2963,7 +2965,7 @@ package-menu--generate
             (let ((filters (mapconcat #'identity keywords ",")))
               (concat "Package[" filters "]"))
           "Package"))
-  (if keywords
+  (if (or keywords (consp packages))
       (define-key package-menu-mode-map "q" 'package-show-package-list)
     (define-key package-menu-mode-map "q" 'quit-window))
   (tabulated-list-init-header)
@@ -3598,6 +3600,28 @@ package-menu-filter
                                    (list keyword)
                                  keyword)))
 
+(defun package-menu-search (name)
+  "Filter the *Packages* buffer.
+Show only those items whose name matches the regular expression
+NAME. If NAME is nil or the empty string, show all packages.
+
+To restore the full package list, type `q'."
+  (interactive (list (read-from-minibuffer "Package name: ")))
+  (if (or (not name) (string-empty-p name))
+      (package-show-package-list t nil)
+    ;; Update `tabulated-list-entries' so that in contains all
+    ;; packages before searching.
+    (package-menu--refresh t nil)
+    (let (matched)
+      (dolist (entry tabulated-list-entries)
+        (let* ((pkg-name-sym (package-desc-name (car entry)))
+               (pkg-name (symbol-name pkg-name-sym)))
+          (when (string-match name pkg-name)
+            (push pkg-name-sym matched))))
+      (if matched
+          (package-show-package-list matched nil)
+        (user-error "No packages found")))))
+
 (defun package-list-packages-no-fetch ()
   "Display a list of packages.
 Does not fetch the updated list of packages before displaying.
diff --git a/test/lisp/emacs-lisp/package-tests.el b/test/lisp/emacs-lisp/package-tests.el
index c757bccf67..ea28db83ce 100644
--- a/test/lisp/emacs-lisp/package-tests.el
+++ b/test/lisp/emacs-lisp/package-tests.el
@@ -28,7 +28,7 @@
 
 ;; Run this in a clean Emacs session using:
 ;;
-;;     $ emacs -Q --batch -L . -l package-test.el -l ert -f ert-run-tests-batch-and-exit
+;;     $ emacs -Q --batch -L . -l package-tests.el -l ert -f ert-run-tests-batch-and-exit
 
 ;;; Code:
 
@@ -360,6 +360,15 @@ package-test--compatible-p
       (should-not (re-search-forward "^\\s-+simple-single\\s-+1.3\\s-+\\(available\\|new\\)" nil t))
       (kill-buffer buf))))
 
+(ert-deftest package-test-list-search ()
+  "Ensure package list is filtered correctly by package name."
+  (with-package-test ()
+    (let ((buf (package-list-packages)))
+      (package-menu-search "tetris")
+      (should (= (length tabulated-list-entries) 1))
+      (should (eq (package-desc-name (caar tabulated-list-entries)) 'tetris))
+      (kill-buffer buf))))
+
 (ert-deftest package-test-update-archives ()
   "Test updating package archives."
   (with-package-test ()
-- 
2.17.1