`is_vim` does not detect vim running in a subshell

Question

`is_vim` does not detect vim running in a subshell

jyurek opened this issue 4 years ago · 14 comments

I have a project where I am working in Python. I'm using pipenv to manage the environment, which has a feature where if you run pipenv shell it starts a subshell with everything set up right for the environment. This starts a new TTY.

However, that means that ps ... -t "#{pane_tty}" returns the "wrong" process list. It returns

  PID TTY           TIME CMD
92716 ttys001    0:00.85 -bash
96594 ttys001    0:22.72 [/path/to/]Python /usr/local/Cellar/pipenv/2020.11.1

But just ps alone returns (and you can see vim running inside /dev/ttys002)

  PID TTY           TIME CMD
14445 ttys000    0:00.29 -bash
14803 ttys000    0:00.00 /bin/sh /Users/jyurek/bin/tm [my_project]
14804 ttys000    0:00.01 tmux new-session -As [my_project]
92716 ttys001    0:00.85 -bash
96594 ttys001    0:22.72 [/path/to/]Python /usr/local/Cellar/pipenv/2020.11.1
53522 ttys002    0:19.13 vim [source_file].py
96606 ttys002    0:01.01 /bin/bash -i
 8581 ttys003    0:00.33 -bash
61829 ttys003    0:07.61 [/path/to/]Python /usr/local/Cellar/pipenv/2020.11.1
61835 ttys005    0:04.68 /bin/bash -i

The end result is that the is_vim clause in the pane-switching bindings doesn't return true, so it doesn't send the keys to vim, even though it's the program in the foreground.

I'm not really sure how to fix this, since apparently OSX's ps can't show the lineage of processes to know that vim is, in fact, running as an (eventual) sub-process in /dev/ttys001.

Answer 1 · 2021-05-29T13:20:09.000Z

Hey John 👋. Well, this is certainly a fun one. I'll be honest that I don't have a great grasp of the ps mechanics at play here (those tweaks were added by a contributor), but I think you might be able to tweak things on your end to get a working solution. The subtlety lies in avoiding false positives so I'm always reticent to change the vim matching logic, but here's two suggestions:

