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

[WIP] Use tmux variable to indicate running Vim #201

Closed
wants to merge 5 commits into from
Closed

Conversation

blueyed
Copy link
Collaborator

@blueyed blueyed commented Apr 30, 2018

This uses @tmux_navigator to hold a list of tmux panes when
vim-tmux-navigator is running.
It uses VimSuspend/VimResume (available on Neovim) to remove itself when
it is suspended (Ctrl-z).

(It could easily get extended to detect if vim-tmux-navigator is not
responding (e.g. when a more-prompt is active), but that would involve
an additional (async) command every time vim-tmux-navigator is used.)

Includes #200.

TODO:

  • review docs (which mention e.g. "grep pattern", which is removed now)
  • document / bump minimal tmux version - or can it be fixed?

@blueyed blueyed changed the title Use tmux variable to indicate running Vim [RFC] Use tmux variable to indicate running Vim Apr 30, 2018
@christoomey
Copy link
Owner

@blueyed can you clarify the problem(s) that this is solving, and or new functionality this would bring in?

I'm extremely cautious about adding to or altering the process matching code as it is already very complicated and somewhat unwieldy, and I want to better understand what these changes might gain us in order to review more completely.

@blueyed
Copy link
Collaborator Author

blueyed commented May 3, 2018

@christoomey
It does not match processes anymore, but makes the plugin indicate that it is running in $TMUX_PANE instead (via a tmux variable).

@apazzolini
Copy link

@blueyed This change is awesome - completely eliminates the lag present with the previous if-shell test.

One caveat is that this only works for me on tmux 2.7 - it doesn't detect that it's in vim on tmux 2.5 (which has a nicer choose-window interface IMO). I'm not sure if there's something new in 2.6 or 2.7 that you need to do the detection, but it's at least worth mentioning in the README.

@apazzolini
Copy link

Also, I needed to add

if empty($TMUX)
  finish
endif

to the top of the vim plugin to allow using neovim outside of tmux.

@blueyed
Copy link
Collaborator Author

blueyed commented May 16, 2018

@apazzolini
Thanks for the feedback.
Yes, it certainly requires some newer tmux, which we would need to find out - not sure if it can be made compatible with older versions yet.

As for the if empty($TMUX) check I will add / fix it.

@blueyed
Copy link
Collaborator Author

blueyed commented May 16, 2018

@apazzolini
Do you use g:tmux_navigator_no_mappings = 1?
(since having the finish would skip the setup for the default mappings)

@blueyed
Copy link
Collaborator Author

blueyed commented May 16, 2018

btw: is it a valid use case that empty($TMUX) might change during runtime? /cc @christoomey
I would say that it is not, and that the plugin could be simplified to only setup the direct default mappings if $TMUX is empty.

@apazzolini
Copy link

I think that ideally it should setup the default mappings if you're not running inside of tmux - that's almost certainly what the majority of people would want.

I have the mappings set in my vimrc by default since I also use it on remote servers where I'm not running tmux.

@blueyed
Copy link
Collaborator Author

blueyed commented May 16, 2018

Ok, see #203.

@blueyed
Copy link
Collaborator Author

blueyed commented May 16, 2018

The feature that requires tmux 2.6 is the pattern matching using m: (tmux 2.5-69-gd3959a21).
I've checked if '#{s/#{pane_id}//:@tmux_navigator}' could be used to compare it to @tmux_navigator, but #{pane_id} does not seem to get replaced.
So for tmux 2.5 and older if-shell -F cannot be used, but it would require the following pattern:

bind-key h if-shell 't="#{@tmux_navigator}"; [ "${t##*-#{pane_id}-}" != "$t" ]' 'send-keys C-h' 'select-pane -L'

@sajoku
Copy link

sajoku commented Jun 26, 2018

@blueyed I'm running the indicator branch on my machine and this seems to work marvellously 👍
Is there something I can do to help get this into master?

@blueyed
Copy link
Collaborator Author

blueyed commented Jun 26, 2018

@sajoku
Great!
See the TODOs above. I think reviewing docs would be good.
You can send me a patch here.

Rebased it on master now, fixing the conflict due to d030f75.

I am using this in ~/.tmux.conf currently, after feedback from tmux's author on IRC:

