Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

key-bindings.bash: disable pipefail in Alt+C #3382

Closed
wants to merge 2 commits into from

Conversation

ardnew
Copy link

@ardnew ardnew commented Jul 26, 2023

With set -o pipefail enabled globally, the Alt+C keybinding silently fails without changing directory or indicating an error.

This change temporarily disables the shell option locally within the scope of __fzf_cd__(), as expected by the existing pipelines, and then restores it to the user's original setting prior to invoking Alt+C.

Requires Bash 4.4 at least, which is more than safe to assume here :)

Reference: https://unix.stackexchange.com/a/406332


Proof that this technique works can be demonstrated with:

check(){ echo $*: $(set -o|grep pipefail); }
f() { check at enter f; local -; set +o pipefail; check at exit f; }
check before call f
f
check after call f

@junegunn
Copy link
Owner

junegunn commented Aug 2, 2023

Thanks.

Alt+C keybinding silently fails without changing directory or indicating an error.

If I'm not mistaken, it doesn't work only when an entry is selected while the list is still being populated by the underlying command because eval "$cmd" is interrupted.

dir=$(eval "$cmd" | FZF_DEFAULT_OPTS="$opts" $(__fzfcmd)) && printf 'builtin cd -- %q' "$dir"

It works well if $cmd is complete.

Requires Bash 4.4 at least

Unfortunately, we can't enforce this because macOS ships with bash 3 due to its GPL license and we don't want to stop supporting the default bash version of macOS.

Having said that, an easy fix would be to make $cmd not fail like so:

diff --git a/shell/key-bindings.bash b/shell/key-bindings.bash
index d83f9d3..58bb1db 100644
--- a/shell/key-bindings.bash
+++ b/shell/key-bindings.bash
@@ -45,7 +45,7 @@ __fzf_cd__() {
   cmd="${FZF_ALT_C_COMMAND:-"command find -L . -mindepth 1 \\( -path '*/\\.*' -o -fstype 'sysfs' -o -fstype 'devfs' -o -fstype 'devtmpfs' -o -fstype 'proc' \\) -prune \
     -o -type d -print 2> /dev/null | cut -b3-"}"
   opts="--height ${FZF_TMUX_HEIGHT:-40%} --bind=ctrl-z:ignore --reverse ${FZF_DEFAULT_OPTS-} ${FZF_ALT_C_OPTS-} +m"
-  dir=$(eval "$cmd" | FZF_DEFAULT_OPTS="$opts" $(__fzfcmd)) && printf 'builtin cd -- %q' "$dir"
+  dir=$(eval "$cmd || :" | FZF_DEFAULT_OPTS="$opts" $(__fzfcmd)) && printf 'builtin cd -- %q' "$dir"
 }
 
 __fzf_history__() {

@junegunn junegunn closed this in 1894304 Aug 18, 2023
@junegunn
Copy link
Owner

Hi, I've decided to disable pipefail only inside the command substitutions where it can be a problem. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants