[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [elpa] externals/transient aea527ed93 25/27: transient--wrap-command
From: |
Jonas Bernoulli |
Subject: |
Re: [elpa] externals/transient aea527ed93 25/27: transient--wrap-command: Use add-function and remove-function |
Date: |
Fri, 28 Feb 2025 17:46:05 +0100 |
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> - (when (symbolp this-command)
>> - (advice-add cmd :around advice '((depth . -99)))))))
>> + (add-function :around (if (symbolp cmd) (symbol-function cmd) cmd)
>> + advice '((depth . -99))))))
>
> I don't think this is quite right.
>
> When `symbolp` returns t, this ends up being a "poor man" version of
> `advice-add` which is probably good enough (tho it could fail if
> (symbol-function cmd) holds an autoload object, not sure if it's
> possible here, but if it is, maybe an `autoload-do-load` can workaround
> the problem 🙁).
When a transient prefix is invoked, all autoloaded suffix are loaded
like that already. But non-suffix commands are not, so I'll start
doing it here as well.
> But if it returns nil then we add the function to the `cmd` place,
> i.e. we do something equivalent to
>
> (setq cmd (advice--make :around advice cmd))
>
> But since `cmd` is a local var, this `setq` is lost when we exit
> the function.
>
> Maybe you want something more like:
>
> (add-function :around (if (symbolp cmd) (symbol-function cmd)
> this-command)
> advice '((depth . -99))))))
>
> so when `this-command` is not a symbol we do something akin to:
>
> (setq this-command (advice--make :around advice this-command))
Since the command could change the value of that variable, we might end
up failing to remove the advice, if we do that. The simplest solutions
seems to be to just use a dedicated global variable, instead of either a
local variable or an existing global variable, something else may choose
to mess with (including real-this-command).
(defvar transient--this-command nil
"Keep your mitts off.")
(defun transient--wrap-command ()
(when (autoloadp this-command)
(autoload-do-load this-command))
(setq transient--this-command this-command)
(static-if ... /s/cmd/transient--this-command/ ...))