unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#36981: 26.2; request: add searching by package name to list-packages
@ 2019-08-09  7:21 ndame
  2019-09-08  0:47 ` Federico Tedin
  0 siblings, 1 reply; 17+ messages in thread
From: ndame @ 2019-08-09  7:21 UTC (permalink / raw)
  To: 36981

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

When installing a package from list-packages it's often
inconvenient to find a package which has a string which occurs
many times in the packages buffer. (e.g. in the descriptions).

The package buffer has f for filtering, but it filters only for
package keywords, not names.

Add a new command s for search which searches (or filters) the
package list by a substring match on the package name.
 

[-- Attachment #2: Type: text/html, Size: 459 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-08-09  7:21 bug#36981: 26.2; request: add searching by package name to list-packages ndame
@ 2019-09-08  0:47 ` Federico Tedin
  2019-09-08 15:20   ` Štěpán Němec
  2019-09-16  0:29   ` Stefan Kangas
  0 siblings, 2 replies; 17+ messages in thread
From: Federico Tedin @ 2019-09-08  0:47 UTC (permalink / raw)
  To: ndame; +Cc: 36981

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

ndame <emacsuser@freemail.hu> writes:

> When installing a package from list-packages it's often
> inconvenient to find a package which has a string which occurs
> many times in the packages buffer. (e.g. in the descriptions).
>
> The package buffer has f for filtering, but it filters only for
> package keywords, not names.
>
> Add a new command s for search which searches (or filters) the
> package list by a substring match on the package name.

I've implemented this feature using the "s" key as you suggested. I'm
attaching a patch with my changes.

- Fede


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 4830 bytes --]

From cb14b951949e26bea8c12b202b70754271d9f4e2 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Sun, 8 Sep 2019 02:38:43 +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.
---
 lisp/emacs-lisp/package.el            | 30 +++++++++++++++++++++++++--
 test/lisp/emacs-lisp/package-tests.el | 11 +++++++++-
 2 files changed, 38 insertions(+), 3 deletions(-)

diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ef0c5171de..0993015fde 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 (and packages (listp 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,30 @@ package-menu-filter
                                    (list keyword)
                                  keyword)))
 
+(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.
+
+To restore the full package list, type `q'."
+  (interactive
+   (list (read-from-minibuffer "Package name: ")
+         current-prefix-arg))
+  (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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-08  0:47 ` Federico Tedin
@ 2019-09-08 15:20   ` Štěpán Němec
  2019-09-09 20:54     ` Federico Tedin
  2019-09-16  0:29   ` Stefan Kangas
  1 sibling, 1 reply; 17+ messages in thread
From: Štěpán Němec @ 2019-09-08 15:20 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 36981, ndame


I don't use package.el, but here are a few observations:

> From: Federico Tedin <federicotedin@gmail.com>
> Date: Sun, 8 Sep 2019 02:38:43 +0200
> Subject: [PATCH 1/1] Search packages by name in list-packages (Bug#36981)
>
> +  (if (or keywords (and packages (listp packages)))
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AKA "(consp packages)", though maybe you find your version more
descriptive.

> +(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. 

> +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?

Thanks!

-- 
Štěpán





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-08 15:20   ` Štěpán Němec
@ 2019-09-09 20:54     ` Federico Tedin
  2019-09-14 11:15       ` Federico Tedin
  0 siblings, 1 reply; 17+ messages in thread
From: Federico Tedin @ 2019-09-09 20:54 UTC (permalink / raw)
  To: Štěpán Němec; +Cc: 36981, ndame

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

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 5972 bytes --]

