davatorium / rofi

Rofi: A window switcher, application launcher and dmenu replacement
https://davatorium.github.io/rofi/
Other
13.06k stars 608 forks source link

rofi-sensible-terminal passes arguments wrong in ssh launcher #953

Closed exi closed 5 years ago

exi commented 5 years ago

Version

Version: 1.5.2

Configuration

https://gist.github.com/exi/c0a1c5bc44b13937e5b58a44488b6a26

Launch Command

rofi -combi-modi window,drun,ssh -show combi -modi combi

Steps to reproduce

open rofi, access any ssh host, in my example localhost

What behaviour you see

What behaviour you expect to see

My default terminal is terminator.

If i try to use the ssh mode and observe the system calls via strace, I can see that paramters are not passed correctly to my terminal:

execve("/run/current-system/sw/bin/rofi-sensible-terminal", ["rofi-sensible-terminal", "-e", "ssh", "localhost"], 0xeea7f0 /* 80 vars */) = 0
execve("/nix/store/a9iip3aqbc0hhhisn2w68qgkfshnqnvv-terminator-1.91/bin/terminator", ["terminator", "-e", "ssh", "localhost"], 0x2500008 /* 79 vars */) = 0

As you can see, "ssh" and "localhost" are passed as two separate arguments. This cannot work. Instead it should do "terminator -e 'ssh localhost'" so both are arguments for the -e flag.

As expected, the terminal outputs: .terminator-wrapped: error: Additional unexpected arguments found: ['localhost']

DaveDavenport commented 5 years ago

A lot of terminal behave differently.. Regrettable, there is not one commandline that will work for all terminals.

Therefor your can set the used commandline option in Rofi configuration ~screen~file.

exi commented 5 years ago

if "-e", "ssh", "localhost" is already passed to the configured terminal, this is already wrong. i would expect it to pass "ssh", "localhost" because the "-e" already makes assumptions about how terminals handle commands. Is there a way to only pass "ssh", "localhost" as arguments? That way it would be possible to easily pass the correct command line to my terminal.

Also since terminator is already in the magic list of supported terminals, would it not make sense to properly support it by handling it correctly, if necessary with terminator-specific code?

DaveDavenport commented 5 years ago

You can fix the -ssh-command.

{terminal} -e '{ssh-client} {host} [-p {port}]'

I will remove terminator from the list, as it does not support the 'xterm/rxvt' syntax. This is the base terminal available, and I see that as standard.

github-actions[bot] commented 3 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.