Skip to content
This repository has been archived by the owner on Oct 23, 2022. It is now read-only.

fzf window closes immediatly on x11 #15

Closed
TeddyDD opened this issue Oct 12, 2018 · 31 comments
Closed

fzf window closes immediatly on x11 #15

TeddyDD opened this issue Oct 12, 2018 · 31 comments
Labels
compatibility OS or environment compability issues

Comments

@TeddyDD
Copy link

TeddyDD commented Oct 12, 2018

Problem description

Running any file related command from fzf mode results in new terminal window blinking for a split of a second. It closes immediately after that. Info box stays open. I did bisect on changes since v0.1.2 and first bad commit is 8ae03b1

Buffer selector, change directory and buffer search works fine.

Steps to reproduce

  1. git checkout 8ae03b1
  2. run kak outide of tmux
  3. run any fzf-mode command

Environment information

Kakoune version: v2018.09.04
OS version: Void 4.18.13_1 x86_64 GenuineIntel uptodate rFFFFFFFF
sh executable version: dash-0.5.10.2
fzf version: 0.17.5

Edit:

It seems that fzf_preview was true by default, disabling it fixes the problem.

Okay, I don't have any compatible highlighter installed so it fails because fzf_preview is true by default.

@andreyorst
Copy link
Owner

Hm. That's weird, because I shouldn't fail if no highlighter is installed. Because it fallbacks to cat. I have no highlighter in my Termux instance, and it works fine there, though inside tmux. I'll investigate it

@andreyorst
Copy link
Owner

@TeddyDD does fzf-buffer work? It uses different implementation. If It isn't working too, there's nothing to do with preview. Also please check fzf-cd

@TeddyDD
Copy link
Author

TeddyDD commented Oct 12, 2018

buffer and cd works. I checked if this might be terminal related but both kitty and xst gives me the same result

@andreyorst
Copy link
Owner

I've found the issue. Please try with latest master

@andreyorst
Copy link
Owner

No. Actually not. The problem is that the size calculation isn't happening.

@andreyorst
Copy link
Owner

I've pushed modified version, in which $pos variable contains default value. Please try to launch fzf with it

@andreyorst
Copy link
Owner

I've pushed fix_preview branch, can you please try it? It works in my setup, I even symlinked sh to dash. Though It worked for me whole time, so my configuration can't be referenced.

@TeddyDD
Copy link
Author

TeddyDD commented Oct 12, 2018

It doesn't close terminal window so you probably fixed it :)

Problem is apparently fzf uses $SHELL in preview command so preview it's broken on Fish shell. I fixed it with this patch

diff --git i/rc/fzf.kak w/rc/fzf.kak
index a038c9..ec160c 100644
--- i/rc/fzf.kak
+++ w/rc/fzf.kak
@@ -334,7 +334,7 @@ define-command -hidden fzf -params 2..3 %{ evaluate-commands %sh{
     elif [ ! -z "${kak_opt_termcmd}" ]; then
         path=$(pwd)
         additional_flags=$(echo $additional_flags | sed "s:\$pos:\\\\\$pos:")
-        cmd="$kak_opt_termcmd \"sh -c \\\"cd $path && $preview_pos $items_command | fzf --expect ctrl-q $additional_flags > $tmp\\\"\""
+        cmd="$kak_opt_termcmd \"sh -c \\\"SHELL=/bin/sh; export SHELL; cd $path && $preview_pos $items_command | fzf --expect ctrl-q $additional_flags > $tmp\\\"\""
     else
         echo "fail termcmd option is not set"
         exit

Not sure if this patch is correct way to do this, but after applying it, it works for me.

@andreyorst
Copy link
Owner

/bin/sh is not always there. For example in Termux there's no /bin/sh

@andreyorst
Copy link
Owner

@TeddyDD can you please try the latest fix_preview branch?

@TeddyDD
Copy link
Author

TeddyDD commented Oct 12, 2018

It doesn't work, terminal shows up for split of a second and closes.

@andreyorst
Copy link
Owner

Weird. It works for me. I suspect that we have single thing that doesn't work on your setup and works on my. I'll try to use fish

@TeddyDD
Copy link
Author

TeddyDD commented Oct 12, 2018

I tried to run Kakoune with bash and sh on fix_preview branch. I'm getting
invalid preview window layout: for a second and terminal closes

@andreyorst
Copy link
Owner

Thats exactly what I've been fixing all the time, and strangely enough it works for me

@andreyorst
Copy link
Owner

fish doesn't support $(...)... mother of god why...

@TeddyDD
Copy link
Author

TeddyDD commented Oct 12, 2018

yeah, but that shoudn't matter since Kakoune uses posix shell in expansions, right?

@andreyorst
Copy link
Owner

yeah, but I pass shell command to terminal via $termcmd -e, then I pass it to sh -c to actually do the thing. This way it should be more portable, since we always use sh. Now I try to debug this 3 screen line command in fish, and the syntax is wrong in the places where I least expect it to be.

@andreyorst
Copy link
Owner

@TeddyDD with latest changes, still doesn't work?

@TeddyDD
Copy link
Author

TeddyDD commented Oct 12, 2018

um, you haven't pushed anything new to fix_preview

@andreyorst
Copy link
Owner

Oh. Sorry. I didn't noticed that my connection was off. I've pushed the changes

@TeddyDD
Copy link
Author

TeddyDD commented Oct 12, 2018

nope, still blinking terminal

@andreyorst
Copy link
Owner

Can I ask you to try with another login shell? Althoughit works for me in KDE and Mate, with bash, zsh, and fish as a login shells...

@TeddyDD
Copy link
Author

TeddyDD commented Oct 12, 2018

sure, I'll try and report back

@andreyorst
Copy link
Owner

andreyorst commented Oct 12, 2018

I've installed kitty, and now it isn't working for me too. I'm testing this mainly with termite. So I thought that other terminals should work. Turns out that that's not always true. Mate terminal brokes too.

@TeddyDD
Copy link
Author

TeddyDD commented Oct 12, 2018

I tried with bash as login shell, it doesn't help. I tried it under st and kitty

@andreyorst
Copy link
Owner

I've noticed that this doesn't work with kitty, and alacritty when they set as termcmd. If I change it, it works. Try changing termcmd to some common terminal, like gnome terminal? Maybe it is the issue with kitty, as it need some additional argument. Currently, setting termcmd to termite -e gnome-terminal -e and mate-terminal -e makes it work.

@andreyorst
Copy link
Owner

andreyorst commented Oct 12, 2018

I see.
when alacritty installed termcmd equals to alacritty -e sh -c
when kitty installed termcmd equals to kitty sh -c
I use termite, and with it termcmd equals to termite -e

Seems likse -e isn't compatible with sh -c in some ways.

@andreyorst
Copy link
Owner

Ok now I know where to dig. Expect a fix in couple of hours :)

@andreyorst
Copy link
Owner

andreyorst commented Oct 12, 2018

@TeddyDD should work now. I've tested with all possible at the moment terminals

@TeddyDD
Copy link
Author

TeddyDD commented Oct 12, 2018

it works, awesome job!

@andreyorst
Copy link
Owner

awesome job

issue solved exactly in 30 comments.

I'll merge this into master now

@andreyorst andreyorst mentioned this issue Oct 12, 2018
@andreyorst andreyorst added the compatibility OS or environment compability issues label Oct 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compatibility OS or environment compability issues
Projects
None yet
Development

No branches or pull requests

2 participants