From e1a8d4ab5b03d5800e5c42c61cd2471e45f2a9f5 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-09 20:54     ` Federico Tedin
@ 2019-09-14 11:15       ` Federico Tedin
  2019-09-18 23:17         ` Stefan Kangas
  0 siblings, 1 reply; 17+ messages in thread
From: Federico Tedin @ 2019-09-14 11:15 UTC (permalink / raw)
  To: Štěpán Němec; +Cc: 36981, ndame

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

Forgot to add '+++' above my NEWS entry, sending a new patch.

- Federico


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 5995 bytes --]

From 419c60d1d96aa0af1cece39c1aeebb6ae95a558d Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Sat, 14 Sep 2019 13:02:07 +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 by name as well.
(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                              |  5 +++++
 lisp/emacs-lisp/package.el            | 28 +++++++++++++++++++++++++--
 test/lisp/emacs-lisp/package-tests.el | 11 ++++++++++-
 4 files changed, 46 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 94c98a7ebe..37eb23c4a9 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -968,6 +968,11 @@ 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


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-08  0:47 ` Federico Tedin
  2019-09-08 15:20   ` Štěpán Němec
@ 2019-09-16  0:29   ` Stefan Kangas
  2019-09-18 16:38     ` Federico Tedin
  1 sibling, 1 reply; 17+ messages in thread
From: Stefan Kangas @ 2019-09-16  0:29 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 36981, ndame

Federico Tedin <federicotedin@gmail.com> writes:

> ndame <emacsuser@freemail.hu> writes:
>
> > When installing a package from list-packages it's often
> > inconvenient to find a package which has a string which occurs
> > many times in the packages buffer. (e.g. in the descriptions).
> >
> > The package buffer has f for filtering, but it filters only for
> > package keywords, not names.
> >
> > Add a new command s for search which searches (or filters) the
> > package list by a substring match on the package name.
>
> I've implemented this feature using the "s" key as you suggested. I'm
> attaching a patch with my changes.

Thanks for working on this.

My first observation is that the search you have implemented works
very much like the package-menu-filter command bound to "f".

Maybe it would be better to put all the filtering commands on "/", and
use something like the following key bindings:

"/ n" - package-menu-search  [from "s"]
"/ k" - package-menu-filter  [from "f"]

And add the new command and key binding:

"/ /" - package-menu-clear-filter

This would be more in line with filtering in ibuffer, bookmark-bmenu,
and others.  Later, we could add more filters (for example to filter
by status, archive, time since update, etc.).  But then we probably
want to rename as follows:

package-menu-search => package-menu-filter-by-name
package-menu-filter => package-menu-filter-by-keyword

Also, this would free up the "s" key for a sorting command, which is
in line with the key binding in e.g. dired.

I also have some other, related ideas:

1. How about implementing an option to search only the package names
using isearch?  Compare to dired-isearch-filenames.

2. How about making the filtering for both package-menu-search and
package-menu-filter interactive -- that is, that the commands shows
the results as you type keys? One could look at bookmark-bmenu-search
for inspiration.

Best regards,
Stefan Kangas





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-16  0:29   ` Stefan Kangas
@ 2019-09-18 16:38     ` Federico Tedin
  2019-09-18 22:22       ` Stefan Kangas
  0 siblings, 1 reply; 17+ messages in thread
From: Federico Tedin @ 2019-09-18 16:38 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 36981, ndame

Hi Stefan, thanks for your input.

> Thanks for working on this.
>
> My first observation is that the search you have implemented works
> very much like the package-menu-filter command bound to "f".
>
> Maybe it would be better to put all the filtering commands on "/", and
> use something like the following key bindings:
>
> "/ n" - package-menu-search  [from "s"]
> "/ k" - package-menu-filter  [from "f"]
>
> And add the new command and key binding:
>
> "/ /" - package-menu-clear-filter
>
> This would be more in line with filtering in ibuffer, bookmark-bmenu,
> and others.  Later, we could add more filters (for example to filter
> by status, archive, time since update, etc.).  But then we probably
> want to rename as follows:
>
> package-menu-search => package-menu-filter-by-name
> package-menu-filter => package-menu-filter-by-keyword
>
> Also, this would free up the "s" key for a sorting command, which is
> in line with the key binding in e.g. dired.

I think the "/" prefix keybindings are a good idea; however I'm not sure
if it's OK to change an already existing keybinding. But on the other
hand this is a really minor thing so I imagine it's not that much of a
problem. The new names you chose for the commands also make more sense
than the current ones, I don't have a problem with updating them.

> I also have some other, related ideas:
>
> 1. How about implementing an option to search only the package names
> using isearch?  Compare to dired-isearch-filenames.

Didn't know about that command, looks really useful. The only problem I
can think of implementing search that way is that it would stop being
similar to `package-menu-filter', in the sense that it currently only
shows packages that have matched the search, instead of jumping between
packages that match. On the other hand, the way I've implemented the
search is really primitive, so I guess using isearch would actually make
it much more powerful.

> 2. How about making the filtering for both package-menu-search and
> package-menu-filter interactive -- that is, that the commands shows
> the results as you type keys? One could look at bookmark-bmenu-search
> for inspiration.

Nice! This is the first command in Emacs that I've used that has
incremental search.  For this patch though I would like not to modify
package-menu-filter and just stick to adding package-menu-search. For
package-menu-search I like more the idea of making the command be just a
regular filter (like it is now) or have it use isearch, like you
suggested.

- Federico





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-18 16:38     ` Federico Tedin
@ 2019-09-18 22:22       ` Stefan Kangas
  2019-09-19 20:29         ` Juri Linkov
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Kangas @ 2019-09-18 22:22 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 36981, ndame

Federico Tedin <federicotedin@gmail.com> writes:

> Hi Stefan, thanks for your input.

Hi again, and thanks for considering it.

> I think the "/" prefix keybindings are a good idea; however I'm not sure
> if it's OK to change an already existing keybinding. But on the other
> hand this is a really minor thing so I imagine it's not that much of a
> problem. The new names you chose for the commands also make more sense
> than the current ones, I don't have a problem with updating them.

It's neither a critical key binding nor a very long-standing one, so I
don't see the harm in changing it to a more logical one.  We'd have to
announce the change in NEWS though.  Of course, I'm not in any
position to make any decisions around here, so that's just my opinion.
If you agree, perhaps you'd be interested in adding that change to
your suggested patch.

On the other hand, just sticking to implementing the already suggested
command would also be an improvement.

But might I suggest that you in any case name the command
package-menu-filter-by-name?  It seems to me that it better describe
what this function does.

