junegunn/fzf

_fzf_complete no longer returns 124 under a certain condition as before

zzJinux opened this issue · 3 comments

Checklist

  • I have read through the manual page (man fzf)
  • I have searched through the existing issues

Output of fzf --version

0.48.1 (brew)

OS

  • Linux
  • macOS
  • Windows
  • Etc.

Shell

  • bash
  • zsh
  • fish

Problem / Steps to reproduce

Steps to reproduce

  1. Put the following script at $XDG_DATA_HOME/bash-completion/completions/doge, which defines and registers the custom completion function for a command:
_fzf_complete_doge() {
  {
    if _fzf_complete --multi --reverse --prompt="cloud> " -- "$@"; then
      return
    fi
    ...
    COMPREPLY=...
    fi < <(completer_command)
  }
}
complete -F _fzf_complete_doge doge
  1. Open a new interactive session.
  2. Do $ doge <tab>.

On fzf 0.48.0

Items from completer_command appear.

On fzf 0.48.1

Nothing happens.

Context

I found that _fzf_complete can dynamically load the completion script from the completions directory and do the normal completion when this is not a fuzzy-finding. But it doesn't look good to me that I have to manage two separate constructs to get both fuzzy-finding and normal completion, so I define one function that can do both. I don't know if this approach is within the stability guarantee, or esoteric, but it has a practical reason.

Before 0.48.1, _fzf_complete returned 124 when it's not fuzzy-finding. On the contrary, 0.48.1 returns 0 in the same condition. In the normal completion context, _fzf_complete acts as a wrapper of the underlying dynamic loader $_fzf_completion_loader and it's natural for the dynamic loader to return 124 following the bash's completion feature.

Currently, I manually edited the completion.bash to workaround the issue:

if type _comp_load > /dev/null 2>&1; then
  # _comp_load was added in bash-completion 2.12 to replace _completion_loader.
  # We use it without -D option so that it does not use _comp_complete_minimal as the fallback.
  _loader_fix() {
    local cmd=${1:-_EmptycmD_}
    _comp_load -- "$cmd" && return 124
  }
  _fzf_completion_loader=_loader_fix
elif type __load_completion > /dev/null 2>&1; then
  # In bash-completion 2.11, _completion_loader internally calls __load_completion
  # and if it returns a non-zero status, it sets the default 'minimal' completion.
  _fzf_completion_loader=__load_completion
elif type _completion_loader > /dev/null 2>&1; then
  _fzf_completion_loader=_completion_loader
fi

Can we just do it like this?

diff --git a/shell/completion.bash b/shell/completion.bash
index 44bb904..761690c 100644
--- a/shell/completion.bash
+++ b/shell/completion.bash
@@ -308,6 +308,7 @@ _fzf_handle_dynamic_completion() {
         eval "$orig_complete"
       fi
     fi
+    [[ $ret -eq 0 ]] && return 124
     return $ret
   fi
 }

@junegunn That will certainly do! I'm thinking a bit...

As long as the zero exit code from the function referred to by $_fzf_completion_loader conveys nothing more than "The compspec is renewed. Retry completion", which also the code 124 indicates, your patch I think won't have any issue.
Since _comp_load and __load_completion behave such, it's good to me!

Thanks, fixed in dff8652.