Try out Mislav's original script (I wouldn't be surprised if this didn't work either as it's purposefully a bit less thorough in how it tries to match processes, but could be an option)
Use an alternative matching function. Testing locally against the ps list you provided, something like ps -t '#{pane_tty}' | tail -n +2 | awk '{print $4}' | grep seems to work (although I'm sure there are other edge cases).

Hopefully something in there will help you get back to working.

Answer 2 · 2021-06-01T11:52:23.000Z

Hi, Chris! Sadly, no, both of those don't work.

Through a bit more digging, I found ps -o pid= -o ppid= which may lead to some angle, but that's going to take longer than I have at the moment.

Answer 3 · 2021-08-10T18:04:13.000Z

Hi, I'm also running into this issue. It seems like '#{pane_tty}' approach alone will not pick up on vim in a pseudoterminal spawned by a process in the original tty (i.e. if I'm using screen, nested tmux, a subshell, or anything that spawns a new pseudoterminal inside of a tmux pane). In my particular case, I'm using https://github.com/withfig/autocomplete which spawns a nested pseudoterminal for each pane in tmux, which unfortunately breaks this extension.

I thought one possible solution would be something like https://stackoverflow.com/a/52544126 which constructs a process tree from a command like what John mentioned above ps -o pid= -o ppid= and then traverses it to find all child processes of a given process.

This approach could be used to get all child processes running in '#{pane_tty}' and check them all for vim.

Wondering if this sounds like a reasonable way to avoid false positives in the full ps list?

Answer 4 · 2021-09-17T21:29:52.000Z

Following up on this - a temporary fix for anyone having this issue is to do the following:

Copy the below script into a shell file and make it executable (e.g. touch ~/is_vim.sh && chmod +x ~/is_vim.sh)

#!/usr/bin/env bash
# Usage: is_vim.sh <tty>
#   tty: Specify tty to check for vim process.
tty=$1

# Construct process tree.
children=();
while read -r pid ppid; do
  [[ -n $pid && pid -ne ppid ]] && children[ppid]+=" $pid"
done <<< "$(ps -e -o pid= -o ppid=)"

# Get all descendant pids of processes in $tty with BFS
idx=0
IFS=$'\n' read -r -d '' -a pids < <(ps -o pid= -t $tty && printf '\0')
while (( ${#pids[@]} > idx )); do
  pid=${pids[idx++]}; pids+=( ${children[pid]-} )
done

# Check whether any child pids are vim
ps -o state= -o comm= -p "${pids[@]}" | grep -iqE '^[^TXZ ]+ +(\\S+\\/)?g?(view|n?vim?x?)(diff)?$'
exit $?

Replace the is_vim line in this plugin with is_vim="~/is_vim.sh '#{pane_tty}'". If you installed this plugin with tpm you can make this change in your .tmux/plugins folder, and if you installed this plugin by copying lines to your .tmux.conf then you can make this change there

@christoomey I'm happy package this in a PR, but wasn't sure if it was preferable to include the bash script as a separate file or to wrestle with multi-line string escaping inside the tmux file itself.

Answer 5 · 2022-01-25T20:42:59.000Z

UPDATE

I found some time to wrestle with tmux's string escaping -- here is a more concise solution that is a drop in for the is_vim variable that robustly accounts for subshells. Rather than making the bash script listed above, you can replace the is_vim function with the following directly:

is_vim="children=(); i=0; pids=( $(ps -o pid= -t '#{pane_tty}') ); \
while read -r c p; do [[ -n c && c -ne p && p -ne 0 ]] && children[p]+=\" $\{c\}\"; done <<< \"$(ps -Ao pid=,ppid=)\"; \
while (( $\{#pids[@]\} > i )); do pid=$\{pids[i++]\}; pids+=( $\{children[pid]-\} ); done; \
ps -o state=,comm= -p \"$\{pids[@]\}\" | grep -iqE '^[^TXZ ]+ +(\\S+\\/)?g?(view|n?vim?x?)(diff)?$'"

It does the same thing as the shell script above - a quick explanation line by line:

Initialize a list, pids, of processes running in the current pane_tty
Construct a tree, children, that maps parent to child pids
Loop through descendents of pids running in pane_tty (some of which may be running in descendant ttys)
Apply normal is_vim function across all descendant processes + ttys

For those using this workaround, I'm hoping this approach is more easily adaptable to future updates to the vim-tmux-navigator logic if support for nested ttys is not merged upstream here. For example, if you are encountering the freezing described in #299, this approach can be modified in a similar way as #300 to not use the -t flag of ps in line 1:

is_vim="children=(); i=0; pids=( $(ps -o pid=,tty= | grep -iE '#{s|/dev/||:pane_tty}' | awk '\{print $1\}') ); \
while read -r c p; do [[ -n c && c -ne p && p -ne 0 ]] && children[p]+=\" $\{c\}\"; done <<< \"$(ps -Ao pid=,ppid=)\"; \
while (( $\{#pids[@]\} > i )); do pid=$\{pids[i++]\}; pids+=( $\{children[pid]-\} ); done; \
ps -o state=,comm= -p \"$\{pids[@]\}\" | grep -iqE '^[^TXZ ]+ +(\\S+\\/)?g?(view|n?vim?x?)(diff)?$'"

Answer 6 · 2022-02-04T18:33:25.000Z

@christoomey - would you be open to merging this upstream? A lot of our users at Fig would like this to be fixed!

Answer 7 · 2022-02-12T15:51:44.000Z

Hi @brendanfalk - apologies for the radio silence here. I've been mulling it over and am hesitant to make a change of this magnitude to the core functionality.

That said, since this change lives in the is_vim check it's much easier for a user to manage themselves (as opposed to adding some configuring deep within the vim plugin side of things). With that in mind, I'd be open to a PR that adds a section to the Troubleshooting section describing the alternative is_vim expression.

Answer 8 · 2022-02-14T20:26:39.000Z

@christoomey thanks for getting back!

Totally understand the concerns about pushing changes that could affect all users. Would you be open to having a chat with me + Sean about the implementation? We've triple checked it and really do think it is just as performant and stable as before, but gives the additional functionality we need to get Fig working.

What happens is people who have vim-tmux-navigator download Fig and it just doesn't work... And most of the time they don't realise it's a conflict between our tools, rather, just assume Fig doesn't work at all. So even if we were in the your troubleshooting documentation, unfortunately I just don't think users would even think to look there.

Open to discussing some other methods like setting an environment variable that makes the change. Or we'd be happy to write a bunch of tests!

Answer 9 · 2022-02-26T08:03:48.000Z

UPDATE

I found some time to wrestle with tmux's string escaping -- here is a more concise solution that is a drop in for the is_vim variable that robustly accounts for subshells. Rather than making the bash script listed above, you can replace the is_vim function with the following directly:
is_vim="children=(); i=0; pids=( $(ps -o pid= -t '#{pane_tty}') ); \
while read -r c p; do [[ -n c && c -ne p && p -ne 0 ]] && children[p]+=\" $\{c\}\"; done <<< \"$(ps -Ao pid=,ppid=)\"; \
while (( $\{#pids[@]\} > i )); do pid=$\{pids[i++]\}; pids+=( $\{children[pid]-\} ); done; \
ps -o state=,comm= -p \"$\{pids[@]\}\" | grep -iqE '^[^TXZ ]+ +(\\S+\\/)?g?(view|n?vim?x?)(diff)?$'"

I can't get this to work in tmux 3.2a on Ubuntu 22.04. I'm not sure how to debug it? I think I also ran into an intermittent bug with the old is_vim.sh (I was getting errors from ps about malformed PIDs in the list, and they seemed to have whitespace in them?). For now, I've got a modified version of this inline script working as is_vim.sh:

#!/usr/bin/env bash
# Usage: is_vim.sh <tty>
#   tty: Specify tty to check for vim process.
tty=$1

# Construct process tree.
children=();
pids=( $(ps -o pid= -t $tty) )

while read -r pid ppid
do
  [[ -n pid && pid -ne ppid ]] && children[ppid]+=" $pid"
done <<< "$(ps -Ao pid=,ppid=)"

# Get all descendant pids of processes in $tty with BFS
idx=0
while (( ${#pids[@]} > idx ))
do
  pid=${pids[idx++]}
  pids+=( ${children[pid]-} )
done

# Check whether any child pids are vim
ps -o state=,comm= -p "${pids[@]}" | grep -iqE '^[^TXZ ]+ +(\\S+\\/)?g?(view|n?vim?x?)(diff)?$'
exit $?

Answer 10 · 2022-05-11T09:44:14.000Z

Here's another approach based on #201

Add this to plugin/tmux_navigator.vim

function! s:set_is_vim()
  call s:TmuxCommand("set-option -p @is_vim yes")
endfunction

function! s:unset_is_vim()
  call s:TmuxCommand("set-option -p -u @is_vim")
endfunction

augroup tmux_navigator_is_vim
  au!
  autocmd VimEnter * call s:set_is_vim()
  autocmd VimLeave * call s:unset_is_vim()
  if exists('##VimSuspend')
    autocmd VimSuspend * call s:unset_is_vim()
    autocmd VimResume * call s:set_is_vim()
  endif
augroup END

Add this to tmux.conf

bind -n C-h if-shell -F "#{@is_vim}" "send-keys C-h"  "select-pane -L"
bind -n C-j if-shell -F "#{@is_vim}" "send-keys C-j"  "select-pane -D"
bind -n C-k if-shell -F "#{@is_vim}" "send-keys C-k"  "select-pane -U"
bind -n C-l if-shell -F "#{@is_vim}" "send-keys C-l"  "select-pane -R"
bind -n C-\\ if-shell -F "#{@is_vim}" "send-keys C-\\" "select-pane -l"

The key advantages are:

it's portable as it doesn't depend on specific details of bash/ps on a particular system
it is really fast as it doesn't spawn a subshell or run any commands

Answer 11 · 2022-07-22T05:33:33.000Z

Here's another approach based on #201

function! s:set_is_vim()
  call s:TmuxCommand("set-option -p @is_vim yes")
endfunction

function! s:unset_is_vim()
  call s:TmuxCommand("set-option -p -u @is_vim")
endfunction

I think this is potentially racy if you move from one tmux split to another and have vim open in both?

Answer 12 · 2022-08-14T13:41:35.000Z

@brendanator thanks for your approach - works well for me. For the bindings, the C-\ binding needs to be in quotes, otherwise tmux raises a Syntax Error.

For context, <C-\><C-n> is one way to switch from a vim terminal back to vim - for example when using something like vim-floaterm.

Here's a related issue and a proposed solution / workaroud on the tmux repo for the unquoted key binding: tmux/tmux#1827 (comment)

Here are the quoted key bindings I'm using, which are working nicely:

bind -n 'C-h' if-shell -F "#{@is_vim}" "send-keys C-h"  "select-pane -L"
bind -n 'C-j' if-shell -F "#{@is_vim}" "send-keys C-j"  "select-pane -D"
bind -n 'C-k' if-shell -F "#{@is_vim}" "send-keys C-k"  "select-pane -U"
bind -n 'C-l' if-shell -F "#{@is_vim}" "send-keys C-l"  "select-pane -R"
bind -n 'C-\' if-shell -F "#{@is_vim}" "send-keys 'C-\\'" "select-pane -l"

Answer 13 · 2023-01-03T16:13:40.000Z

If there is someone out there who also needs to have support for fzf the only change you need to do is on the regex. Just add an extra |fzf at the end:

# vim/fzf tmux integration
# https://github.com/christoomey/vim-tmux-navigator/issues/295#issuecomment-1021591011
is_vim_or_fzf="children=(); i=0; pids=( $(ps -o pid= -t '#{pane_tty}') ); \
while read -r c p; do [[ -n c && c -ne p && p -ne 0 ]] && children[p]+=\" $\{c\}\"; done <<< \"$(ps -Ao pid=,ppid=)\"; \
while (( $\{#pids[@]\} > i )); do pid=$\{pids[i++]\}; pids+=( $\{children[pid]-\} ); done; \
ps -o state=,comm= -p \"$\{pids[@]\}\" | grep -iqE '^[^TXZ ]+ +(\\S+\\/)?g?(view|n?vim?x?|fzf)(diff)?$'"

bind -n C-h run "($is_vim_or_fzf && tmux send-keys C-h) || tmux select-pane -L"
bind -n C-j run "($is_vim_or_fzf && tmux send-keys C-j) || tmux select-pane -D"
bind -n C-k run "($is_vim_or_fzf && tmux send-keys C-k) || tmux select-pane -U"
bind -n C-l run "($is_vim_or_fzf && tmux send-keys C-l) || tmux select-pane -R"

Answer 14 · 2023-02-14T18:08:41.000Z

Hey, thanks a bunch for the solution! I just implemented it and it largely fix vim split navigation. The only issue I'm seeing is that if I split nvim horizontally, I can move down a split but not back up.