> > 1. How about implementing an option to search only the package names
> > using isearch?  Compare to dired-isearch-filenames.
>
> Didn't know about that command, looks really useful. The only problem I
> can think of implementing search that way is that it would stop being
> similar to `package-menu-filter', in the sense that it currently only
> shows packages that have matched the search, instead of jumping between
> packages that match. On the other hand, the way I've implemented the
> search is really primitive, so I guess using isearch would actually make
> it much more powerful.

I don't want to discourage you from continuing work on the filter
command by floating this idea.  Having filter commands and an isearch
option are not mutually exclusive.  On the contrary, I could see
myself wanting to use both.

Perhaps it's better to regard this idea as separate from the "filter
by name" command for now.  Maybe I shouldn't have sidetracked this
issue by even raising it, but it's good to hear that you agree that
it's not a bad idea.

> > 2. How about making the filtering for both package-menu-search and
> > package-menu-filter interactive -- that is, that the commands shows
> > the results as you type keys? One could look at bookmark-bmenu-search
> > for inspiration.
>
> Nice! This is the first command in Emacs that I've used that has
> incremental search.  For this patch though I would like not to modify
> package-menu-filter and just stick to adding package-menu-search.

Sounds good to me.  We could always revisit the idea of making them
incremental later.

Best regards,
Stefan Kangas





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-14 11:15       ` Federico Tedin
@ 2019-09-18 23:17         ` Stefan Kangas
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Kangas @ 2019-09-18 23:17 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 36981, ndame, Štěpán Němec

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

Hi Federico,

Here's a more detailed review of your patch.

Federico Tedin <federicotedin@gmail.com> writes:

> Subject: [PATCH 1/1] Search packages by name in list-packages (Bug#36981)

Perhaps instead: "Filter packages by name in list-packages."

> * lisp/emacs-lisp/package.el (package-menu-search): Added a new
> function that allows filtering packages by name.

Or, shorter: "New function to filter packages by name."

> (package-menu--generate): Show full packages list with 'q' if current
> list has been filtered by name as well.
> (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.

We prefer the imperative rather than the past tense in commit messages,
so that should just be "Add".

> +Search packages by name (@code{package-menu-search}).  This prompts

Suggest to change "Search" to "Filter".

> +(defun package-menu-search (name)

Suggest to rename to package-menu-filter-by-name

> +  "Filter the *Packages* buffer.

I suggest "Filter the \"*Packages\" buffer by NAME.

> +  (interactive (list (read-from-minibuffer "Package name: ")))

I suggest "Filter by name (regexp): "

> +  (if (or (not name) (string-empty-p name))
> +      (package-show-package-list t nil)
> +    ;; Update `tabulated-list-entries' so that in contains all

"in" -> "it"