# tmux >= 2.6
# session_format (like pane_format and window_format) are always 1, at least
# when used with "display".  Otherwise a shell would be required to test for
# non-empty.
# nicm | it can be 0 in the mode formats
# nicm | ie choose-tree -F
# nicm | but it should be 1 otherwise i think
# nicm | could also probably try one of the new things like && or || to detect it
# …
# nicm | blueyed: something like #{||:#{==:#{version},master},#{||:#{m:#{version},[3456789].*},#{m:#{version},2.[6789]}}}
# nicm | i think it would fail to empty on <2.6
# nicm | but i didn't test it
# nicm | er also i got the m: pattern wrong way round
# nicm | #{||:#{==:#{version},master},#{||:#{m:[3456789].*,#{version}},#{m:2.[6789],#{version}}}}
if -F '#{session_format}' "\
  bind-key h if-shell -F '#{m:*-#{pane_id}-*,#{@tmux_navigator}}' 'send-keys C-h' 'select-pane -L' ;\
  bind-key j if-shell -F '#{m:*-#{pane_id}-*,#{@tmux_navigator}}' 'send-keys C-j' 'select-pane -D' ;\
  bind-key k if-shell -F '#{m:*-#{pane_id}-*,#{@tmux_navigator}}' 'send-keys C-k' 'select-pane -U' ;\
  bind-key l if-shell -F '#{m:*-#{pane_id}-*,#{@tmux_navigator}}' 'send-keys C-l' 'select-pane -R'" \
  "bind-key h if-shell 't=#{@tmux_navigator}; [ x\${t%-#{pane_id}-*} != x\$t ]' 'send-keys C-h' 'select-pane -L' ;\
  bind-key j if-shell 't=#{@tmux_navigator}; [ x\${t%-#{pane_id}-*} != x\$t ]'  'send-keys C-j' 'select-pane -D' ;\
  bind-key k if-shell 't=#{@tmux_navigator}; [ x\${t%-#{pane_id}-*} != x\$t ]'  'send-keys C-k' 'select-pane -U' ;\
  bind-key l if-shell 't=#{@tmux_navigator}; [ x\${t%-#{pane_id}-*} != x\$t ]'  'send-keys C-l' 'select-pane -R'"

This should work with older tmux then, too.
/cc @apazzolini

@blueyed blueyed changed the title [RFC] Use tmux variable to indicate running Vim [WIP] Use tmux variable to indicate running Vim Jun 26, 2018
@dkao1978
Copy link

dkao1978 commented Jul 3, 2018

Indicator branch also fixed all my problems. Thanks!

blueyed referenced this pull request Jul 15, 2018
Previously we used the `{pane_current_command}` tmux variable and
matched against that, but a while back we switched to using the process
list to better handle some edge cases.

Overall that change has been a win, but the documentation has lagged a
bit around the debugging command `:TmuxPaneCurrentCommand` which is no
longer relevant.

This change replaces `:TmuxPaneCurrentCommand` with a new
`:TmuxNavigatorProcessList` which should allow us to debug the actual
issue related to the `ps` command.
@dmhenry
Copy link

dmhenry commented Jul 17, 2018

@blueyed Regarding your comment on the issue I opened: I'm using the default Ctrl-h,i,j,k (no prefix) for Vim/NeoVim and Tmux both. I ran tmux show @tmux_navigator as you suggested within a Tmux pane (with Vim 8.1.50 or NeoVim 0.3.0 running in one of the other panes). This resulted in unknown option @tmux_navigator. The master branch of vim-tmux-navigator works properly within Vim splits, but won't allow me to escape to another pane. This branch fixes Vim->Tmux, but then the Vim Ctrl-h,i,j,k binds don't work for Vim splits within a Tmux pane.

@blueyed
Copy link
Collaborator Author

blueyed commented Jul 17, 2018

@dmhenry

unknown option @tmux_navigator

That means the binding from this PR here are either not installed, or Vim wasn't started in any pane yet - this custom options/var gets used to communicate with tmux about in which panes this plugin is running.

@sajoku
Copy link

sajoku commented Jul 25, 2018

@blueyed I do have a bug, or what I think is a bug.

