Use `builtin cd` in fzf Alt+C Key Bindings for Fish Shell

[Undefined01]

Checklist

• [X] I have read through the manual page (man fzf)
• [X] I have searched through the existing issues
• [X] For bug reports, I have checked if the bug is reproducible in the latest version of fzf

Output of fzf --version

0.52.0 (bcda25a5)

OS

• [X] Linux
• [ ] macOS
• [ ] Windows
• [ ] Etc.

Shell

• [ ] bash
• [ ] zsh
• [X] fish

Problem / Steps to reproduce

I'm encountering an issue with the Alt+C key binding in the Fish shell when using zoxide, which overrides the cd command. When I activate the Alt+C shortcut, it is not calling the builtin cd because of the alias setup by zoxide, leading to a failure in switching directories.

I think changing this line to builtin cd -- $result is sufficient to solve this problem.

This modification is aligned with the implementation in the Bash script, where builtin cd is explicitly used to avoid similar conflicts.

[junegunn]

3830 should fix the issue. It's in the devel branch, and will be included in the next release.

[harkabeeparolus]

I can't reproduce the bug. I installed the latest version of zoxide, and tried:

fzf --fish | sed 's/builtin //' > $__fish_config_dir/conf.d/fzf.fish

Reloaded fish shell, and then:

zoxide init fish | source

And both z, fzf Alt-C and Directory History seems to be working.

As far as I can tell, zoxide actually doesn't override the default cd command. In fact, zoxide saves a copy of the cd function, to prevent any problems, which is something that fzf does not do. For this reason, I was able to break fzf by running an additional alias cd z, but this is not the default zoxide behaviour.

Seems to me that alias cd z in fish is a bad idea. A much better fish solution would be to use abbr cd z instead of alias.

[harkabeeparolus]

Perhaps a good enough fix would be to document that fzf Alt-C in fish is only guaranteed to work if you do not alias cd to another command.

And in general, I would recommend abbr instead of alias. I think many former bash and zsh users are very fond of aliases, but really, in fish it's a lot better to learn to use abbreviations instead, since aliases are actually implemented as functions behind the scenes. So alias cd z will actually remove the builtin Directory History function. 😟

[LangLangBart]

As far as I can tell, zoxide actually doesn't override the default cd command.

zoxide init --cmd cd fish | source

The author of the issue has this setup[^1][^2]

command -v zoxide >/dev/null 2>&1 && eval "$(zoxide init --cmd cd $(__common_rc_get_shell))"

# fish
paria@Paria ~> type -a cd
cd is a function with definition
# Defined via `source`
function cd --wraps=__zoxide_z --description 'alias cd=__zoxide_z'
  __zoxide_z $argv

end
cd is a builtin
cd is /usr/bin/cd

[^1]: dotfiles/scripts/function/commonrc.sh at main · Undefined01/dotfiles · GitHub [^2]: ajeetdsouza/zoxide - configuration

[harkabeeparolus]

Ah, thanks! That explains it. Using zoxide init --cmd cd fish generates the following fish code:

# =============================================================================
#
# Commands for zoxide. Disable these using --no-cmd.
#

abbr --erase cd &>/dev/null
alias cd=__zoxide_z

abbr --erase cdi &>/dev/null
alias cdi=__zoxide_zi

This setup does actually (mostly) work; it preserves fish shell's Directory History and cd -, since zoxide saves a copy of the previous cd function. That's clever. 😊

The underlying problem between zoxide and fzf is caused by zoxide actually being incompatible with the default cd command in fish. Regular cd can handle the safer syntax cd -- newdirectory #2659, but zoxide can not ajeetdsouza/zoxide#332. The zoxide people tried to work around this by modifying fzf #2799, which caused fzf Alt-C to break fish shell's default Directory History and cd -.

Normally in fish you should be able to run cd -- foo and also cd --help. For this reason, I would recommend petitioning zoxide to fix their bug: ajeetdsouza/zoxide#332

[harkabeeparolus]

An alternative strategy, if zoxide is not able to fix their bug for whatever reason, could be to improve the solution from #2659.

Depending on "--" isn't optimal. The best solution is to prepend filenames with "./" when necessary:

[...] if you read in a pathname, as early as possible see if it begins with “-”... if it does, prepend “./”. This eliminates this source of pathnames that are confused as option flags.

[LangLangBart]

The best solution is to prepend filenames with "./" when necessary:

Would you argue for this change, or should there be a - check before using ./?

--- a/shell/key-bindings.fish
+++ b/shell/key-bindings.fish
@@ -105,5 +105,5 @@ function fzf_key_bindings

       if [ -n "$result" ]
-        builtin cd -- $result
+        cd ./$result

         # Remove last token from commandline.

Applied the patch from above, and it works (only tested on macOS).

# Create a dummy folder for testing with a leading '-' (dash)
 mkdir -- "-zsh"
# Start minimal Fish environment
command env -i HOME=$HOME TERM=$TERM USER=$USER PATH=$HOME/Developer/fzf/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin /usr/local/bin/fish
fzf --version
# 0.54.0 (9e92b6f1)

fzf --fish | source
zoxide init --cmd cd fish | source
# Press 'alt-c' and select '-zsh'
# 'cdh' and 'cd -' work as expected

---

Current workaround:

fzf --fish | sed 's|builtin cd -- |cd ./|' | source

[junegunn]

+        cd ./$result

This won't work if a custom FZF_ALT_C_COMMAND prints absolute paths.

[LangLangBart]

Checking for a leading dash:

--- a/shell/key-bindings.fish
+++ b/shell/key-bindings.fish
@@ -105,5 +105,9 @@ function fzf_key_bindings

       if [ -n "$result" ]
-        builtin cd -- $result
+        if string match --quiet -- '-*' "$result"
+            cd ./$result
+        else
+            cd $result
+        end

         # Remove last token from commandline.

[harkabeeparolus]

That looks good to me! I think this is the best solution. It's as safe as the "--" strategy, while avoiding the bug in zoxide.