[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#38424: [PATCH] Add new filter functions to Package Menu
From: |
Stefan Kangas |
Subject: |
bug#38424: [PATCH] Add new filter functions to Package Menu |
Date: |
Sun, 01 Dec 2019 12:12:58 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux) |
Eli Zaretskii <eliz@gnu.org> writes:
>> From: Stefan Kangas <stefan@marxist.se>
>> Date: Fri, 29 Nov 2019 13:31:10 +0100
>>
>> The attached patches adds new commands to filter the "*Packages*"
>> buffer by version, status and archive. (The first patch only adds new
>> version list comparison predicates, something I needed to simplify the
>> second patch.)
>
> We deliberately didn't define the functions you are now adding, since
> they are just one 'not' away. Do they really simplify the callers so
> much that we now want to add them?
I don't know, but I'm also not sure I understand the benefit of not
adding them.
In this case, it let me do this:
+ (let ((fun (cond ((eq predicate '=) 'version-list-=)
+ ((eq predicate '<) 'version-list-<)
+ ((eq predicate '>) 'version-list->)
I could of course use a lambda here instead, but it makes the code a
bit uglier. Let me know if that's preferable.
>> * doc/emacs/package.texi (Package Menu): Document it.
>
> This tells nothing about the changes which aren't "documenting it".
> (And, btw, what is "it" here is not clear at all.)
Thanks. I thought that was how we usually wrote.
Is "Document the new commands." better?
>> - (when (or (eq packages t) (memq name packages))
>> + (when (or (not packages) (memq name packages))
>> (dolist (pkg (cdr elt))
>> (when (package--has-keyword-p pkg keywords)
>> (push pkg info-list))))))
>> @@ -2950,7 +2958,7 @@ package-menu--refresh
>> (when (and (package--has-keyword-p pkg keywords)
>> (or package-list-unversioned
>> (package--bi-desc-version (cdr elt)))
>> - (or (eq packages t) (memq name packages)))
>> + (or (not packages) (memq name packages)))
>> (push pkg info-list)))))
>>
>> ;; Available and disabled packages:
>> @@ -2959,7 +2967,7 @@ package-menu--refresh
>> (dolist (elt package-archive-contents)
>> (let ((name (car elt)))
>> ;; To be displayed it must be in PACKAGES;
>> - (when (and (or (eq packages t) (memq name packages))
>> + (when (and (or (not packages) (memq name packages))
>
> Does the above mean you are suggesting a backward-incompatible API
> change?
AFAIU, this does not change the API since this is an internal
function.
>> +Arguments PACKAGES and KEYWORDS are like `package-menu--refresh'."
>
> Arguments cannot be "like" a function. Suggest to say "like in
> `package-menu--refresh'" instead.
Right, I'll fix that.
> I don't use package.el, so I'd like someone who does or knows the code
> well to review the patch.
I'll wait for that review. Thank you for taking a look.
Best regards,
Stefan Kangas
- bug#38424: [PATCH] Add new filter functions to Package Menu,
Stefan Kangas <=