From a28a6c4ad4c7b9430fcbb29017a328190939c456 Mon Sep 17 00:00:00 2001 From: Jonas Bernoulli Date: Fri, 27 May 2022 20:33:04 +0200 Subject: [PATCH] Fix and improve conflict removal when adding new suffixes Fixes #203. --- docs/transient.org | 15 +++++++++++---- docs/transient.texi | 18 +++++++++++++----- lisp/transient.el | 45 +++++++++++++++++++++------------------------ 3 files changed, 45 insertions(+), 33 deletions(-) diff --git a/docs/transient.org b/docs/transient.org index 9217d7d2..7d111bc4 100644 --- a/docs/transient.org +++ b/docs/transient.org @@ -774,13 +774,20 @@ These functions operate on the information stored in the that tree are not objects but have the form ~(LEVEL CLASS PLIST)~, where plist should set at least ~:key~, ~:description~ and ~:command~. -- Function: transient-insert-suffix prefix loc suffix :: +- Function: transient-insert-suffix prefix loc suffix &optional keep-other :: +- Function: transient-append-suffix prefix loc suffix &optional keep-other :: - This function inserts suffix or group SUFFIX into PREFIX before LOC. + These functions insert the suffix or group SUFFIX into PREFIX before + or after LOC. -- Function: transient-append-suffix prefix loc suffix :: + Conceptually adding a binding to a transient prefix is similar to + adding a binding to a keymap, but this is complicated by the fact + that multiple suffix commands can be bound to the same key, provided + they are never active at the same time, see [[*Predicate Slots]]. - This function inserts suffix or group SUFFIX into PREFIX after LOC. + Unfortunately both false-positives and false-negatives are possible. + To deal with the former use non-nil KEEP-OTHER. To deal with the + latter remove the conflicting binding explicitly. - Function: transient-replace-suffix prefix loc suffix :: diff --git a/docs/transient.texi b/docs/transient.texi index 7ff4fa66..6ee2d625 100644 --- a/docs/transient.texi +++ b/docs/transient.texi @@ -940,12 +940,20 @@ These functions operate on the information stored in the that tree are not objects but have the form @code{(LEVEL CLASS PLIST)}, where plist should set at least @code{:key}, @code{:description} and @code{:command}. -@defun transient-insert-suffix prefix loc suffix -This function inserts suffix or group SUFFIX into PREFIX before LOC@. +@defun transient-insert-suffix prefix loc suffix &optional keep-other @end defun - -@defun transient-append-suffix prefix loc suffix -This function inserts suffix or group SUFFIX into PREFIX after LOC@. +@defun transient-append-suffix prefix loc suffix &optional keep-other +These functions insert the suffix or group SUFFIX into PREFIX before +or after LOC@. + +Conceptually adding a binding to a transient prefix is similar to +adding a binding to a keymap, but this is complicated by the fact +that multiple suffix commands can be bound to the same key, provided +they are never active at the same time, see @ref{Predicate Slots}. + +Unfortunately both false-positives and false-negatives are possible. +To deal with the former use non-nil KEEP-OTHER@. To deal with the +latter remove the conflicting binding explicitly. @end defun @defun transient-replace-suffix prefix loc suffix diff --git a/lisp/transient.el b/lisp/transient.el index acb38ee1..b9603e4a 100644 --- a/lisp/transient.el +++ b/lisp/transient.el @@ -1141,7 +1141,7 @@ example, sets a variable use `transient-define-infix' instead. ;;; Edit -(defun transient--insert-suffix (prefix loc suffix action) +(defun transient--insert-suffix (prefix loc suffix action &optional keep-other) (let* ((suf (cl-etypecase suffix (vector (transient--parse-group prefix suffix)) (list (transient--parse-suffix prefix suffix)) @@ -1159,25 +1159,18 @@ example, sets a variable use `transient-define-infix' instead. suffix prefix loc "suffixes and groups cannot be siblings")) (t - (when (and (listp suffix) - (listp elt)) - ;; Both suffixes are key bindings; not heading strings. - (let ((key (transient--spec-key suf))) - (if (equal (transient--kbd key) - (transient--kbd (transient--spec-key elt))) - ;; We must keep `mem' until after we have inserted - ;; behind it, which `transient-remove-suffix' does - ;; not allow us to do. - (let ((spred (transient--suffix-predicate suf)) - (epred (transient--suffix-predicate elt))) - ;; If both suffixes have a predicate and they - ;; are not identical, then there is a high - ;; probability that we want to keep both. - (when (or (not spred) - (not epred) - (equal spred epred)) - (setq action 'replace))) - (transient-remove-suffix prefix key)))) + (when-let* ((bindingp (listp suf)) + (key (transient--spec-key suf)) + (conflict (car (transient--layout-member key prefix))) + (conflictp + (and (not (and (eq action 'replace) + (eq conflict elt))) + (or (not keep-other) + (eq (plist-get (nth 2 suf) :command) + (plist-get (nth 2 conflict) :command))) + (equal (transient--suffix-predicate suf) + (transient--suffix-predicate conflict))))) + (transient-remove-suffix prefix key)) (cl-ecase action (insert (setcdr mem (cons elt (cdr mem))) (setcar mem suf)) @@ -1185,7 +1178,7 @@ example, sets a variable use `transient-define-infix' instead. (replace (setcar mem suf))))))) ;;;###autoload -(defun transient-insert-suffix (prefix loc suffix) +(defun transient-insert-suffix (prefix loc suffix &optional keep-other) "Insert a SUFFIX into PREFIX before LOC. PREFIX is a prefix command, a symbol. SUFFIX is a suffix command or a group specification (of @@ -1193,12 +1186,14 @@ SUFFIX is a suffix command or a group specification (of LOC is a command, a key vector, a key description (a string as returned by `key-description'), or a coordination list (whose last element may also be a command or key). +Remove a conflicting binding unless optional KEEP-OTHER is + non-nil. See info node `(transient)Modifying Existing Transients'." (declare (indent defun)) - (transient--insert-suffix prefix loc suffix 'insert)) + (transient--insert-suffix prefix loc suffix 'insert keep-other)) ;;;###autoload -(defun transient-append-suffix (prefix loc suffix) +(defun transient-append-suffix (prefix loc suffix &optional keep-other) "Insert a SUFFIX into PREFIX after LOC. PREFIX is a prefix command, a symbol. SUFFIX is a suffix command or a group specification (of @@ -1206,9 +1201,11 @@ SUFFIX is a suffix command or a group specification (of LOC is a command, a key vector, a key description (a string as returned by `key-description'), or a coordination list (whose last element may also be a command or key). +Remove a conflicting binding unless optional KEEP-OTHER is + non-nil. See info node `(transient)Modifying Existing Transients'." (declare (indent defun)) - (transient--insert-suffix prefix loc suffix 'append)) + (transient--insert-suffix prefix loc suffix 'append keep-other)) ;;;###autoload (defun transient-replace-suffix (prefix loc suffix)