junegunn / fzf

:cherry_blossom: A command-line fuzzy finder
https://junegunn.github.io/fzf/
MIT License
63.4k stars 2.36k forks source link

Issue with nested `fzf` usage in bash script causing script to halt #3685

Closed LangLangBart closed 5 months ago

LangLangBart commented 5 months ago

Info

Problem / Steps to reproduce

I am encountering an issue when trying to use fzf within fzf in a bash script. The script stops when I make a second selection from ⌃ Control + H, and continues only when ⌃ Control + C is pressed.

The idea I am pursuing is using the --history flag read from my past queries and make a selection, this selection will then replace my current query. For simplicity, the script has been reduced to the following:

#!/usr/bin/env bash
set -o errexit -o nounset -o pipefail

export FZF_DEFAULT_OPTS=""
export FZF_DEFAULT_COMMAND=""
SHELL=$(which bash)

curl_custom() {
    command curl --request POST "localhost:$FZF_PORT" --silent --data "$@"
}

query() {
    for i in {1..100}; do
        curl_custom "change-header:$i"
    done
}

view_history_commands() {
    command fzf --no-multi <<<$'A\nB' | while read -r item; do
        curl_custom "change-query:$item"
    done
}

export -f curl_custom query view_history_commands

: | command fzf \
    --bind "change:reload:query {q}" \
    --bind 'ctrl-h:execute:view_history_commands' \
    --listen

Here is the GIF of the script in action:

  1. First, I press ⌃ Control + H and select A.
  2. While the query is still running, I press ⌃ Control + H again and select B.
  3. The script is blocked and only continues when I press ⌃ Control + C.

Alternatives

The idea worked when modifying the script using the --expect flag.


Related issue(s):

LangLangBart commented 5 months ago

Adopting a stop_loop check appears to be effective. Would you recommend that?

#!/usr/bin/env bash
set -o errexit -o nounset -o pipefail

export FZF_DEFAULT_OPTS=""
export FZF_DEFAULT_COMMAND=""
SHELL=$(which bash)

export stop_loop="/tmp/stop_loop"
trap 'rm -rf $stop_loop' EXIT

curl_custom() {
    command curl --request POST "localhost:$FZF_PORT" --silent --data "$@"
}

query() {
    for i in {1..100}; do
        [[ -s $stop_loop ]] && break
        curl_custom "change-header:$i"
    done
}

view_history_commands() {
    echo "stop" >"$stop_loop"
    item=$(command fzf --no-multi <<<$'A\nB')
    curl_custom "change-query:$item"
    : >"$stop_loop"
}

export -f curl_custom query view_history_commands

: | command fzf \
    --bind "change:reload:query {q}" \
    --bind 'ctrl-h:execute:view_history_commands' \
    --listen
junegunn commented 5 months ago

Interesting. The reason that the curl command in view_history_commands doesn't finish is that fzf uses a bounded buffer for server requests.

https://github.com/junegunn/fzf/blob/25e61056b6394d251dc73b85e07a70f5813d081d/src/terminal.go#L779

This small buffer is already filled with change-header requests, and you can't push more into it until fzf starts processing the buffered ones, but fzf can't process more until execute stops, so everything halts.

There is no particular reason the buffer size should be 10, I just thought it was enough. We can increase it to a larger value to prevent this from happening. But how large should it be? 1k? 10k? 100k? Can you share what you were trying to implement?

LangLangBart commented 5 months ago

The reason that the curl command in view_history_commands doesn't finish is that fzf uses a bounded buffer for server requests.

Thank you, increasing the buffer size does work.

Can you share what you were trying to implement?

The script is designed to search GitHub code and list the results. The header updates to reflect the progress of file retrieval.

The hotkey ⌃ Control + H allows viewing of past queries and select one.

Refer to the GIF below for further clarity.

The code is present here. For the time being, I have implemented the stop_loop workaround.

interactive history by LangLangBart · Pull Request #2 · LangLangBart/gh-find-code · GitHub

junegunn commented 5 months ago

I'm thinking of increasing the buffer size to 1K. Do you think this will be enough? We can't make it absurdly large because that would waste memory.

junegunn commented 5 months ago

I'm also going to put a timeout on POST requests. So with the channel size 10,

$ fzf --listen --bind 'start:execute(for i in $(seq 1000); do curl -if localhost:$FZF_PORT -d up || break; done)+abort'
HTTP/1.1 200 OK

HTTP/1.1 200 OK

HTTP/1.1 200 OK

HTTP/1.1 200 OK

HTTP/1.1 200 OK

HTTP/1.1 200 OK

HTTP/1.1 200 OK

HTTP/1.1 200 OK

HTTP/1.1 200 OK

HTTP/1.1 200 OK

HTTP/1.1 503 Service Unavailable

curl: (22) The requested URL returned error: 503
LangLangBart commented 5 months ago

Do you think this will be enough?

Indeed, tests with 100 as the parameter (serverInputChan in src/terminal.go) were successful on the script in the issue description. Therefore, 1k should be more than adequate.

I'm also going to put a timeout on POST requests.

A timeout feature would indeed be a valuable addition.

junegunn commented 5 months ago

Therefore, 1k should be more than adequate.

Alright, let's settle with 100 with 2-second timeout. 1K does seem a bit too much to be honest. We'll see if anyone hits the limit.