joshmedeski / sesh

Smart session manager for the terminal
MIT License
410 stars 16 forks source link

Panic when connecting to the result of gum filter #114

Closed kevinrobayna closed 1 month ago

kevinrobayna commented 1 month ago

What happened?

When I execute:

sesh connect $(
        sesh list -i | gum filter --limit 1 --placeholder 'Pick a session' --height 60 --prompt='⚡'
)

within a tmux session. I expect the same behaviour as when I run it as part of alacritty/tmux keybinding but I get a panic saying out of bound exception

Version

sesh version 1.1.1

Relevant log output

sesh connect $( sesh list -i | gum filter --limit 1 --placeholder 'Pick a session' --height 60 --prompt='⚡' ) panic: runtime error: slice bounds out of range [4:3]

goroutine 1 [running]: github.com/joshmedeski/sesh/connect.Connect({0x16ced2f5d, 0x3}, 0x7?, {0x0, 0x0}, 0x0?) /home/runner/work/sesh/sesh/connect/connect.go:21 +0x2f4 github.com/joshmedeski/sesh/seshcli.App.Connect.func1(0x140000fd958?) /home/runner/work/sesh/sesh/cmds/connect.go:38 +0x14c github.com/urfave/cli/v2.(Command).Run(0x14000130160, 0x140000aa480, {0x1400009e6c0, 0x3, 0x3}) /home/runner/go/pkg/mod/github.com/urfave/cli/v2@v2.27.1/command.go:279 +0x754 github.com/urfave/cli/v2.(Command).Run(0x14000130580, 0x140000aa340, {0x140000aa040, 0x4, 0x4}) /home/runner/go/pkg/mod/github.com/urfave/cli/v2@v2.27.1/command.go:272 +0x964 github.com/urfave/cli/v2.(App).RunContext(0x1400012c000, {0x1031612d8?, 0x1032ddac0}, {0x140000aa040, 0x4, 0x4}) /home/runner/go/pkg/mod/github.com/urfave/cli/v2@v2.27.1/app.go:337 +0x534 github.com/urfave/cli/v2.(App).Run(0x1030b7f61?, {0x140000aa040?, 0x0?, 0x0?}) /home/runner/go/pkg/mod/github.com/urfave/cli/v2@v2.27.1/app.go:311 +0x3c main.main() /home/runner/work/sesh/sesh/main.go:14 +0xb0

Reviewed

joshmedeski commented 1 month ago

Can you show me your tmux keybinding specifically?

kevinrobayna commented 1 month ago

Can you show me your tmux keybinding specifically?

I was not running it as part of a tmux keybinding but as part of a terminal script

When I use it as part of the keybinding through tmux it works https://github.com/kevinrobayna/dotfiles/blob/master/config/.config/tmux/conf/keybind.conf#L59-L61

PookieBuns commented 1 month ago

This is due to sesh list -i pre-pending the nerd font icon in front of the path, causing sesh connect to try to look for a path with an icon in front of it. If you remove -i in your command, the command will work fine.

within tmux for some reason it does not have problems with the prepended icon as I saw this behavior as well (not exactly sure why, theoretically it should still panic since the command passed to sesh connect still contains the icon).

If you want to fix this, you need to strip the icon after your picker selects the corresponding item. My extension fzf-sesh deals with this by calling

_fzf_sesh_extract_path_from_icon_path() {
    echo $1 | awk '{if ($2 != "") print $2; else print}'
}

to strip the icon from the selected item. You could do something similar if you use gum for your picker as well.

kevinrobayna commented 1 month ago

That does make sense it's odd about it not happening when calling it through tmux but I guess the change needed is here

https://github.com/joshmedeski/sesh/blob/72f844199fef95b8cce388bcac795d933ae09469/cmds/connect.go#L31

we need to split the session name and pick the last part or something like that. I can raise a quick fix for this

kevinrobayna commented 1 month ago

actually that's what in theory we are doing after 🤔

func Connect(
    choice string,
    alwaysSwitch bool,
    command string,
    config *config.Config,
) error {
    if strings.HasPrefix(choice, icons.TmuxIcon) || strings.HasPrefix(choice, icons.ZoxideIcon) || strings.HasPrefix(choice, icons.ConfigIcon) {
        choice = choice[4:]
    }
kevinrobayna commented 1 month ago

so the reason it's failing is because some shells sends the response in a funny way

sesh via  v1.22.3
 go run github.com/joshmedeski/sesh connect  frontier

sesh via  v1.22.3
 go run github.com/joshmedeski/sesh connect " frontier"

with the current version the first way it does not work because the icon and the session name are different args but in the second one it's only one and the strings.HasPrefix can catch it.

kevinrobayna commented 1 month ago

Just raised a PR for this, if you like the solution I can add some tests!

https://github.com/joshmedeski/sesh/pull/116

joshmedeski commented 1 month ago

Cool, I'll look over the PR.

I recommend wrapping commands in quotes so it's one argument. There are examples in the README for wrapping quotes from a tmux command.

PookieBuns commented 1 month ago

Nice catch. I see what's going on now.

sesh connect "icon_placeholder frontier" passes the entire string "icon_placeholder frontier" as one argument to sesh connect so it handles the strip properly

sesh connect icon_placeholder frontier gets parsed as 2 arguments so the code panics when trying to strip the first argument (which is the icon)`

I guess the proper way to call it in the CLI is using

sesh connect "$(
sesh list -i | gum filter --limit 1 --placeholder 'Pick a session' --height 60 --prompt='⚡'
)"
joshmedeski commented 1 month ago

Yep, we may change the behavior soon but for now connect command expects one argument, so wrapping the sesh list in quotes is best.

Feel free to watch the releases and I'll be sure to document changed behavior. For now, I'm going to consider this resolved.