christoomey / vim-tmux-navigator

Seamless navigation between tmux panes and vim splits
MIT License
5.22k stars 322 forks source link

Vim -> Vim not working in MSYS2 and Cygwin #246

Open pnetherwood opened 4 years ago

pnetherwood commented 4 years ago

The tmux plugin doesn't work for systems like MSYS2 and Cygwin because the ps command is much simpler and hasn't got the format options required for the Tmux plug in.

The original line:

is_vim="ps -o state= -o comm= -t '#{pane_tty}' \
    | grep -iqE '^[^TXZ ]+ +(\\S+\\/)?g?(view|n?vim?x?)(diff)?$'"

fails in MSYS2 and Cygwin because the native ps command does no support -o or -t.

I have fixed it with the following line:

is_vim="ps -a |grep -q '.*''#{s|/dev/||:pane_tty}'' .*vim$'"

which works on both MSYS2 and on my Ubuntu systems. The command relies on the tmux variable substitution to remove the /dev/ from the pane tty name and then greps for a tty name and vim from the ps output. It doesn't do the same process state filtering that the original does but that info is not available in the MSYS2 and Cygwin ps command.

It might be worth considering if this is a more generic cross platform solution.

pnetherwood commented 4 years ago

Here's the extended is_vim line with the regex to cover the variants of the vim name as per the original:

is_vim="ps -a |grep -qE '.*''#{s|/dev/||:pane_tty}'' .*+(\\S+\\/)?g?(view|n?vim?x?)(diff)?$'"
christoomey commented 4 years ago

Hey @pnetherwood, thanks for the detailed notes and the suggestion. I'm generally very conservative with changes to the core vim matching logic, but this seems straightforward enough and should improve portability, so I think it sounds like a worthwhile change.

Would you mind turning this into a PR, updating the reference to the snippet in both the README and the tmux plugin manager version?

christoomey commented 4 years ago

Hmm, so I'm running with this update locally and it doesn't seem to work, specifically the Vim detection portion. I'm running nvim, so I'm guessing it has to do with the grep pattern, but this gives me a bit more pause in moving to this. Any thoughts?

pnetherwood commented 4 years ago

What platform are you on? Does the output of ps -a include the tty?

Since there is no -t for the ps command on MSYS2 I just add the pane tty to the grep but strip out the /dev/ part using tmux variable substitution. Try this in a shell inside tmux to test:

tmux run-shell "echo '#{s|/dev/||:pane_tty}'" 

You should see a tty name that should in the the ps -a list. We can grep out the current pane's tty from the list using.

tmux run-shell "ps -a |grep -E '#{s|/dev/||:pane_tty}'"

The grep pattern is then extended to have the second part of the original grep. Try running this from nvim inside tmux from the nvim command line:

!tmux run-shell "ps -a|grep -E '.*''\#{s|/dev/||:pane_tty}'' .*+(\\S+\\/)?g?(view|n?vim?x?)(diff)?$'"

(Note I've escaped the # so it can run from vim). You should see the output from the ps command for the nvim session that you are running if it succeeds.

This is what I used to debug it but I've only tried it in MSYS2 and on Ubuntu.

pnetherwood commented 4 years ago

I've figured out the issue with this. It relies on tmux substitution format which needs to be in this form: s|/dev/|| to avoid conflicts with /. This appears to be only available in later versions of tmux and I'm using version 3 on MSYS2 and Ubuntu. So if you run

tmux run-shell "echo '#{s|/dev/||:pane_tty}'" 

on tmux 3 you get the tty without the /dev/. On other earlier versions you get no output. So minimum requirement for this PR is Tmux 3.0 so should not be merged unless a work around can be found perhaps using sed.

You could point people to this PR if they are trying to get it to work on MSYS2. It works fine on WSL as its ps command supports -t.