-
Notifications
You must be signed in to change notification settings - Fork 87
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
[Search Directory] Use fd or fdfind unaliased to resolve apparent hang #282
Conversation
Ensure no user functions are called
functions/_fzf_search_directory.fish
Outdated
@@ -1,4 +1,7 @@ | |||
function _fzf_search_directory --description "Search the current directory. Replace the current token with the selected file paths." | |||
# On Ubuntu and other Debian-based Linux distributions, fd binary is called fdfind. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*fd binary is installed as
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. |
On Ubuntu there is a default So when the set fd_cmd (command -v fd || command -v fdfind || echo "fd") Instead, if the first check is for set fd_cmd (command -v fdfind || command -v fd || echo "fd") this operates properly even though there is a preexisting, incorrect |
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 😅️ |
Follow up to a bug in #282. #282 (comment)
Follow up to a bug and missing readme update in #282. #282 (comment)
Fixed in 7b14fa7 |
Yep, it's at least installed by default in 22.04! Thanks for applying the fix! 🔥 |
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.