andreyorst / fzf.kak

FZF for Kakoune
MIT License
143 stars 33 forks source link

[Feature Request] Allow opening in terminal-tab instead of terminal #63

Closed aecepoglu closed 5 years ago

aecepoglu commented 5 years ago

It'd be nice if fzf.kak could open windows using the command terminal-tab instead of terminal.

A solution for this could be allowing the user to enter what command to use to open the new window. This new feature could replace the existing conditional code for tmux/x11 separation, and depend entirely on user configuration.

andreyorst commented 5 years ago

Sorry, I don't understand. Currently plugin automatically decides what to use based on your environment. If you're using TMUX it will create new tab/split accordingly to your keypresses by using TMUX natively or by uisng tmux-terminal commands. If you're not using TMUX, I only support creating new window with terminal because kakoune doesn't support arguments to the terminal command. Ideally I would like something like this: https://github.com/mawww/kakoune/issues/2814

aecepoglu commented 5 years ago

I'm asking why not allow the user to choose to have fzf.kak use "terminal-tab" command instead of "terminal" command.

andreyorst commented 5 years ago

I understood, but I think that that should be done by Kakoune. Current mechanism is smart enough to detect TMUX environment, and otherwise it uses terminal alias that should handle all cases related to X11 and various terminal emulators. I believe that we should instead implement parameters to terminal so it could accept tab parameter or horizontal and vertical so I could provide full support for this features baked up by Kakoune itself. This will be a proper fix, and fzf.kak will automatically support everything that provide such alias, an have a proper error handling if it isn't.

Also, another way can be to add an keybinding to call terminal-tab explicitly, but unfortunately it may not be available (as this alias only available for Kitty, which is strange since similar command exists for iTerm: iterm-terminal-tab, but iterm.kak doesn't provide such alias for some reason) and there's no test mechanism to handle that, except of failing to catch block.

I don't mind if you provide pull request that adds the configurable feature for X11 on top of existing behavior or additional keybinding. Unfortunately I have little spare time right now and can't work on this, testing this in different terminals can take lot of time. So I would appreciate a PR.

aecepoglu commented 5 years ago

Okay, I am using a patched version of kak.fzf atm but of course it is not a real fix. I will (like you, when I find the time) will look into this.

andreyorst commented 5 years ago

I think I'll visit this problem sooner that I've expected, because new release of Kakoune deprecated some commands that I've used as fallback one in the past. So I will do a cleanup and possible will work on a patch for Kakoune terminal command too.

andreyorst commented 5 years ago

Another solution for you instead of patching the plugin add this to your configuration:

define-command -override -hidden fzf-window -params .. %{
    evaluate-commands %{
        terminal-tab kak -c %val{session} -e "%arg{@}"
    }
}

Basically just override the command in user config.

If you're using plug.kak this should go to the defer configurattion like this:

plug "andreyorst/fzf.kak" config %{
    map -docstring 'fzf mode' global normal '<c-p>' ': fzf-mode<ret>'
} defer fzf %{
    define-command -override -hidden fzf-window -params .. %{
        evaluate-commands %{
            terminal-tab kak -c %val{session} -e "%arg{@}"
        }
    }
    # rest of config
}

You can keep the logic of original command in case you plan to use it in TMUX though.

andreyorst commented 5 years ago

You also can test configuration option fzf_terminal_cmd on the v1.1.0-dev branch

aecepoglu commented 5 years ago

Tested v1.1.0-dev. Works with defer "fzf" %{ set-option global fzf_terminal_cmd ... }

I couldn't make the override work with defer "fzf" though.

Also, I just wanted to remark that there are some minor irregularities in option naming: Some options (fzf_file_command) end in the word "command", and the others (fzf_terminal_cmd) end in "cmd".

The issue is resolved as far as I'm concerned

andreyorst commented 5 years ago

Thanks for the suggestion

aecepoglu commented 5 years ago

You're welcome and you're very responsive, it's great!