soyuka / tmux-current-pane-hostname

Tmux plugin that enables displaying hostname and user of the current pane in your status bar.
MIT License
76 stars 23 forks source link

Stops working when tmux-ssh-split creates a new pane. #14

Open stormsz opened 3 years ago

stormsz commented 3 years ago

The tmux-ssh-split is a plugin that allow us to, when inside an SSH pane, press the split key and it will create a new pane already ssh'ed in. (so we dont have to always ssh server_with@tmux_from_3years_ago).

This plugin seems to break when a new split is created, the hostname will revert back to the hostname where the tmux server is running and not the remote server.

Also I'm not sure if its a problem with this plugin or the ssh-split plugin, but since this is the one that breaks i created the issue here. TMUX version: tmux 3.0a Tested everything on Ubuntu WSL

EDIT: after further testing i have noticed some things:

Since i have created a new pane from the hosting machine and then SSH'ed in and the hostname updated to the hostname of the remote, this leads me to believe that is the tmux-ssh-split messing something. Should i create an issue there instead?

soyuka commented 3 years ago

Hi,

Indeed it's possible that there's something wrong in either of our plugins. I suggest opening an issue there as well. This plugins is a bit hackish and reads the command executed in the pane. It might be that we only need to quick fix the list at :

https://github.com/soyuka/tmux-current-pane-hostname/blob/master/scripts/shared.sh#L98-L103

pschmitt commented 3 years ago

I've commented on this on my repo:

I'm not familiar with this plugin, but after briefly checking out the code I can tell this is definitely not a tmux-ssh-split issue.

tmux-current-pane-hostname does not correctly detect if the ssh command is running in the current pane. It actually only works if ssh is run directly. But tmux-ssh-split does a bit more than just run ssh HOST.

The actual command spawned by tmux-ssh-split would be something like the following:

echo '🧙👉 Running "ssh HOST"'; trap /usr/bin/zsh EXIT INT; ssh HOST

Some pointers as to how to fix this:

  • tmux display-message -p "#{pane_current_command}" (HERE) will only return the name of the leading command (bash or zsh in our case).

  • this needs to updated to something akin to what I've implemented here

pschmitt commented 3 years ago

The linked code is out of date (discovered a bug).

Here's what you want to really do:


ssh_connected() {                                                    
  local child_cmd                                                                                                                                              
  local pane_id                                                                
  local pane_pid                                                               

  pane_id="${1:-$(get_current_pane_id)}"                                       
  pane_pid="$(get_pane_pid_from_pane_id "$pane_id")"                                                                                                           

  if [[ -z "$pane_pid" ]]                                                                                                                                      
  then                                                                         
    echo "Could not determine pane PID" >&2                                    
    return 3                                                                                                                                                   
  fi                                                                           

  ps -o command= -g "${pane_pid}" | while read -r child_cmd                    
  do                                   
    if [[ "$child_cmd" =~ ^(auto)?ssh.* ]]                                     
    then                                                                                                                          
      return 0                                                                                                                                                  
    fi                                                                         
  done                                                                                                                                                         

  return 1
}
soyuka commented 3 years ago

Thanks @pschmitt! @stormsz feel free to open a PR with this patch

pschmitt commented 3 years ago

Yeah. I've done my part. ;)

soyuka commented 3 years ago

@stormsz may you try the patched version ?

Maybe that we could just do:

  # Get current pane command
  local cmd=$(tmux display-message -p "#{pane_current_command}")

  [[ "$cmd" =~ ^(auto)?ssh.* ]]  

I'm not confident using ps as this program is not the same cross-platform nor always installed

pschmitt commented 3 years ago

Maybe that we could just do:

As mentioned above this won't work. You have to use ps for the detection to work reliably. pane_current_command will only give you the "starting command" that's executed in the pane. You need look at the children commands to detect if SSH is running in that pane. If you start a shell in your pane, then ssh pane_current_command will return you the shell command (bash or something like this).

I'm not confident using ps as this program is not the same cross-platform nor always installed

I've never ever encountered a unix-y system where ps is not available by default. Even the alpine docker image has ps. Achieving cross-platform compatibility isn't hard as well - it's a single ps call after all.

pschmitt commented 3 years ago

BTW you are already using ps here: https://github.com/soyuka/tmux-current-pane-hostname/blob/1e2eb70d4345b7b4847eedeef34756b7b9622229/scripts/shared.sh#L61

stormsz commented 3 years ago

Yo guys. Sorry for the delay, borked my system and had to do some restores from backups.

I have implemented the new code change that @pschmitt suggested to the shared.sh file, however this code change seems to break the basic functionality of tmux-current-pane-hostname as it stops updating the hostname even when not using the tmux-ssh-split and just doing a simple ssh ... on a single pane

pschmitt commented 3 years ago

This wasn't meant to be used verbatim. Otherwise I'd have sent a PR.

stormsz commented 3 years ago

Oh, goddamit I'm dumb. let me see what i can do.

kurktchiev commented 3 years ago

Did you ever get this merged in? it would make life somewhat easier when sshing all over the place :)