PatrickF1 / fzf.fish

🔍🐟 Fzf plugin for Fish
MIT License
2k stars 81 forks source link

[Search Directory] Use fd or fdfind unaliased to resolve apparent hang #282

Closed Susensio closed 1 year ago

Susensio commented 1 year ago

A cursory search through GitHub code for "alias fd fdfind" reveals hundreds of pieces of code that aliases fdfind to fd instead of using symlinks (in spite of fd's instructions to symlink instead). And that's just the code that's public! The problem with this is that fish functions buffer their output, so when fd is aliased, Search Directory does not pop open fzf until fd is done outputting files. If Search Directory is triggered in $HOME or some other large directory, then Search Directory appears to hang unresponsively.

We fix this by using the fd binary directly. Since presumably many of fzf.fish users on certain Linux distros have it installed as fdfind, we look for either binaries and use it. Resolves #281.

PatrickF1 commented 1 year ago

The test "~/ is expanded to HOME" failed because the test works by capturing the fd arguments using a wrapper function, but now that we are using command, the wrapper function is bypassed. We'll need to rewrite the test. I'd test it now by creating a directory with an certain number of files in it, scope the search to that directory, select all, and test that the number of files outputted = number of files in that directory.

zolrath commented 1 year ago

On Ubuntu there is a default fd which is a different CLI utility entirely (File & Directory tool)

So when the fd_cmd searches for fd first, it finds it but it still results in a broken directory search

set fd_cmd (command -v fd || command -v fdfind || echo "fd")

Instead, if the first check is for fdfind

set fd_cmd (command -v fdfind || command -v fd || echo "fd")

this operates properly even though there is a preexisting, incorrect fd.

PatrickF1 commented 1 year ago

Thanks @zolrath for reporting this. My bad, should've been an easy catch if I had thought it through. This is why I need code reviews 😅️ Just curious, is fd (File & Directory tool) a default tool on Ubuntu? I hope I didn't just break this for ALL Ubuntu users.

PatrickF1 commented 1 year ago

Fixed in https://github.com/PatrickF1/fzf.fish/commit/7b14fa7a54d1f91ce946c3f1651e26cc4a3dcd6c

zolrath commented 1 year ago

Thanks @zolrath for reporting this. My bad, should've been an easy catch if I had thought it through. This is why I need code reviews 😅️ Just curious, is fd (File & Directory tool) a default tool on Ubuntu? I hope I didn't just break this for ALL Ubuntu users.

Yep, it's at least installed by default in 22.04! Thanks for applying the fix! 🔥