tmux-plugins / tpm

Tmux Plugin Manager
MIT License
11.98k stars 419 forks source link

TPM doesn't read tmux options correctly—switch from regex to querying option value #85

Open zeorin opened 8 years ago

zeorin commented 8 years ago

TL;DR: Help me convince tmux to support appendable user options (like other string options), then you can inspect e.g. @plugins user option and we can do cool stuff like include plugins depending on some condition like tmux version or OS type or whatever.

I tried to have a bit in my .tmux.conf file that reads like this:

if-shell "~/.tmux/tmux-version.sh '-ge' '2.1'" \
    "set -g @plugin 'NHDaly/tmux-scroll-copy-mode'"

But this doesn't work. Weird. It works for other options.

After reading through the TPM code, I determined that TPM is trying to get the list of plugins by reading .tmux.conf instead of asking tmux what @plugin option values have actually been set.

I know if-shell returns true because echo "$(tmux start-server\; show-option -gqv "@plugin")" prints NHDaly/tmux-scroll-copy-mode to the console, even though that plugin is not loaded.

Wait a sec… I'd expect there to be more than one plugin listed (I've defined more before this conditional assignment).

Some digging has revealed that this is presumably because user options cannot be appended to, i.e you can't construct a string to explode into an array of values later, unlike other, builtin, string options.

I've opened an issue in tmux about this: https://github.com/tmux/tmux/issues/533. If we get appendable user options you could perhaps create a new option @plugins to explode into an array and loop through. That way we can have one .tmux.conf file for many different systems where different plugins are desired to be loaded.

ashb commented 7 years ago

\o/ looks like this landed in tmux 2.3:

From the changelog:

  • 'set -a' for appending to user options (@foo) is now supported.
samm81 commented 5 years ago

I'm running into this issue still about two and a half years later. Should I open an issue for moving away from the awk based parsing?

Currently I work around this by leaving the " on the first line so that the set -g @plugin 'plugin-name' still matches the awk regex, like so:

if-shell "[ ! -z ${TMUX_CONTINUUM} ]" " \
  set -g @plugin 'tmux-plugins/tmux-resurrect'"

but it seems crazy that this is still done by parsing the .tmux.conf file with grep... if tpm queried the tmux user options correctly this would also solve #57

cspotcode commented 4 years ago

If tpm isn't even reading the @plugin value then why are we using set -g syntax? That seems so unnecessarily bizarre and confusing.

I would prefer to list plugins in a file like ~/.tmux/plugins.txt, one per line. It's much simpler and it actually makes sense.

spencerbutler commented 4 years ago

With PR #163 You can just add the following to your custom config file:

set -g @tpm_conf_default_location '/path/to/tmux.conf.local'

samm81 commented 4 years ago

@spencerbutler I'm not sure how #163 actually solves this issue. The issue here is that tpm is not actually checking the value of @plugin. #163 seems to solve the problem of allowing a new default tpm configuration location.

What @cspotcode wants is not the ability to move the set -g @plug commands to a new location, but for tpm to simply just read the list of plugins from a separate text file.

zeorin commented 4 years ago

Having them in a single text file is less flexible. With the ability to set the option (and tmux actually querying the value of that option), we can do interesting things like loading plugins conditionally.

If we have that, you could still opt to set your plugins in a separate sourceable tmux file, specifying one plugin per line (appending one plugin to the option per line).

It gives everyone what they want.

cspotcode commented 4 years ago

I don't actually need or want to have the options in a separate file; I was just rhetorically pointing out that the current implementation -- hacky regex parsing of the config file instead of actually reading a value from the proper API -- is nonsense.

zeorin commented 4 years ago

Right. I suggest that we create a new option @plugins and query that. If it's set, then we should ignore @plugin. @plugin should be deprecated, and eventually removed.

FranklinYu commented 2 years ago

About the implementation detail, a separator is needed, because it’s simply string concatenation instead of a real array. What could be the separator? I guess colon (:) isn’t great, since it also appears in the Git addresses like git@github.com:user/plugin. I noticed that built-in options for styles (like status-style) is essentially a “comma or space separated list”; I guess we can accept the same for @plugins?

Bash implementation could be

arr=(${str//,/ })

(Note that this doesn’t work for Zsh.)

aspiers commented 2 years ago

In Zsh you can do that like this:

$ foo='one,two three,four'
$ arr=("${(s:,:)foo}")
$ typeset arr
arr=( one 'two three' four )

This is documented in https://zsh.sourceforge.io/Doc/Release/Expansion.html#Parameter-Expansion-Flags (search for "field splitting").

FranklinYu commented 1 year ago

Hey everyone, I found that the @plugins we asked for actually already exist. As https://github.com/tmux-plugins/tpm/pull/112#issuecomment-439616732 and https://github.com/tmux-plugins/tpm/pull/142#issuecomment-439616514 mentioned, it is called @tpm_plugins. Now that tmux has the -a flag for set-option, I think it’s time to update ReadMe recommending this approach?

set-option -g @tpm_plugins ''  # to be idempotent
set-option -ag @tpm_plugins 'tmux-plugins/tpm '
set-option -ag @tpm_plugins 'tmux-plugins/tmux-resurrect '

Caveat: only space is supported as the delimiter; comma is not. I’m good with it, though. If people do care about it, we can add a string replacement I mentioned above:

https://github.com/tmux-plugins/tpm/blob/fc412cbdf765175ea9f3380db6b9e1b1603a68a7/scripts/helpers/plugin_functions.sh#L73

Update: it isn’t too much work, so I went ahead and created #238.

edentsai commented 1 year ago

I use set-option -g -a @tpm_plugins "{plugin} \n" as similar workaround, for example:

set-option -g @tpm_plugins '';
set-option -g -a @tpm_plugins "tmux-plugins/tpm \n";
set-option -g -a @tpm_plugins "tmux-plugins/tmux-resurrect \n";

this works with current version TPM.