protesilaos / pulsar

Emacs package to pulse the current line after running select functions.
https://protesilaos.com/emacs/pulsar
GNU General Public License v3.0
62 stars 3 forks source link

Pulsar and function aliases #11

Closed shipmints closed 3 weeks ago

shipmints commented 1 month ago

Function aliases are the kind of thing might trip up new users. The core tab-bar package commands, listed below next to their convenience aliases, do not trigger pulsing while their listed aliases do. Command handling via this-command does not resolve function aliases on its own.

(defcustom pulsar-pulse-functions
...
    ; triggered    not triggered
    tab-close ; -> tab-bar-close-tab
    tab-new   ; -> tab-bar-new-tab
    tab-next  ; -> tab-bar-switch-to-next-tab
...

My suggestion is that either they're all listed twice, once for the convenience alias, and once for the real function, or add a test via function-alias-p if any of their resolved function names appear in pulsar-pulse-functions. Considering how often post-command-hook gets called, I'd opt for the most runtime-efficient option.

Thank you for another fine package.

protesilaos commented 4 weeks ago

From: shipmints @.***> Date: Sun, 11 Aug 2024 12:01:54 -0700

[... 11 lines elided]

My suggestion is that either they're all listed twice, once for the alias, and once for the real function (I use only the real functions, for example), or add a test via function-alias-p if any of their resolved function names appear in pulsar-pulse-functions. Considering how often post-command-hook gets called, I'd opt for the most runtime-efficient option, and perhaps advise that list be kept to a minimum.

I think it is better for us to refine the code internally so that it does the right thing. This will be easier for us but also better for the user who will not need to list the aliases as well.

Do you want to send a patch or pull request for this? I am happy to merge it.

Thank you for another fine package.

You are welcome!

-- Protesilaos Stavrou https://protesilaos.com

shipmints commented 3 weeks ago

Hi, @protesilaos, let me know if there's anything else we need to do with this.

protesilaos commented 3 weeks ago

I could not review this earlier. Did it now, thanks! Let's close this and keep the conversation in the pull request.