From 522b625cf39cb1a9322719dcb039f25bc49ab35b Mon Sep 17 00:00:00 2001 From: Jonas Bernoulli Date: Sun, 21 Jan 2024 21:00:03 +0100 Subject: [PATCH] Use a dedicated symbol to hide from read-extended-command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 c84ac8f6..7965cc14 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.