Open a split in tmux (numbers are the splits). [1|2]
Open vim in split 1.
Open vim in split 2.
Close vim in split 2.
Now try to navigate to split 1. <- This step fails in my case.

Extra:
Open vim in split 2.
Navigate to split 1. Everything works correct.

Are you able to reproduce this?

blueyed added 3 commits July 26, 2018 13:10
- use a list instead of single string
- add s:GetTmuxCommand to be used later when not using `system()` only
- remove `:silent` when calling tmux from `s:TmuxAwareNavigate`: it
  should not be necessary and is bad practice to use `:silent`
  unnecessarily.  It was added in b068a04 with no explanation.
This uses @tmux_navigator to hold a list of tmux panes when
vim-tmux-navigator is running.
It uses VimSuspend/VimResume (available on Neovim) to remove itself when
it is suspended (`Ctrl-z`).

(It could easily get extended to detect if vim-tmux-navigator is not
responding (e.g. when a more-prompt is active), but that would involve
an additional (async) command every time vim-tmux-navigator is used.)
@blueyed
Copy link
Collaborator Author

blueyed commented Jul 26, 2018

@sajoku
Hmm, worked for me in Neovim, but could reproduce in Vim.
Pushed some new commits - should work now.
Otherwise check tmux show @tmux_navigator from another pane - it should have the list of panes where Vim is running - also available as :TmuxNavigatorPaneIndicator.

@phantomwhale
Copy link

Just moved from vim to nvim and found using this branch (as well as the suggested tmux changes within the updated documentation on the branch) was required to re-enable my Ctrl-h/j/k/l movements in Neovim again.

Thought I managed to recreate @sajoku's bug (using vim) initially, but tried 20 more times and couldn't reproduce it again, so a little unsure how I managed it and if it's a "real" bug, or just a hiccup from changing and reloading tmux / vim configuration whilst within those tools.

@gdraps
Copy link

gdraps commented Jan 6, 2019

Thanks for this PR. I came here from #208. However, on the current indicator branch commit afef84a, with vim (not neovim) on windows/cygwin, simply opening and closing vim leaves tmux show @tmux_navigator at "-%0-" instead of "". Looks like indicator is being removed with just tmux set @tmux_navigator (missing ""), which returns "empty value" and does not clear the tmux variable.

Here's a quick hack I made to escape the empty string. Not sure whether this is robust (i.e., do any functions need to use fnameescape() if that's no longer done in TmuxCommand()).

diff --git a/plugin/tmux_navigator.vim b/plugin/tmux_navigator.vim
index 5445bfe..c7a9005 100644
--- a/plugin/tmux_navigator.vim
+++ b/plugin/tmux_navigator.vim
@@ -73 +73 @@ else
-    let cmd = join(map(s:GetTmuxCommand(a:args), 'fnameescape(v:val)'))
+    let cmd = join(s:GetTmuxCommand(a:args))
@@ -156,0 +157,4 @@ function! s:remove_indicator() abort
+  if !has('nvim')
+    " Shellescape new string, in case it's empty
+    let new = shellescape(new)
+  endif

@Xenorage
Copy link

Xenorage commented Jan 6, 2019

Unhmm took me 2 hours to figure out this is another branch ...
Can confirm this works on Cygwin unlike master branch. This should be merged to master ASAP!

@Xenorage
Copy link

Xenorage commented Jan 6, 2019

Its a bit hard to find witch pane is active. Can the active pane border flash for a second or some other method to see witch plane is active?
There is a small bug. If you go right (or left) in circle all panes. When you enter again in a vim pane you will be in the far right vim split not in the far left.

@blueyed
Copy link
Collaborator Author

blueyed commented Jan 6, 2019

Its a bit hard to find witch pane is active. Can the active pane border flash for a second or some other method to see witch plane is active?

That would be a new feature request.
AFAIK it is not possible easily, except for maybe changing border style temporarily etc.
Anyway, it is also out of scope of this plugin, since you would have the same problem with normal pane switching.

I'll close this PR - I am not using this plugin anymore myself (too much magic - I rather prefer to distinguish between Vim and tmux explicitly).
Feel free to create a new PR (including the fix from above) from my branch.

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.

9 participants