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

From: Stefan Kangas <[email-masked]>
Date: Fri, 27 Sep 2019 12:32:20 +0200

Federico Tedin <[email-masked]> 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