emacs-elpa-diffs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[elpa] externals/transient 522b625cf3 7/7: Use a dedicated symbol to hid


From: Jonas Bernoulli
Subject: [elpa] externals/transient 522b625cf3 7/7: Use a dedicated symbol to hide from read-extended-command
Date: Tue, 23 Jan 2024 09:23:46 -0500 (EST)

branch: externals/transient
commit 522b625cf39cb1a9322719dcb039f25bc49ab35b
Author: Jonas Bernoulli <jonas@bernoul.li>
Commit: Jonas Bernoulli <jonas@bernoul.li>

    Use a dedicated symbol to hide from read-extended-command
    
    By using a dedicated symbol, which uses our package prefix, we ensure
    that we can set `read-extended-command-predicate', without affecting
    any unrelated package.
    
    `transient-command-completion-not-suffix-only-p' only prevents
    "anonymously defined" transient suffix commands from being offered as
    `read-extended-command' completion candidates.  Furthermore, because
    we only set `read-extended-command-predicate' iff it is non-nil, this
    does not mess with any user customization of this option.  Also note
    that if the user uses `command-completion-default-include-p' instead,
    then the suffix-only commands are also filtered out.
    
    IMO it is reasonable for a package to use this feature to declare
    that certain commands should absolutely never be offered as completion
    candidates.  The interface of this feature is not optimized for this
    use-case, but I predict that `read-extended-command-predicate' will
    one day be a hook, at which point I would not have to write this
    explanation in the hope that it convincingly makes the case for why
    daring to us it to achieve the goal that it was designed to achieve
    is not Wrong™, not even if it is a library that does it, as opposed
    to a user.
    
    It has been suggested that Transient should instead avoid `intern'ing
    any symbols for these (admittedly not fully) anonymous suffix
    commands.  The proposed patch does not actually work, because these
    symbols *have to be* interned, and for backward compatibility reasons
    `transient--init-suffix' ensures that they are.  So initially (at
    load-time) these suffix command symbols would not be interned but as
    the user invokes transient commands, more and more of them would
    become interned.
    
    We could avoid interning anonymously defined suffix commands
    altogether, but that would have unfortunate consequences.  In fact
    Transient used to do that because `read-extended-command-predicate'
    did not exist yet.  But I sure wished it did, and I was considering
    implementing something like that myself.  So of course once that
    feature appear, I was happy to take advantage, in [1: 226db67b].
    
    Going back to not binding every suffix command to an interned symbol,
    would complicate matters considerably.  It would lead to bugs, and
    would slow down development, because I would constantly have to double
    check changes, to reduce the risk of introducing such bugs.  The code
    would be more difficult to understand.  Frankly, doing so would mean
    intentionally adding accidental complexity.  I very much want to avoid
    that oxymoron.
    
    Furthermore, not interning anonymously defined suffixes, would do
    nothing to hide infix arguments that are explicitly defined using
    `transient-define-infix', but those should also not be offered as
    completion candidates.  The approach I have chosen does.
    
    Instead of #273.
    
    1: 2023-08-12 226db67b3680acbeb74cb0403e1a302917054174
       All suffix commands now must be accessed through fbound symbols
---
 lisp/transient.el | 26 ++++++++++++++++++--------
 1 file changed, 18 insertions(+), 8 deletions(-)

diff --git a/lisp/transient.el b/lisp/transient.el
index c84ac8f65d..7965cc1467 100644
--- a/lisp/transient.el
+++ b/lisp/transient.el
@@ -1007,7 +1007,7 @@ keyword.
     `(progn
        (defalias ',name #'transient--default-infix-command)
        (put ',name 'interactive-only t)
-       (put ',name 'completion-predicate #'ignore)
+       (put ',name 'completion-predicate #'transient--suffix-only)
        (put ',name 'function-documentation ,docstr)
        (put ',name 'transient--suffix
             (,(or class 'transient-switch) :command ',name ,@slots)))))
@@ -1037,7 +1037,8 @@ this case, because the `man-page' slot was not set in 
this case."
     (transient-infix-set obj (transient-infix-read obj)))
   (transient--show))
 (put 'transient--default-infix-command 'interactive-only t)
-(put 'transient--default-infix-command 'command-predicate #'ignore)
+(put 'transient--default-infix-command 'command-predicate
+     #'transient--suffix-only)
 
 (eval-and-compile
   (defun transient--expand-define-args (args &optional arglist)
@@ -1148,7 +1149,7 @@ this case, because the `man-page' slot was not set in 
this case."
                       args :command
                       `(prog1 ',sym
                          (put ',sym 'interactive-only t)
-                         (put ',sym 'command-predicate #'ignore)
+                         (put ',sym 'command-predicate 
#'transient--suffix-only)
                          (defalias ',sym
                            ,(if (eq (car-safe cmd) 'lambda)
                                 cmd
@@ -1171,7 +1172,7 @@ this case, because the `man-page' slot was not set in 
this case."
                       args :command
                       `(prog1 ',sym
                          (put ',sym 'interactive-only t)
-                         (put ',sym 'command-predicate #'ignore)
+                         (put ',sym 'command-predicate 
#'transient--suffix-only)
                          (defalias ',sym #'transient--default-infix-command))))
           (cond ((and car (not (keywordp car)))
                  (setq class 'transient-option)
@@ -1209,16 +1210,25 @@ this case, because the `man-page' slot was not set in 
this case."
     (and (string-match "\\`\\(-[a-zA-Z]\\)\\(\\'\\|=\\)" arg)
          (match-string 1 arg))))
 
-(defun transient-command-completion-not-ignored-p (symbol _buffer)
+(defun transient-command-completion-not-suffix-only-p (symbol _buffer)
   "Say whether SYMBOL should be offered as a completion.
 If the value of SYMBOL's `completion-predicate' property is
-`ignore', then return nil, otherwise return t."
-  (not (eq (get symbol 'completion-predicate) 'ignore)))
+`transient--suffix-only', then return nil, otherwise return t.
+This is the case when a command should only ever be used as a
+suffix of a transient prefix command (as opposed to bindings
+in regular keymaps or by using `execute-extended-command')."
+  (not (eq (get symbol 'completion-predicate) 'transient--suffix-only)))
+
+(defalias 'transient--suffix-only #'ignore
+  "Ignore ARGUMENTS, do nothing, and return nil.
+Also see `transient-command-completion-not-suffix-only-p'.
+Only use this alias as the value of the `command-predicate'
+symbol property.")
 
 (static-if (and (boundp 'read-extended-command-predicate) ; since Emacs 28.1
                 (not read-extended-command-predicate))
     (setq read-extended-command-predicate
-          'transient-command-completion-not-ignored-p))
+          'transient-command-completion-not-suffix-only-p))
 
 (defun transient-parse-suffix (prefix suffix)
   "Parse SUFFIX, to be added to PREFIX.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]