[fish] Allow setting multi-select and list reload for history

[bitraid]

The purpose of this PR is twofold:

• Allow users to extend the history widget functionality, so that it can delete selected items and reload the list, with something like:

set -gx FZF_CTRL_R_OPTS '--multi --bind=\'ctrl-x:execute(eval history delete -- (string escape -- {+2..} | string replace -a "\n\t" "\n"))+reload(eval $FZF_DEFAULT_COMMAND)\''

• Make the loading of history items non-blocking.

[bitraid]

The tests fail because the -f switch of set didn't exist in older versions (I forgot about that). I'll fix it tomorrow.

[bitraid]

There seems to be some other issues, too (also with <CTRL-T> ???). I'll have to have closer look.

[junegunn]

Thank you. I'm not a fish user, so I can't really say, but if you think there's no point in supporting older versions of fish, because it makes the code overly complex and few people actually use such old versions, let me know.

[bitraid]

I just figured out why <CTRL-T> was failing. In the last commit I tried to fix a bug (which I forgot to document) where if an environment variable called result existed, the output of fzf would include the contents of the variable, and the results of fzf would be added to the variable (in the script the variable was not explicitly set as local). I now have a proper fix, but I still have to find the issue with <CTRL-R> tests, which probably doesn't have to do with the compatibility changes.

[bitraid]

Apparently, using $FZF_DEFAULT_COMMAND for history causes the tests to fail.
I also have some issues running the tests locally, which fail for fish, even without any changes. This is strange, because the last time I ran them a while back, I didn't have this issue. I'll try to resolve it and figure out what's wrong.

[junegunn]

Apparently, using $FZF_DEFAULT_COMMAND for history causes the tests to fail.

Maybe it's because $SHELL is not set to fish on the test environment? Would adding --with-shell "fish -c" help?

[bitraid]

Apparently, using $FZF_DEFAULT_COMMAND for history causes the tests to fail.

Maybe it's because $SHELL is not set to fish on the test environment? Would adding --with-shell "fish -c" help?

I originally (before the forced push of only the first commit) had a check (in the second commit) that added this parameter when $SHELL wasn't fish, but I forgot about it, and now I realize that it was also missing the -c switch.

What is really confusing though, is that even with that correction, running the tests locally, they inconsistently fail most of the time with:

  1) Failure:
TestFish#test_ctrl_r_multiline [test/test_go.rb:3877]:
Expected false to be truthy.

I don't know why this happens, or why it passes some times. I must look more into it.

[junegunn]

A couple observations.

The test (ruby test/test_go.rb --name test_ctrl_r_multiline) never passes without --with-shell option on my system. But even if I add it, it works only sometimes, as you also noticed.

diff --git a/shell/key-bindings.fish b/shell/key-bindings.fish
index 9039348..7f0931e 100644
--- a/shell/key-bindings.fish
+++ b/shell/key-bindings.fish
@@ -84,7 +84,7 @@ function fzf_key_bindings
             'string join0 -- $i\t(string replace -a -- \n \n\t $h[$i] | string collect);' \
             'end'
         end
-        eval (__fzfcmd) --read0 --print0 -q '(commandline)' | string replace -r '^\d*\t' '' | read -lz result
+        eval (__fzfcmd) --with-shell 'fish\ -c' --read0 --print0 -q '(commandline)' | string replace -r '^\d*\t' '' | read -lz result
         and commandline -- $result
       else
         builtin history | eval (__fzfcmd) -q '(commandline)' | read -l result

So I tried to see what's really going on by binding a reload action to re-evaluate $FZF_DEFAULT_COMMAND, and I noticed when I press 'space' myself on the tmux pane, the list correctly populates and the test passes. So it seems like a fish issue to me.

diff --git a/shell/key-bindings.fish b/shell/key-bindings.fish
index 9039348..f0fb45f 100644
--- a/shell/key-bindings.fish
+++ b/shell/key-bindings.fish
@@ -84,7 +84,7 @@ function fzf_key_bindings
             'string join0 -- $i\t(string replace -a -- \n \n\t $h[$i] | string collect);' \
             'end'
         end
-        eval (__fzfcmd) --read0 --print0 -q '(commandline)' | string replace -r '^\d*\t' '' | read -lz result
+        fzf --with-shell 'fish -c' --bind 'space:reload:eval $FZF_DEFAULT_COMMAND' --read0 --print0 -q (commandline) | string replace -r '^\d*\t' '' | read -lz result
         and commandline -- $result
       else
         builtin history | eval (__fzfcmd) -q '(commandline)' | read -l result

[bitraid]

I don't like this workaround - closing.

[bitraid]

I reopened this because upon closer testing 0a10d14, I realized that it was still blocking.
As it turns out, even though these command are non-blocking:

set -l cmd fzf
begin echo start; sleep 3; echo end; end | $cmd
$cmd | read -l result

This becomes blocking:

begin echo start; sleep 3; echo end; end | fzf | read -l result

Upon further testing, I figured that this is non-blocking:

set -l result (begin echo start; sleep 3; echo end; end | fzf)

So I adjusted this PR with those changes, and everything is working fine locally, with the test consistently passing.
I also removed the support for fish versions older that 3.0b1 to simplify the code (the script was using set -a which was not supported by those versions anyway).

But the tests here now fail with:

  1) Failure:
TestFish#test_ctrl_r_abort [test/test_go.rb:3929]:
Expected: "> foo"
  Actual: "foo"

Any idea why this might be happening?

[bitraid]

I even ran the tests a few times on a VM with ubuntu-24.04.1 (same as GitHub actions): They always pass...

[junegunn]

I reran the task, and the test passed this time. And it constantly passes on my system. I'll see if there can be some timing issues, but anyway I think this is ready for merge. Thanks.