From: Tony Zorman via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
To: Philip Kaludercic <philipk@posteo.net>
Cc: stefankangas@gmail.com, 66567@debbugs.gnu.org
Subject: bug#66567: [PATCH] use-package: Add ignored-files support to :vc keyword
Date: Wed, 01 Nov 2023 15:36:58 +0100 [thread overview]
Message-ID: <87r0l9fsl1.fsf@hyperspace> (raw)
In-Reply-To: <87r0l91vww.fsf@posteo.net>
[-- Attachment #1: Type: text/plain, Size: 2462 bytes --]
On Wed, Nov 01 2023 12:48, Philip Kaludercic wrote:
> 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?
Yes, a declaration like
(use-package foo
:vc (:url "url" :blargh "123"))
would result in the following message
⛔ Error (use-package): Failed to parse package foo: use-package: Keyword :vc received unknown argument: :blargh. Supported keywords are: (:url :branch :lisp-dir :main-file :vc-backend :rev :shell-command :make :ignored-files)
Things get a bit muddier if ':blargh' would be passed down to
package-vc-install.
Now that you mention it, I noticed a tiny mistake (resulting in a worse
error message!) in the second of the original patches. I've attached a
corrected version.
>>>> 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.
Thanks! I have Cc'd Stefan, hoping to not come across as too pushy :)
Tony
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-use-package-Add-ignored-files-support-to-vc-keyword.patch --]
[-- Type: text/x-patch, Size: 6970 bytes --]
From f8590d37b29a96a7984d8acae2d6c2b557e0dc59 Mon Sep 17 00:00:00 2001
From: Tony Zorman <soliditsallgood@mailbox.org>
Date: Sun, 15 Oct 2023 16:51:00 +0200
Subject: [PATCH] use-package: Add :ignored-files support to :vc keyword
* 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.
* test/lisp/use-package/use-package-tests.el (use-package-test-normalize/:vc):
Add tests for :ignored-files keyword.
---
lisp/use-package/use-package-core.el | 60 ++++++++++++++++------
test/lisp/use-package/use-package-tests.el | 10 +++-
2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/lisp/use-package/use-package-core.el b/lisp/use-package/use-package-core.el
index 5d0d554baf..974059a850 100644
--- a/lisp/use-package/use-package-core.el
+++ b/lisp/use-package/use-package-core.el
@@ -521,6 +521,24 @@ use-package-split-list-at-keys
(let ((xs (use-package-split-list (apply-partially #'eq key) lst)))
(cons (car xs) (use-package-split-list-at-keys key (cddr xs))))))
+(defun use-package-split-when (pred xs)
+ "Repeatedly split a list according to PRED.
+Split XS every time PRED returns t. Keep the delimiters, and
+arrange the result in an alist. For example:
+
+ (use-package-split-when #\\='keywordp \\='(:a 1 :b 2 3 4 :c 5))
+ ;; => \\='((:a 1) (:b 2 3 4) (:c 5))
+
+ (use-package-split-when (lambda (x) (> x 2)) \\='(10 1 3 2 4 -1 8 9))
+ ;; => \\='((10 1) (3 2) (4 -1) (8) (9))"
+ (unless (seq-empty-p xs)
+ (pcase-let* ((`(,first . ,rest) (if (funcall pred (car xs))
+ (cons (car xs) (cdr xs))
+ (use-package-split-list pred xs)))
+ (`(,val . ,recur) (use-package-split-list pred rest)))
+ (cons (cons first val)
+ (use-package-split-when pred recur)))))
+
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;;
;;; Keywords
@@ -1634,6 +1652,12 @@ use-package-handler/:vc
(push `(use-package-vc-install ',arg ,local-path) body)) ; runtime
body))
+(defconst use-package-vc-valid-keywords
+ '( :url :branch :lisp-dir :main-file :vc-backend :rev
+ :shell-command :make :ignored-files)
+ "Valid keywords for the `:vc' keyword, see the Info
+node `(emacs)Fetching Package Sources'.")
+
(defun use-package-normalize--vc-arg (arg)
"Normalize possible arguments to the `:vc' keyword.
ARG is a cons-cell of approximately the form that
@@ -1653,24 +1677,26 @@ use-package-normalize--vc-arg
((eq v :newest) nil)
(t (ensure-string v))))
(:vc-backend (ensure-symbol v))
+ (:ignored-files (if (listp v) v (list v)))
(_ (ensure-string v)))))
- (pcase-let ((valid-kws '( :url :branch :lisp-dir :main-file :vc-backend :rev
- :shell-command :make))
- (`(,name . ,opts) arg))
+ (pcase-let* ((`(,name . ,opts) arg))
(if (stringp opts) ; (NAME . VERSION-STRING) ?
(list name opts)
- ;; Error handling
- (cl-loop for (k _) on opts by #'cddr
- if (not (member k valid-kws))
- do (use-package-error
- (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
- k valid-kws)))
- ;; Actual normalization
- (list name
- (cl-loop for (k v) on opts by #'cddr
- if (not (eq k :rev))
- nconc (list k (normalize k v)))
- (normalize :rev (plist-get opts :rev)))))))
+ (let ((opts (use-package-split-when
+ (lambda (el) (and (keywordp el) (not (equal :newest el))))
+ opts)))
+ ;; Error handling
+ (cl-loop for (k . _) in opts
+ if (not (member k use-package-vc-valid-keywords))
+ do (use-package-error
+ (format "Keyword :vc received unknown argument: %s. Supported keywords are: %s"
+ k use-package-vc-valid-keywords)))
+ ;; Actual normalization
+ (list name
+ (cl-loop for (k . v) in opts
+ if (not (eq k :rev))
+ nconc (list k (normalize k (if (length= v 1) (car v) v))))
+ (normalize :rev (car (alist-get :rev opts)))))))))
(defun use-package-normalize/:vc (name _keyword args)
"Normalize possible arguments to the `:vc' keyword.
@@ -1686,9 +1712,9 @@ use-package-normalize/:vc
((or 'nil 't) (list name)) ; guess name
((pred symbolp) (list arg)) ; use this name
((pred stringp) (list name arg)) ; version string + guess name
- ((pred plistp) ; plist + guess name
+ (`(,(pred keywordp) . ,(pred listp)) ; list + guess name
(use-package-normalize--vc-arg (cons name arg)))
- (`(,(pred symbolp) . ,(or (pred plistp) ; plist/version string + name
+ (`(,(pred symbolp) . ,(or (pred listp) ; list/version string + name
(pred stringp)))
(use-package-normalize--vc-arg arg))
(_ (use-package-error "Unrecognised argument to :vc.\
diff --git a/test/lisp/use-package/use-package-tests.el b/test/lisp/use-package/use-package-tests.el
index 9181a8171a..5636ba8a4f 100644
--- a/test/lisp/use-package/use-package-tests.el
+++ b/test/lisp/use-package/use-package-tests.el
@@ -2014,7 +2014,15 @@ use-package-test-normalize/:vc
(should (equal '(foo)
(use-package-normalize/:vc 'foo :vc nil)))
(should (equal '(bar)
- (use-package-normalize/:vc 'foo :vc '(bar)))))
+ (use-package-normalize/:vc 'foo :vc '(bar))))
+ (should (equal
+ '(foo (:ignored-files ("a" "b" "c")) :last-release)
+ (use-package-normalize/:vc 'foo :vc '((:ignored-files "a" "b" "c")))))
+ (should (equal
+ (use-package-normalize/:vc 'foo :vc '((:ignored-files "a")))
+ (use-package-normalize/:vc 'foo :vc '((:ignored-files ("a"))))))
+ (should (equal (use-package-normalize/:vc 'foo :vc '((:ignored-files "a" "b" "c")))
+ (use-package-normalize/:vc 'foo :vc '((:ignored-files ("a" "b" "c")))))))
;; Local Variables:
;; no-byte-compile: t
--
2.42.0
[-- Attachment #3: Type: text/plain, Size: 44 bytes --]
--
Tony Zorman | https://tony-zorman.com/
next prev parent reply other threads:[~2023-11-01 14:36 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-15 16:42 bug#66567: [PATCH] use-package: Add ignored-files support to :vc keyword Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
[not found] ` <handler.66567.B.169739278328671.ack@debbugs.gnu.org>
2023-11-01 7:43 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-01 9:09 ` Philip Kaludercic
2023-11-01 10:13 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-01 12:48 ` Philip Kaludercic
2023-11-01 14:36 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors [this message]
2023-11-01 16:38 ` Philip Kaludercic
2023-11-07 19:39 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-07 21:24 ` Philip Kaludercic
2023-11-10 12:00 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2023-11-16 7:32 ` Philip Kaludercic
2024-05-02 18:57 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-03 6:35 ` Eli Zaretskii
2024-05-04 6:16 ` John Wiegley
2024-05-14 16:08 ` Tony Zorman via Bug reports for GNU Emacs, the Swiss army knife of text editors
2024-05-15 1:19 ` John Wiegley
2024-05-18 10:32 ` Eli Zaretskii
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87r0l9fsl1.fsf@hyperspace \
--to=bug-gnu-emacs@gnu.org \
--cc=66567@debbugs.gnu.org \
--cc=philipk@posteo.net \
--cc=stefankangas@gmail.com \
--cc=tonyzorman@mailbox.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.