Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add command aliases #211

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

verschmelzen
Copy link

Added aliases with the same function as the bindings and the option @tpm-aliases to enable/disable them. In the current PR the option is on by default. Should this be changed to 'off' for the backward compatibility?

@bruno-
Copy link
Member

bruno- commented Aug 21, 2021

I didn't know tmux had this feature! I wanted something like this for a long time. Too bad the manpage is so terse.

Here are a couple thoughts on this:

  • I think most of the plugins will eventually implement commands.
  • It's kinda important what we do in tpm because people may follow the pattern (copy - paste - tweak) from here. Let's be thoughtful about this.
  • I can see this change in all the plugins: Instead of binding keys to run-shell commands, we'll:
    • First add command aliases
    • Bind keys to command aliases
    • Less used commands will not have bindings
  • I propose we name this feature commands instead of command-aliases. They are "aliases" (and not real commands) from the perspective of core tmux. But, from the perspective of the users they are just commands.

Naming

  • Should we prefix commands with the plugin name? All the commands provided by tpm would start with tpm-? This could be a nice convention, but it's more to type, so I'm not sure about this.
  • I think command names should be named the same as shell scripts. So install_plugins script would have install-plugins commands. We can change script names if needed.

@bruno-
Copy link
Member

bruno- commented Aug 21, 2021

I'm so much under the impression of what can be done with this tmux feature...

@verschmelzen
Copy link
Author

@bruno- you have to dig for some stuff in the manual indeed)

I very much agree on all the points and have updated the PR accordingly. Vim/Neovim plugins are already following this logic. I believe it to be very convenient when plugins implement commands first and then leave it up to the user to implement Fzf / completion / static bindings / use default bindings / etc for the quick access to the features they provide. Few things to consider:

  • Since command-alias is used first time here we need to make sure that the minimal tmux version is correct
  • How far should we go with renaming of the scripts. In the current PR bindings directory was renames to the commands and the scripts in this directory renamed to the names of corresponding commands.

Please share any other requirements that need to be implemented.

@bruno-
Copy link
Member

bruno- commented Aug 24, 2021

Thank you for making the changes, almost all of them are great.

So, I made a bunch of comments above while I was under the initial impression of what's possible with this feature. After sleeping on it for a couple days and looking at your changes I have some fresh thoughts.

Hopefully you are ok with this slow process? The impact of the changes we make here is big.

New thoughts:

  • Now I think we should NOT prefix command names with plugin name. tpm- is short to type, but take for example a relatively popular tmux-resurrect plugin. Should resurrect commands be: resurrect-save and resurrect-restore? That looks ugly to me.
  • At this point I think tpm commands should be named install-plugins, update-plugins and clean-plugins.

Since command-alias is used first time here we need to make sure that the minimal tmux version is correct

If you can find tmux version when command-alias was introduced, I'd be ok bumping minimum required tmux version for tpm.

  • It seems command-alias variable is just an array. If you run tmux set -ag command-alias "foo=echo bar" multiple times, it will show multiple times in the output of tmux show -g command-alias. This is something we need to solve because we can expect the users to source their .tmux.conf multiple times, and this will run tmux set -ag command-alias ... each time.

Thoughts related to commands, but not related for this PR:

  • How often do users install/update plugins? One a month? Once a year? Now that we have convenient commands, I think actions that happen as often as "once a week" don't justify having a default key binding. So, after we merge this I think I'll make another PR that deprecates key bindings in favor of commands. Long term, I'll remove default key bindings from tpm.

@verschmelzen
Copy link
Author

verschmelzen commented Oct 9, 2021

@bruno- agree with your point regarding command naming. Just think this should be opt-out feature in case someone might have their own meaning for these commands. Or make both tpm- prefixed commands and plain names (like :update), and make the plain names opt-out

EDIT: if we take a look at the vim plugin managers, i believe, they have their plugin name as prefixes as common practice. Maybe we could just explain to the user in the README that: hey, you can make the commands more convenient for yourself, if you wish, here is the snippet, put it in your tmux.conf

tpm
# tpm-update - updates a plugin (or all of them) and reloads TMUX environment
# tpm-clean - remove unused TPM plugins and reloads TMUX environment
set_tpm_commands() {
tmux set-option -ag command-alias "tpm-install=run-shell $COMMANDS_DIR/tpm-install"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement isn't idempotent because of the -a flag, so be careful. If someone reloads their config, this file could get sourced again and append duplicate aliases. tmux seems to use the first registered alias for a given key, so subsequent calls to set_tpm_commands would effectively be ignored until a new session is started.

I also noticed a bug where using -a causes aliases that contain commas to be split across multiple entries. This shouldn't be an issue for this PR, but it makes me feel like using -a with array options like command-alias isn't fully tested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants