andreyorst / fzf.kak

FZF for Kakoune
MIT License
143 stars 33 forks source link

FZF tmux split is closed immediately with fish shell #49

Closed krobelus closed 5 years ago

krobelus commented 5 years ago

Hi, thanks for all the work on this!

Problem description

Since commit 9b07bddfdcd576614e32dfa41cb91ad8f68e66c8, the fzf window pops up and vanishes instantly. The temporary fzfcmd script requires some posix features that fish doesn't implement. Tmux tries to execute it in the default shell, which fails with fish. The patch below fixes this by explicitly calling sh. It will also not spawn fish at all in my example, because tmux doesn't call the shell if the command consists of multiple words.

Steps to reproduce

  1. Put set -g default-shell "/usr/bin/fish" in ~/.tmux.conf
  2. Start kak
  3. Source fzf.kak
  4. Type :fzf<ret>

Environment information

Kakoune v2019.01.20 fzf 0.17.5 tmux 2.8 fish 3.0.2

Patch

diff --git a/rc/fzf.kak b/rc/fzf.kak
index 4076143..f78c4ff 100644
--- a/rc/fzf.kak
+++ b/rc/fzf.kak
@@ -176,7 +176,7 @@ fzf -params .. %{ evaluate-commands %sh{
         # if height contains `%' then `-p' will be used
         [ -n "${tmux_height%%*%}" ] && measure="-l" || measure="-p"
         # `terminal' doesn't support any kind of width and height parameters, so tmux panes are created by tmux itself
-        cmd="nop %sh{ command tmux split-window ${measure} ${tmux_height%%%*} '${fzfcmd}' }"
+        cmd="nop %sh{ command tmux split-window ${measure} ${tmux_height%%%*} sh '${fzfcmd}' }"
     else
         cmd="terminal %{${fzfcmd}}"
     fi
andreyorst commented 5 years ago

Hm, I see. Can you try fix in fix-fish branch?

krobelus commented 5 years ago

On Sun, Mar 31, 2019 at 12:40:07AM -0700, Andrey Orst wrote:

Hm, I see. Can you try fix in fix-fish branch?

Yep, that fix works as well!

However I just realized that there is another issue not fixed by either of those changes: the preview window does not show the contents of a file, but an error message from fish instead.

"fish: Command substitutions not allowed"

This is probably because of the subshell in preview_cmd

(${highlight_cmd} || cat {})

because () in fish is equivalent to $() in sh.

So we should also call sh explicitly here

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/andreyorst/fzf.kak/issues/49#issuecomment-478319517

andreyorst commented 5 years ago

oh well, I know how to fix this, and I've thought that using shebang would work for that too. I'll fix it ASAP

On Sun, Mar 31, 2019, 11:57 Johannes Altmanninger notifications@github.com wrote:

On Sun, Mar 31, 2019 at 12:40:07AM -0700, Andrey Orst wrote:

Hm, I see. Can you try fix in fix-fish branch?

Yep, that fix works as well!

However I just realized that there is another issue not fixed by either of those changes: the preview window does not show the contents of a file, but an error message from fish instead.

"fish: Command substitutions not allowed"

This is probably because of the subshell in preview_cmd

(${highlight_cmd} || cat {})

because () in fish is equivalent to $() in sh.

So we should also call sh explicitly here

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/andreyorst/fzf.kak/issues/49#issuecomment-478319517

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/andreyorst/fzf.kak/issues/49#issuecomment-478323984, or mute the thread https://github.com/notifications/unsubscribe-auth/ASkXT-OueV9f3mCuhHfOzPEN-W5Rs50Xks5vcHhygaJpZM4cT-mS .

andreyorst commented 5 years ago

@krobelus can you try with last commit? Seems working to me

krobelus commented 5 years ago

On Sun, Mar 31, 2019 at 02:59:15AM -0700, Andrey Orst wrote:

@krobelus can you try with last commit? Seems working to me

Yes, that works as expected. Thank you!

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/andreyorst/fzf.kak/issues/49#issuecomment-478327803