> +    (let (matched)
> +      (dolist (entry tabulated-list-entries)
> +        (let* ((pkg-name-sym (package-desc-name (car entry)))
> +               (pkg-name (symbol-name pkg-name-sym)))

This is nitpicking, but perhaps it would be easier to read if you use
only one variable here.  That means to keep "pkg-name-sym" but rename it
to "pkg-name", and...

> +          (when (string-match name pkg-name)

... changing this to (when (string-match name (symbol-name pkg-name))

That would be preference, anyways.

> +(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))))

Wouldn't it be better to verify the buffer contents here?  That way we
it's less coupled to the implementation of tabulated-list-mode.

Best regards,
Stefan Kangas

[-- Attachment #2: Type: text/html, Size: 3064 bytes --]

^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-18 22:22       ` Stefan Kangas
@ 2019-09-19 20:29         ` Juri Linkov
  2019-09-20 18:49           ` Federico Tedin
  0 siblings, 1 reply; 17+ messages in thread
From: Juri Linkov @ 2019-09-19 20:29 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 36981, ndame, Federico Tedin

> I don't want to discourage you from continuing work on the filter
> command by floating this idea.  Having filter commands and an isearch
> option are not mutually exclusive.  On the contrary, I could see
> myself wanting to use both.
>
> Perhaps it's better to regard this idea as separate from the "filter
> by name" command for now.  Maybe I shouldn't have sidetracked this
> issue by even raising it, but it's good to hear that you agree that
> it's not a bad idea.

I agree, let's first have a basic implementation of "filter by name",
and then maybe add an isearch option later.  The way currently I have
to search by name is to start isearch, type a name, and open occur
from isearch.  Then jumping from the *Occur* buffer back to packages
is so ugly workaround for having a proper filter by name.
Filtering a huge list of packages will improve the current situation.
(Another useful command would be "filter by description and name"
like in Synaptic package manager.)





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-19 20:29         ` Juri Linkov
@ 2019-09-20 18:49           ` Federico Tedin
  2019-09-20 22:09             ` Stefan Kangas
  0 siblings, 1 reply; 17+ messages in thread
From: Federico Tedin @ 2019-09-20 18:49 UTC (permalink / raw)
  To: Juri Linkov; +Cc: 36981, ndame, Stefan Kangas

>> I don't want to discourage you from continuing work on the filter
>> command by floating this idea.  Having filter commands and an isearch
>> option are not mutually exclusive.  On the contrary, I could see
>> myself wanting to use both.
>>
>> Perhaps it's better to regard this idea as separate from the "filter
>> by name" command for now.  Maybe I shouldn't have sidetracked this
>> issue by even raising it, but it's good to hear that you agree that
>> it's not a bad idea.
>
> I agree, let's first have a basic implementation of "filter by name",
> and then maybe add an isearch option later.  The way currently I have
> to search by name is to start isearch, type a name, and open occur
> from isearch.  Then jumping from the *Occur* buffer back to packages
> is so ugly workaround for having a proper filter by name.
> Filtering a huge list of packages will improve the current situation.
> (Another useful command would be "filter by description and name"
> like in Synaptic package manager.)

No problem then, I'll change the function names as Stefan suggested:

package-menu-search => package-menu-filter-by-name
package-menu-filter => package-menu-filter-by-keyword

Then, I'll add the new keybindings and also address the feedback he gave
me regarding my last patch. Thanks for the feedback!





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-20 18:49           ` Federico Tedin
@ 2019-09-20 22:09             ` Stefan Kangas
  2019-09-26 18:28               ` Federico Tedin
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Kangas @ 2019-09-20 22:09 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 36981, ndame, Juri Linkov

Federico Tedin <federicotedin@gmail.com> writes:

> No problem then, I'll change the function names as Stefan suggested:
>
> package-menu-search => package-menu-filter-by-name
> package-menu-filter => package-menu-filter-by-keyword
>
> Then, I'll add the new keybindings and also address the feedback he gave
> me regarding my last patch. Thanks for the feedback!

Sounds great, thanks.

Best regards,
Stefan Kangas





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-20 22:09             ` Stefan Kangas
@ 2019-09-26 18:28               ` Federico Tedin
  2019-09-27 10:32                 ` Stefan Kangas
  0 siblings, 1 reply; 17+ messages in thread
From: Federico Tedin @ 2019-09-26 18:28 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 36981, ndame, Juri Linkov

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

Stefan, Juri, here's a new version of the patch for filtering packages
by name.

Because I added the new function `package-menu-clear-filter' (bound to
'/ /'), I decided to remove the behavior in `package-menu--generate'
that previously bound 'q' to clearing the filter. I think it's better
the way it is now because the user can now bind
`package-menu-clear-filter' to whatever they like.

Thanks!

- Federico


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 9131 bytes --]

From c38a35406a9c76d5bf5479d01aee169a056a254c Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Thu, 26 Sep 2019 19:18:58 +0200
Subject: [PATCH 1/1] Filter packages by name in list-packages. (Bug#36981)

* lisp/emacs-lisp/package.el (package-menu-filter-by-name): New
function to filter packages by name.
(package-menu-clear-filter): New function to clear applied filters.
(package-menu-filter-by-keyword): Rename function from
package-menu-filter.
(package-menu--generate): Don't change 'q' binding anymore.
(package-menu-mode-map): Bind '/ n' to package-menu-filter-by-name, '/
k' to package-menu-filter-by-keyword and '/ /' to
package-menu-clear-filter.
(package-menu-mode-menu): Update menu entries for the three functions.
* test/lisp/emacs-lisp/package-tests.el (package-test-list-filter-by-name):
Add a test for package-menu-filter-by-name.
(package-test-list-clear-filter): Add a test for
package-menu-clear-filter.
* doc/emacs/package.texi: Document usage of
package-menu-filter-by-name, package-menu-clear-filter and update
reference to package-menu-filter-by-keyword.
* etc/NEWS: Announce changes.
---
 doc/emacs/package.texi                | 19 ++++++++----
 etc/NEWS                              | 16 ++++++++++
 lisp/emacs-lisp/package.el            | 43 ++++++++++++++++++++-------
 test/lisp/emacs-lisp/package-tests.el | 24 ++++++++++++++-
 4 files changed, 85 insertions(+), 17 deletions(-)

diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
index 2c09ca8902..f7ff4f01a1 100644
--- a/doc/emacs/package.texi
+++ b/doc/emacs/package.texi
@@ -136,11 +136,20 @@ Package Menu
 the list of available packages from the package archive again, and
 recomputes the package list.
 
-@item f
-Filter the package list (@code{package-menu-filter}).  This prompts
-for a keyword (e.g., @samp{games}), then shows only the packages
-that relate to that keyword.  To restore the full package list,
-type @kbd{q}.
+@item / k
+Filter the package list by keyword
+(@code{package-menu-filter-by-keyword}).  This prompts for a keyword
+(e.g., @samp{games}), then shows only the packages that relate to that
+keyword.
+
+@item / s
+Filter the package list by name (@code{package-menu-filter-by-name}).
+This prompts for a string, then shows only the packages whose names
+match a regexp with that value.
+
+@item / /
+Clear filter currently applied to the package list
+(@code{package-menu-clear-filter}).
 
 @item H
 Permanently hide packages that match a regexp
diff --git a/etc/NEWS b/etc/NEWS
index 09394432aa..75b01f7372 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -974,6 +974,22 @@ early init file.
 
 *** New function 'package-activate-all'.
 
++++
+*** New function 'package-menu-filter-by-name'.
+Allows users to filter packages by name on the packages list.  By
+default, it is bound to '/ n'.  To clear the filter, use '/ /'.
+
++++
+*** Function 'package-menu-fiter-by-keyword' has been renamed
+from 'package-menu-filter'.  Its keybinding has also been changed to
+'/ k' (from 'f').  To clear the filter, '/ /' must now be used instead
+of 'q'.
+
++++
+*** New function 'package-menu-clear-filter'
+Allows users to clear a filter applied with 'package-menu-filter-by-name' or
+'package-menu-filter-by-keyword'.
+
 ---
 *** Imenu support has been added to 'package-menu-mode'.
 
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index a8362cb205..ef3da5298c 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2667,7 +2667,9 @@ package-menu-mode-map
     (define-key map "i" 'package-menu-mark-install)
     (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 (kbd "/ k") 'package-menu-filter-by-keyword)
+    (define-key map (kbd "/ n") 'package-menu-filter-by-name)
+    (define-key map (kbd "/ /") 'package-menu-clear-filter)
     (define-key map "~" 'package-menu-mark-obsolete-for-deletion)
     (define-key map "x" 'package-menu-execute)
     (define-key map "h" 'package-menu-quick-help)
@@ -2699,7 +2701,9 @@ 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 Keyword" package-menu-filter-by-keyword :help "Filter packages by keyword"]
+    ["Filter Packages by Name" package-menu-filter-by-name :help "Filter packages by name"]
+    ["Clear Packages Filter" package-menu-clear-filter :help "Clear package list filter"]
     ["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)
@@ -3010,9 +3014,6 @@ package-menu--generate
             (let ((filters (mapconcat #'identity keywords ",")))
               (concat "Package[" filters "]"))
           "Package"))
-  (if keywords
-      (define-key package-menu-mode-map "q" 'package-show-package-list)
-    (define-key package-menu-mode-map "q" 'quit-window))
   (tabulated-list-init-header)
   (tabulated-list-print remember-pos))
 
@@ -3627,9 +3628,7 @@ package-show-package-list
         (select-window win)
       (switch-to-buffer buf))))
 
-;; package-menu--generate rebinds "q" on the fly, so we have to
-;; hard-code the binding in the doc-string here.
-(defun package-menu-filter (keyword)
+(defun package-menu-filter-by-keyword (keyword)
   "Filter the *Packages* buffer.
 Show only those items that relate to the specified KEYWORD.
 
@@ -3640,9 +3639,7 @@ package-menu-filter
 KEYWORD can also be used to filter by status or archive name by
 using keywords like \"arc:gnu\" and \"status:available\".
 Statuses available include \"incompat\", \"available\",
-\"built-in\" and \"installed\".
-
-To restore the full package list, type `q'."
+\"built-in\" and \"installed\"."
   (interactive
    (list (completing-read-multiple
           "Keywords (comma separated): " (package-all-keywords))))
@@ -3650,6 +3647,30 @@ package-menu-filter
                                    (list keyword)
                                  keyword)))
 
