Respect value of g:VimuxRunnerIndex

[3ximus]

Hello!
This PR aims to solve an issue I have that is to select a specific pane to be the runner (any pane across all sessions). I found that the behavior to interact with the runner was slightly inconsistent and didn't quite allow for that.

The best solution I found was to select the runner through the g:VimuxRunnerIndex, but without these fixes I wasn't able to use it reliably since VimuxOpenRunner would try to get a new runner every time and VimuxRunCommand wouldn't recognize the pane unless it's on the current window.

I tested most use cases and it seems to respect the old behavior. Let me know if you agree with this.

PS: This is how I'm using it btw using fzf to pick a pane from the current session (could be adapted to do the same for all sessions)

function s:VimuxSetPane(line)
  let l:pane_path = substitute(a:line, '[* ]\+\(\d\+\.\d\+\) .*','\1', '')
  let g:VimuxRunnerIndex = substitute(VimuxTmux('display -t '.l:pane_path.' -p "#{pane_id}"'), '\n$', '', '')
endfunction

function s:VimuxPickPaneFzf()
  let opts = {
        \ 'source': 'tmux list-panes -s -F "\033[1;33m#{?pane_active,*, }\033[m:#I.#P:#{pane_current_command}:\033[1;30m#{?#{==:#W,#{pane_current_command}},,(#W)}\033[m:#{?window_linked,\033[1;36m[linked]\033[m,}"|column -ts":" -o" "|while read -r l;do echo "$l";done',
        \ 'sink': function('s:VimuxSetPane'),
        \ 'options': "--prompt 'Vimux Pane> ' --delimiter='[* ]+' --nth=2.. --ansi --no-info --preview='tmux capture-pane -ep -t {2}|cat -s|tail -n $(tput lines)' --preview-window=up,70%"}
  call fzf#run(opts)
endfunction

command! -nargs=0 FZFVimuxPickPane call s:VimuxPickPaneFzf()

[andrewdollard]

Thank you for this! This PR fixes an ongoing headache in my workflow. Would love to see this PR get merged.

[mvanderkamp]

I think VimuxTogglePane is broken at the moment for this workflow.

I really like the use of -a for tmux list-panes. It has me wondering if we could simplify the plugin by always tracking a pane ID. At the moment, commands can be issued to the wrong pane in a window runner if there are multiple panes in the window, and this seems like a good way of resolving that while also potentially simplifying the code.

I don't think that should be done in this PR though.

As for toggling not working, since using a runner pane from a different window or session is a new workflow, I think it's probably not critical to support it. Can you take a look to see if there's a simple solution though?

[mvanderkamp]

tmux list-windows supports -a as well, is there a reason why we shouldn't use it for window runners?

[3ximus]

I didn't know about the existence of VimuxTogglePane, it's not documented, and it doesn't really make sense to me to have something like that on this plugin. In what scenario is it of use to you as opposed to zooming or break the pane?

For that reason I don't feel like digging too much into the issue but putting this at the beginning of the function at least prevents the error:

function! VimuxTogglePane() abort
  if exists('g:VimuxRunnerIndex') && g:VimuxRunnerIndex !=# ''
    ...

Regardless, I agree with you that it might not be a necessity to make VimuxTogglePane work in this PR, and using the global pane id to identify the pane would definitely make it simpler to manage that function.

[mvanderkamp]

I didn't know about the existence of VimuxTogglePane, it's not documented, and it doesn't really make sense to me to have something like that on this plugin.

It's not something I use either, but that doesn't mean no one does. I imagine someone might like to "unclutter" their window from time to time by sending their runner away without closing it or having to zoom in to just one pane. In any case the command has been around for over a decade now so I don't want to break anyone's workflow. It should be documented though, that's a fair point.

I think for this PR just update the hasRunner changes so that it uses -a for both panes and windows and I'll merge this in.

Odd that checks haven't run on this PR though, maybe they will when you push another commit?

[mvanderkamp]

@3ximus Checking in- I'd like to build on top of this, and I think we can simplify by just using -a for both runner types.

[3ximus]

Hi there, sorry for not getting back to you sooner. Adding the -a for both options breaks it when RunnerType is set to window.

I've also fixed the merge conflicts and rebased

[mvanderkamp]

Can you clarify what you mean by -a not working for windows? It seems to behave correctly for me:

% tmux list-windows -a -F '#{window_id}'
@1
@2
@3
@4
@5
@6
@7
@8
@9
@26
@15
@16
@17
@21

🤔 Perhaps this flag was recently added, I'll check.

[mvanderkamp]

This needs to compare against -1, since non-zero is truthy.

[mvanderkamp]

This function does not accept index anymore.

[3ximus]

I meant that using the -a option with VimuxRunnerType = 'window' was breaking vimux for me. That option still doesn't work well for me but I don't think it's from the -a and idk if I was testing it correctly.
Regardless. I've pushed the changes you requested. It should be ready to merge

[mvanderkamp]

I meant that using the -a option with VimuxRunnerType = 'window' was breaking vimux for me.

Oh that's definitely possible. I found a few new bugs the other day while working on a branch to try out always using a pane ID for the runner index, even for window runners.

Thank you for making those changes! There's still a bug in this branch though- see my comment.

[mvanderkamp]

We still need this returning a comparison against -1, as it was before, since match() returns the byte location where the match was found, or -1 if no match was found, and -1 is truthy in vim.

I like to avoid doing too much in one line, even if it means more lines of code and more variables- unless a good performance case can be made. Can you please keep the separate lines for defining the command and capturing the match result?