bug#66567: [PATCH] use-package: Add ignored-files support to :vc keyword

[Philip Kaludercic]

Tony Zorman <tonyzorman@mailbox.org> writes:

> On Wed, Nov 01 2023 09:09, Philip Kaludercic wrote:
>>> diff --git a/lisp/use-package/use-package-core.el 
>>> b/lisp/use-package/use-package-core.el
>>> index 34c45b7aec..5d0d554baf 100644
>>> --- a/lisp/use-package/use-package-core.el
>>> +++ b/lisp/use-package/use-package-core.el
>>> @@ -1654,7 +1654,8 @@ use-package-normalize--vc-arg
>>>                               (t (ensure-string v))))
>>>                   (:vc-backend (ensure-symbol v))
>>>                   (_ (ensure-string v)))))
>>> -    (pcase-let ((valid-kws '(:url :branch :lisp-dir :main-file :vc-backend 
>>> :rev))
>>> +    (pcase-let ((valid-kws '( :url :branch :lisp-dir :main-file 
>>> :vc-backend :rev
>>> +                              :shell-command :make))
>>
>> Why is use-package checking for valid keywords in the first place?
>
> Better error messages, mostly. Especially people switching from
> quelpa/straight/vc-use-package might be surprised that :vc is not a
> drop-in replacement for those packages. I feel like alerting them to
> this fact sooner rather than later makes for a better experience.

IIUC this would raise an error when an unknown keyword is encountered,
right?

>>> * lisp/use-package/use-package-core.el (use-package-split-when):
>>> New utility function to split a list whenever a specified predicate
>>> returns t.
>>> (use-package-vc-valid-keywords): A new defconst to gather all allowed
>>> keywords.
>>> (use-package-normalize--vc-arg): Properly normalize the :ignored-files
>>> keyword, in that the following are all valid ways of entering files:
>>>   :ignored-files "a"
>>>   :ignored-files ("a")
>>>   :ignored-files "a" "b" "c"
>>>   :ignored-files ("a" "b" "c")
>>> (use-package-normalize/:vc): Adjust normalization, now that we do not
>>> necessarily receive a valid plist as an input.
>>
>> I would much prefer that package specifications have a canonical form
>> and that use-package doesn't try to introduce variations that wouldn't
>> be compatible with package-vc-install proper and elpa-admin.  Or is this
>> necessary for use-package?
>
> It's not *necessary*, but it's quite common for use-package keywords to
> do their best in order to be as unobtrusive as possible. This includes
> omitting parentheses that might not be strictly needed, or to cleverly
> transform the input in some other way (e.g., :hook makes great use of
> this).

My experience is that this is more likely to cause confusion, e.g. when
people write :config (progn ...).  But as I said, since I don't use
use-package, I don't want to give the final verdict, I am just
expressing my preferences.

>>> I will cheekily bump this, and also Cc. Philip as the most likely
>>> reviewer.
>>
>> I don't use use-package nor am I familiar with the code base, so I
>> wouldn't value my input that much.
>
> Oh, fair enough. In either case, I couldn't think of anyone else—sorry
> for the noise :)

I think that Stefan Kangas would probably be the best to ask, since he
was the one responsible for merging use-package into the core.