+(defun package-menu-filter-by-name (name)
+  "Filter the *Packages* buffer by NAME.
+Show only those items whose name matches the regular expression
+NAME. If NAME is nil or the empty string, show all packages."
+  (interactive (list (read-from-minibuffer "Filter by name (regexp): ")))
+  (if (or (not name) (string-empty-p name))
+      (package-show-package-list t nil)
+    ;; Update `tabulated-list-entries' so that it contains all
+    ;; packages before searching.
+    (package-menu--refresh t nil)
+    (let (matched)
+      (dolist (entry tabulated-list-entries)
+        (let* ((pkg-name (package-desc-name (car entry))))
+          (when (string-match name (symbol-name pkg-name))
+            (push pkg-name matched))))
+      (if matched
+          (package-show-package-list matched nil)
+        (user-error "No packages found")))))
+
+(defun package-menu-clear-filter ()
+  "Clear any filter currently applied to the *Packages* buffer."
+  (interactive)
+  (package-menu--generate t t))
+
 (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..a2ee9f06b1 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,28 @@ 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-filter-by-name ()
+  "Ensure package list is filtered correctly by package name."
+  (with-package-test ()
+    (let ((buf (package-list-packages)))
+      (package-menu-filter-by-name "tetris")
+      (goto-char (point-min))
+      (should (re-search-forward "^\\s-+tetris" nil t))
+      (should (= (count-lines (point-min) (point-max)) 1))
+      (kill-buffer buf))))
+
+(ert-deftest package-test-list-clear-filter ()
+  "Ensure package list filter is cleared correctly."
+  (with-package-test ()
+    (let ((buf (package-list-packages)))
+      (let ((num-packages (count-lines (point-min) (point-max))))
+        (should (> num-packages 1))
+        (package-menu-filter-by-name "tetris")
+        (should (= (count-lines (point-min) (point-max)) 1))
+        (package-menu-clear-filter)
+        (should (= (count-lines (point-min) (point-max)) num-packages)))
+      (kill-buffer buf))))
+
 (ert-deftest package-test-update-archives ()
   "Test updating package archives."
   (with-package-test ()
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-26 18:28               ` Federico Tedin
@ 2019-09-27 10:32                 ` Stefan Kangas
  2019-09-30 21:26                   ` Federico Tedin
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Kangas @ 2019-09-27 10:32 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 36981, ndame, Juri Linkov

Federico Tedin <federicotedin@gmail.com> writes:

> Stefan, Juri, here's a new version of the patch for filtering packages
> by name.

Thanks.  Looks good to me, and the code seems to work as announced.
Nit-picks below.

> Because I added the new function `package-menu-clear-filter' (bound to
> '/ /'), I decided to remove the behavior in `package-menu--generate'
> that previously bound 'q' to clearing the filter. I think it's better
> the way it is now because the user can now bind
> `package-menu-clear-filter' to whatever they like.

Yes, that sounds like the right thing.

> * test/lisp/emacs-lisp/package-tests.el (package-test-list-filter-by-name):
> Add a test for package-menu-filter-by-name.
> (package-test-list-clear-filter): Add a test for
> package-menu-clear-filter.

Perhaps just:

* test/lisp/emacs-lisp/package-tests.el (package-test-list-filter-by-name)
(package-test-list-clear-filter): New tests.

> +@item / s
> +Filter the package list by name (@code{package-menu-filter-by-name}).

That should be "/ n" as below.

> ++++
> +*** New function 'package-menu-filter-by-name'.
> +Allows users to filter packages by name on the packages list.  By
> +default, it is bound to '/ n'.  To clear the filter, use '/ /'.
> +
> ++++
> +*** Function 'package-menu-fiter-by-keyword' has been renamed
> +from 'package-menu-filter'.  Its keybinding has also been changed to
> +'/ k' (from 'f').  To clear the filter, '/ /' must now be used instead
> +of 'q'.
> +
> ++++
> +*** New function 'package-menu-clear-filter'
> +Allows users to clear a filter applied with 'package-menu-filter-by-name' or
> +'package-menu-filter-by-keyword'.

Perhaps this would be better as just one item where all changes are
explained together?

> @@ -2699,7 +2701,9 @@ 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 Keyword" package-menu-filter-by-keyword :help "Filter packages by keyword"]
> +    ["Filter Packages by Name" package-menu-filter-by-name :help "Filter packages by name"]
> +    ["Clear Packages Filter" package-menu-clear-filter :help "Clear package list filter"]

This is fine by me, but perhaps it would be better to add a submenu
for filtering?

Best regards,
Stefan Kangas





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-27 10:32                 ` Stefan Kangas
@ 2019-09-30 21:26                   ` Federico Tedin
  2019-09-30 21:59                     ` Stefan Kangas
  0 siblings, 1 reply; 17+ messages in thread
From: Federico Tedin @ 2019-09-30 21:26 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: 36981

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

Hey Stefan, thanks again for the input.

> Perhaps just:
>
> * test/lisp/emacs-lisp/package-tests.el (package-test-list-filter-by-name)
> (package-test-list-clear-filter): New tests.
>
>> +@item / s
>> +Filter the package list by name (@code{package-menu-filter-by-name}).
>
> That should be "/ n" as below.

Good catch!

>> ++++
>> +*** New function 'package-menu-filter-by-name'.
>> +Allows users to filter packages by name on the packages list.  By
>> +default, it is bound to '/ n'.  To clear the filter, use '/ /'.
>> +
>> ++++
>> +*** Function 'package-menu-fiter-by-keyword' has been renamed
>> +from 'package-menu-filter'.  Its keybinding has also been changed to
>> +'/ k' (from 'f').  To clear the filter, '/ /' must now be used instead
>> +of 'q'.
>> +
>> ++++
>> +*** New function 'package-menu-clear-filter'
>> +Allows users to clear a filter applied with 'package-menu-filter-by-name' or
>> +'package-menu-filter-by-keyword'.
>
> Perhaps this would be better as just one item where all changes are
> explained together?
>
>> @@ -2699,7 +2701,9 @@ 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 Keyword" package-menu-filter-by-keyword :help "Filter packages by keyword"]
>> +    ["Filter Packages by Name" package-menu-filter-by-name :help "Filter packages by name"]
>> +    ["Clear Packages Filter" package-menu-clear-filter :help "Clear package list filter"]
>
> This is fine by me, but perhaps it would be better to add a submenu
> for filtering?

Good idea, I've changed the menu entries so that they're part of a
submenu now. I'm attaching the new patch.

Thanks!


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 8979 bytes --]

From a6cb17c6d99113d6f6a6b2a3164a9101c8fecb51 Mon Sep 17 00:00:00 2001
From: Federico Tedin <federicotedin@gmail.com>
Date: Thu, 26 Sep 2019 19:18:58 +0200
Subject: [PATCH 1/1] Filter packages by name in list-packages. (Bug#36981)

* lisp/emacs-lisp/package.el (package-menu-filter-by-name): New
function to filter packages by name.
(package-menu-clear-filter): New function to clear applied filters.
(package-menu-filter-by-keyword): Rename function from
package-menu-filter.
(package-menu--generate): Don't change 'q' binding anymore.
(package-menu-mode-map): Bind '/ n' to package-menu-filter-by-name, '/
k' to package-menu-filter-by-keyword and '/ /' to
package-menu-clear-filter.
(package-menu-mode-menu): Update menu entries for the three functions.
* test/lisp/emacs-lisp/package-tests.el (package-test-list-filter-by-name)
(package-test-list-clear-filter): New tests.
* doc/emacs/package.texi: Document usage of
package-menu-filter-by-name, package-menu-clear-filter and update
reference to package-menu-filter-by-keyword.
* etc/NEWS: Announce changes.
---
 doc/emacs/package.texi                | 19 ++++++++---
 etc/NEWS                              | 10 ++++++
 lisp/emacs-lisp/package.el            | 45 ++++++++++++++++++++-------
 test/lisp/emacs-lisp/package-tests.el | 24 +++++++++++++-
 4 files changed, 81 insertions(+), 17 deletions(-)

diff --git a/doc/emacs/package.texi b/doc/emacs/package.texi
index 2c09ca8902..d97648af1b 100644
--- a/doc/emacs/package.texi
+++ b/doc/emacs/package.texi
@@ -136,11 +136,20 @@ Package Menu
 the list of available packages from the package archive again, and
 recomputes the package list.
 
-@item f
-Filter the package list (@code{package-menu-filter}).  This prompts
-for a keyword (e.g., @samp{games}), then shows only the packages
-that relate to that keyword.  To restore the full package list,
-type @kbd{q}.
+@item / k
+Filter the package list by keyword
+(@code{package-menu-filter-by-keyword}).  This prompts for a keyword
+(e.g., @samp{games}), then shows only the packages that relate to that
+keyword.
+
+@item / n
+Filter the package list by name (@code{package-menu-filter-by-name}).
+This prompts for a string, then shows only the packages whose names
+match a regexp with that value.
+
+@item / /
+Clear filter currently applied to the package list
+(@code{package-menu-clear-filter}).
 
 @item H
 Permanently hide packages that match a regexp
diff --git a/etc/NEWS b/etc/NEWS
index cb8b6fcac1..015322c111 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -983,6 +983,16 @@ early init file.
 
 *** New function 'package-activate-all'.
 
++++
+*** New functions for filtering packages list
+A new function has been added which allows users to filter the
+packages list by name: 'package-menu-filter-by-name'.  By default, it
+is bound to '/ n'. Additionally, the function
+'package-menu-fiter-by-keyword' has been renamed from
+'package-menu-filter'.  Its keybinding has also been changed to '/ k'
+(from 'f').  To clear any of the two filters, the user can now call
+the 'package-menu-clear-filter' function, bound to '/ /' by default.
+
 ---
 *** Imenu support has been added to 'package-menu-mode'.
 
diff --git a/lisp/emacs-lisp/package.el b/lisp/emacs-lisp/package.el
index ab1fb8b90f..28e971044e 100644
--- a/lisp/emacs-lisp/package.el
+++ b/lisp/emacs-lisp/package.el
@@ -2667,7 +2667,9 @@ package-menu-mode-map
     (define-key map "i" 'package-menu-mark-install)
     (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 (kbd "/ k") 'package-menu-filter-by-keyword)
+    (define-key map (kbd "/ n") 'package-menu-filter-by-name)
+    (define-key map (kbd "/ /") 'package-menu-clear-filter)
     (define-key map "~" 'package-menu-mark-obsolete-for-deletion)
     (define-key map "x" 'package-menu-execute)
     (define-key map "h" 'package-menu-quick-help)
@@ -2699,7 +2701,11 @@ 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"
+     ["Filter by Keyword" package-menu-filter-by-keyword :help "Filter packages by keyword"]
+     ["Filter by Name" package-menu-filter-by-name :help "Filter packages by name"]
+     ["Clear Filter" package-menu-clear-filter :help "Clear package list filter"])
+
     ["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)
@@ -3010,9 +3016,6 @@ package-menu--generate
             (let ((filters (mapconcat #'identity keywords ",")))
               (concat "Package[" filters "]"))
           "Package"))
-  (if keywords
-      (define-key package-menu-mode-map "q" 'package-show-package-list)
-    (define-key package-menu-mode-map "q" 'quit-window))
   (tabulated-list-init-header)
   (tabulated-list-print remember-pos))
 
@@ -3640,9 +3643,7 @@ package-show-package-list
         (select-window win)
       (switch-to-buffer buf))))
 
-;; package-menu--generate rebinds "q" on the fly, so we have to
-;; hard-code the binding in the doc-string here.
-(defun package-menu-filter (keyword)
+(defun package-menu-filter-by-keyword (keyword)
   "Filter the *Packages* buffer.
 Show only those items that relate to the specified KEYWORD.
 
@@ -3653,9 +3654,7 @@ package-menu-filter
 KEYWORD can also be used to filter by status or archive name by
 using keywords like \"arc:gnu\" and \"status:available\".
 Statuses available include \"incompat\", \"available\",
-\"built-in\" and \"installed\".
-
-To restore the full package list, type `q'."
+\"built-in\" and \"installed\"."
   (interactive
    (list (completing-read-multiple
           "Keywords (comma separated): " (package-all-keywords))))
@@ -3663,6 +3662,30 @@ package-menu-filter
                                    (list keyword)
                                  keyword)))
 
+(defun package-menu-filter-by-name (name)
+  "Filter the *Packages* buffer by NAME.
+Show only those items whose name matches the regular expression
+NAME. If NAME is nil or the empty string, show all packages."
+  (interactive (list (read-from-minibuffer "Filter by name (regexp): ")))
+  (if (or (not name) (string-empty-p name))
+      (package-show-package-list t nil)
+    ;; Update `tabulated-list-entries' so that it contains all
+    ;; packages before searching.
+    (package-menu--refresh t nil)
+    (let (matched)
+      (dolist (entry tabulated-list-entries)
+        (let* ((pkg-name (package-desc-name (car entry))))
+          (when (string-match name (symbol-name pkg-name))
+            (push pkg-name matched))))
+      (if matched
+          (package-show-package-list matched nil)
+        (user-error "No packages found")))))
+
+(defun package-menu-clear-filter ()
+  "Clear any filter currently applied to the *Packages* buffer."
+  (interactive)
+  (package-menu--generate t t))
+
 (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..a2ee9f06b1 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,28 @@ 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-filter-by-name ()
+  "Ensure package list is filtered correctly by package name."
+  (with-package-test ()
+    (let ((buf (package-list-packages)))
+      (package-menu-filter-by-name "tetris")
+      (goto-char (point-min))
+      (should (re-search-forward "^\\s-+tetris" nil t))
+      (should (= (count-lines (point-min) (point-max)) 1))
+      (kill-buffer buf))))
+
+(ert-deftest package-test-list-clear-filter ()
+  "Ensure package list filter is cleared correctly."
+  (with-package-test ()
+    (let ((buf (package-list-packages)))
+      (let ((num-packages (count-lines (point-min) (point-max))))
+        (should (> num-packages 1))
+        (package-menu-filter-by-name "tetris")
+        (should (= (count-lines (point-min) (point-max)) 1))
+        (package-menu-clear-filter)
+        (should (= (count-lines (point-min) (point-max)) num-packages)))
+      (kill-buffer buf))))
+
 (ert-deftest package-test-update-archives ()
   "Test updating package archives."
   (with-package-test ()
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-30 21:26                   ` Federico Tedin
@ 2019-09-30 21:59                     ` Stefan Kangas
  2019-10-08 17:28                       ` Stefan Kangas
  0 siblings, 1 reply; 17+ messages in thread
From: Stefan Kangas @ 2019-09-30 21:59 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 36981

Federico Tedin <federicotedin@gmail.com> writes:

> I'm attaching the new patch.

Looks good to me.

Best regards,
Stefan Kangas





^ permalink raw reply	[flat|nested] 17+ messages in thread

* bug#36981: 26.2; request: add searching by package name to list-packages
  2019-09-30 21:59                     ` Stefan Kangas
@ 2019-10-08 17:28                       ` Stefan Kangas
  0 siblings, 0 replies; 17+ messages in thread
From: Stefan Kangas @ 2019-10-08 17:28 UTC (permalink / raw)
  To: Federico Tedin; +Cc: 36981

close 36981 27.1
thanks

Stefan Kangas <stefan@marxist.se> writes:

> Federico Tedin <federicotedin@gmail.com> writes:
>
> > I'm attaching the new patch.
>
> Looks good to me.

No one has had any further comments, so I've now pushed your patch to
master as commit f96b8fd27c.  Note that I made some minor copy-edits
to the documentation before pushing.

Thanks again for your contribution; I'm closing this bug.

Best regards,
Stefan Kangas





^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2019-10-08 17:28 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-09  7:21 bug#36981: 26.2; request: add searching by package name to list-packages ndame
2019-09-08  0:47 ` Federico Tedin
2019-09-08 15:20   ` Štěpán Němec
2019-09-09 20:54     ` Federico Tedin
2019-09-14 11:15       ` Federico Tedin
2019-09-18 23:17         ` Stefan Kangas
2019-09-16  0:29   ` Stefan Kangas
2019-09-18 16:38     ` Federico Tedin
2019-09-18 22:22       ` Stefan Kangas
2019-09-19 20:29         ` Juri Linkov
2019-09-20 18:49           ` Federico Tedin
2019-09-20 22:09             ` Stefan Kangas
2019-09-26 18:28               ` Federico Tedin
2019-09-27 10:32                 ` Stefan Kangas
2019-09-30 21:26                   ` Federico Tedin
2019-09-30 21:59                     ` Stefan Kangas
2019-10-08 17:28                       ` Stefan Kangas

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).