GNU bug report logs - #36981
26.2; request: add searching by package name to list-packages

Previous Next

Package: emacs;

Reported by: ndame <emacsuser <at> freemail.hu>

Date: Fri, 9 Aug 2019 07:22:02 UTC

Severity: wishlist

Tags: patch

Found in version 26.2

Fixed in version 27.1

Done: Stefan Kangas <stefan <at> marxist.se>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 36981 in the body.
You can then email your comments to 36981 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Fri, 09 Aug 2019 07:22:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to ndame <emacsuser <at> freemail.hu>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Fri, 09 Aug 2019 07:22:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: ndame <emacsuser <at> freemail.hu>
To: "bug-gnu-emacs <at> gnu.org" <bug-gnu-emacs <at> gnu.org>
Subject: 26.2; request: add searching by package name to list-packages
Date: Fri,  9 Aug 2019 09:21:02 +0200 (CEST)
[Message part 1 (text/plain, inline)]
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.
 
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Sun, 08 Sep 2019 00:49:01 GMT) Full text and rfc822 format available.

Message #8 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: ndame <emacsuser <at> freemail.hu>
Cc: 36981 <at> debbugs.gnu.org
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Sun, 08 Sep 2019 02:47:57 +0200
[Message part 1 (text/plain, inline)]
ndame <emacsuser <at> 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

[search.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Sun, 08 Sep 2019 15:21:02 GMT) Full text and rfc822 format available.

Message #11 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Štěpán Němec <stepnem <at> gmail.com>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>
Subject: Re: bug#36981: 26.2; request: add searching by package name to
 list-packages
Date: Sun, 08 Sep 2019 17:20:00 +0200
I don't use package.el, but here are a few observations:

> From: Federico Tedin <federicotedin <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Mon, 09 Sep 2019 20:56:01 GMT) Full text and rfc822 format available.

Message #14 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Štěpán Němec <stepnem <at> gmail.com>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Mon, 09 Sep 2019 22:54:56 +0200
[Message part 1 (text/plain, inline)]
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

[search.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Sat, 14 Sep 2019 11:16:02 GMT) Full text and rfc822 format available.

Message #17 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Štěpán Němec <stepnem <at> gmail.com>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Sat, 14 Sep 2019 13:15:12 +0200
[Message part 1 (text/plain, inline)]
Forgot to add '+++' above my NEWS entry, sending a new patch.

- Federico

[search.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Mon, 16 Sep 2019 00:31:01 GMT) Full text and rfc822 format available.

Message #20 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Mon, 16 Sep 2019 02:29:49 +0200
Federico Tedin <federicotedin <at> gmail.com> writes:

> ndame <emacsuser <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Wed, 18 Sep 2019 16:39:01 GMT) Full text and rfc822 format available.

Message #23 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Wed, 18 Sep 2019 18:38:39 +0200
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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Wed, 18 Sep 2019 22:23:01 GMT) Full text and rfc822 format available.

Message #26 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Thu, 19 Sep 2019 00:22:30 +0200
Federico Tedin <federicotedin <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Wed, 18 Sep 2019 23:19:02 GMT) Full text and rfc822 format available.

Message #29 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>,
 Štěpán Němec <stepnem <at> gmail.com>
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Thu, 19 Sep 2019 01:17:43 +0200
[Message part 1 (text/plain, inline)]
Hi Federico,

Here's a more detailed review of your patch.

Federico Tedin <federicotedin <at> 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
[Message part 2 (text/html, inline)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Thu, 19 Sep 2019 20:50:03 GMT) Full text and rfc822 format available.

Message #32 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Juri Linkov <juri <at> linkov.net>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>,
 Federico Tedin <federicotedin <at> gmail.com>
Subject: Re: bug#36981: 26.2; request: add searching by package name to
 list-packages
Date: Thu, 19 Sep 2019 23:29:30 +0300
> 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.)




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Fri, 20 Sep 2019 18:50:01 GMT) Full text and rfc822 format available.

Message #35 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Juri Linkov <juri <at> linkov.net>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>,
 Stefan Kangas <stefan <at> marxist.se>
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Fri, 20 Sep 2019 20:49:45 +0200
>> 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!




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Fri, 20 Sep 2019 22:10:01 GMT) Full text and rfc822 format available.

Message #38 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Sat, 21 Sep 2019 00:09:34 +0200
Federico Tedin <federicotedin <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Thu, 26 Sep 2019 18:29:02 GMT) Full text and rfc822 format available.

Message #41 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Thu, 26 Sep 2019 20:28:18 +0200
[Message part 1 (text/plain, inline)]
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

[search.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Fri, 27 Sep 2019 10:33:02 GMT) Full text and rfc822 format available.

Message #44 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: 36981 <at> debbugs.gnu.org, ndame <emacsuser <at> freemail.hu>,
 Juri Linkov <juri <at> linkov.net>
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Fri, 27 Sep 2019 12:32:20 +0200
Federico Tedin <federicotedin <at> 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




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Mon, 30 Sep 2019 21:27:01 GMT) Full text and rfc822 format available.

Message #47 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Federico Tedin <federicotedin <at> gmail.com>
To: Stefan Kangas <stefan <at> marxist.se>
Cc: 36981 <at> debbugs.gnu.org
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Mon, 30 Sep 2019 23:26:49 +0200
[Message part 1 (text/plain, inline)]
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!

[search.patch (text/x-diff, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Mon, 30 Sep 2019 22:00:02 GMT) Full text and rfc822 format available.

Message #50 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: 36981 <at> debbugs.gnu.org
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Mon, 30 Sep 2019 23:59:29 +0200
Federico Tedin <federicotedin <at> gmail.com> writes:

> I'm attaching the new patch.

Looks good to me.

Best regards,
Stefan Kangas




Added tag(s) patch. Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Mon, 30 Sep 2019 22:01:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#36981; Package emacs. (Tue, 08 Oct 2019 17:29:01 GMT) Full text and rfc822 format available.

Message #55 received at 36981 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Kangas <stefan <at> marxist.se>
To: Federico Tedin <federicotedin <at> gmail.com>
Cc: 36981 <at> debbugs.gnu.org
Subject: Re: bug#36981: 26.2;
 request: add searching by package name to list-packages
Date: Tue, 8 Oct 2019 19:28:37 +0200
close 36981 27.1
thanks

Stefan Kangas <stefan <at> marxist.se> writes:

> Federico Tedin <federicotedin <at> 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




bug marked as fixed in version 27.1, send any further explanations to 36981 <at> debbugs.gnu.org and ndame <emacsuser <at> freemail.hu> Request was from Stefan Kangas <stefan <at> marxist.se> to control <at> debbugs.gnu.org. (Tue, 08 Oct 2019 17:29:02 GMT) Full text and rfc822 format available.

bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 06 Nov 2019 12:24:14 GMT) Full text and rfc822 format available.

This bug report was last modified 4 years and 166 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.