branch: elpa/git-commit
commit ad68015aa1f7cab19eae0d86983193fa2b7038ca
Author: Jonas Bernoulli <jo...@bernoul.li>
Commit: Jonas Bernoulli <jo...@bernoul.li>
magit-confirm: Directly support two rounds of prompt formatting
The PROMPT and PROMPT-N arguments are used as f-strings passed to
`format'. Many callers of this function themselves use `format'
themselves to prepare these arguments. The "%s" in the original
f-string, which isn't to be replaced until the second round, has
to be escaped as "%%s".
Many of the first round `format' calls are used to embed branch
names and other refs into the prompt, and here a problem arises:
"%" is allowed in Git refs, so we may end up with unescaped "%d"
in the final f-string.
To avoid having to escape "%s" in many callers, add support for
handling the first round of prompt formatting inside `magit-confirm'.
PROMPT and PROMPT-N can now be lists of the form (F-STRING . ARGS),
in which case they applied to `format', after escaping every "%s" in
every argument that is a string.
Closes #5178.
---
lisp/magit-apply.el | 13 +++++++------
lisp/magit-base.el | 10 ++++++++++
lisp/magit-branch.el | 16 ++++++++--------
lisp/magit-push.el | 12 ++++--------
lisp/magit-remote.el | 4 ++--
lisp/magit-stash.el | 2 +-
6 files changed, 32 insertions(+), 25 deletions(-)
diff --git a/lisp/magit-apply.el b/lisp/magit-apply.el
index 2c7a79601f..7ad55e1401 100644
--- a/lisp/magit-apply.el
+++ b/lisp/magit-apply.el
@@ -548,9 +548,10 @@ of a side, then keep that side without prompting."
(funcall apply section "--reverse" "--index"))))
(defun magit-discard-hunks (sections)
- (magit-confirm 'discard (format "Discard %d hunks from %s"
- (length sections)
- (magit-section-parent-value (car sections))))
+ (magit-confirm 'discard
+ (list "Discard %d hunks from %s"
+ (length sections)
+ (magit-section-parent-value (car sections))))
(magit-discard-apply-n sections #'magit-apply-hunks))
(defun magit-discard-apply-n (sections apply)
@@ -738,9 +739,9 @@ so causes the change to be applied to the index as well."
(defun magit-reverse-hunks (sections args)
(magit-confirm 'reverse
- (format "Reverse %d hunks from %s"
- (length sections)
- (magit-section-parent-value (car sections))))
+ (list "Reverse %d hunks from %s"
+ (length sections)
+ (magit-section-parent-value (car sections))))
(magit-reverse-apply sections #'magit-apply-hunks args))
(defun magit-reverse-file (section args)
diff --git a/lisp/magit-base.el b/lisp/magit-base.el
index a1c00f27d5..af809b286d 100644
--- a/lisp/magit-base.el
+++ b/lisp/magit-base.el
@@ -794,6 +794,16 @@ ACTION is a member of option `magit-slow-confirm'."
(cl-defun magit-confirm ( action &optional prompt prompt-n noabort
(items nil sitems) prompt-suffix)
(declare (indent defun))
+ (when (and prompt (listp prompt))
+ (setq prompt
+ (apply #'format (car prompt)
+ (mapcar (lambda (a) (if (stringp a) (string-replace "%" "%%"
a) a))
+ (cdr prompt)))))
+ (when (and prompt-n (listp prompt-n))
+ (setq prompt-n
+ (apply #'format (car prompt-n)
+ (mapcar (lambda (a) (if (stringp a) (string-replace "%" "%%"
a) a))
+ (cdr prompt-n)))))
(setq prompt-n (format (concat (or prompt-n prompt) "? ") (length items)))
(setq prompt (format (concat (or prompt (magit-confirm-make-prompt action))
"? ")
diff --git a/lisp/magit-branch.el b/lisp/magit-branch.el
index e313da4f78..d696265935 100644
--- a/lisp/magit-branch.el
+++ b/lisp/magit-branch.el
@@ -633,12 +633,12 @@ prompt is confusing."
(offset (1+ (length remote))))
(cond
((magit-confirm 'delete-branch-on-remote
- (format "Deleting local %s. Also delete on %s"
- (magit-ref-fullname (car branches))
- remote)
- (format "Deleting %d local refs. Also delete on %s"
- (length refs)
- remote)
+ (list "Deleting local %s. Also delete on %s"
+ (magit-ref-fullname (car branches))
+ remote)
+ (list "Deleting %d local refs. Also delete on %s"
+ (length refs)
+ remote)
'noabort refs)
;; The ref may actually point at another rev on the remote,
;; but this is better than nothing.
@@ -724,8 +724,8 @@ prompt is confusing."
(when (member refspec refspecs)
(if (and (length= refspecs 1)
(magit-confirm 'delete-pr-remote
- (format "Also delete remote %s (%s)" remote
- "no pull-request branch remains")
+ (list "Also delete remote %s (%s)" remote
+ "no pull-request branch remains")
nil t))
(magit-call-git "remote" "rm" remote)
(magit-call-git "config" "--unset-all" variable
diff --git a/lisp/magit-push.el b/lisp/magit-push.el
index ddeeec9490..8e89d7362c 100644
--- a/lisp/magit-push.el
+++ b/lisp/magit-push.el
@@ -87,10 +87,8 @@ argument the push-remote can be changed before pushed to it."
(magit--select-push-remote "push there")))
(when changed
(magit-confirm 'set-and-push
- (string-replace
- "%" "%%"
- (format "Really use \"%s\" as push-remote and push \"%s\" there"
- remote branch))))
+ (list "Really use \"%s\" as push-remote and push \"%s\" there"
+ remote branch)))
(run-hooks 'magit-credential-hook)
(magit-run-git-async "push" "-v" args remote
(format "refs/heads/%s:refs/heads/%s"
@@ -150,10 +148,8 @@ the upstream."
;; is what the user wants to happen.
(setq merge (concat "refs/heads/" merge)))
(magit-confirm 'set-and-push
- (string-replace
- "%" "%%"
- (format "Really use \"%s\" as upstream and push \"%s\" there"
- upstream branch))))
+ (list "Really use \"%s\" as upstream and push \"%s\" there"
+ upstream branch)))
(cl-pushnew "--set-upstream" args :test #'equal))
(run-hooks 'magit-credential-hook)
(magit-run-git-async "push" "-v" args remote (concat branch ":" merge))))
diff --git a/lisp/magit-remote.el b/lisp/magit-remote.el
index 3cc22b88e3..2cf913cffd 100644
--- a/lisp/magit-remote.el
+++ b/lisp/magit-remote.el
@@ -206,8 +206,8 @@ the now stale refspecs. Other stale branches are not
removed."
(if (if (length= stale 1)
(pcase-let ((`(,refspec . ,refs) (car stale)))
(magit-confirm 'prune-stale-refspecs
- (format "Prune stale refspec %s and branch %%s" refspec)
- (format "Prune stale refspec %s and %%d branches" refspec)
+ (list "Prune stale refspec %s and branch %%s" refspec)
+ (list "Prune stale refspec %s and %%d branches" refspec)
nil refs))
(magit-confirm 'prune-stale-refspecs nil
(format "Prune %%d stale refspecs and %d branches"
diff --git a/lisp/magit-stash.el b/lisp/magit-stash.el
index 37b60cf2f6..4ab2e20872 100644
--- a/lisp/magit-stash.el
+++ b/lisp/magit-stash.el
@@ -348,7 +348,7 @@ When the region is active offer to drop all contained
stashes."
(defun magit-stash-clear (ref)
"Remove all stashes saved in REF's reflog by deleting REF."
(interactive (let ((ref (or (magit-section-value-if 'stashes) "refs/stash")))
- (magit-confirm t (format "Drop all stashes in %s" ref))
+ (magit-confirm t (list "Drop all stashes in %s" ref))
(list ref)))
(magit-run-git "update-ref" "-d" ref))
