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

Hide ./ prefix when fd version >= 8.3.0 #211

Merged
merged 7 commits into from
Dec 1, 2021

Conversation

kidonng
Copy link
Contributor

@kidonng kidonng commented Nov 29, 2021

fd 8.3.0 introduced a --strip-cwd-prefix flag to revert a new behavior: ./ prefix is shown when output is not a tty (terminal).

This is what you get in _fzf_search_directory without this patch:
截屏2021-11-29 20 06 42
And with this patch:
截屏2021-11-29 20 06 53

@kidonng
Copy link
Contributor Author

kidonng commented Nov 29, 2021

BTW, there are some typos on https://github.com/PatrickF1/fzf.fish/wiki/Thanks 😄

@PatrickF1
Copy link
Owner

Thanks, I was just about to do this myself.

Btw, you have my email--feel free to email me the typos and I'll fix them. Sorry about that.

@@ -7,6 +7,12 @@ function _fzf_search_directory --description "Search the current directory. Repl
# unescape token because it's already quoted so backslashes will mess up the path
set unescaped_exp_token (string unescape -- $expanded_token)

# Hide ./ prefix when fd version >= 8.3.0
# https://github.com/sharkdp/fd/releases/tag/v8.3.0
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you move this up to line 3?
And could you add a todo here that we'll remove this check in a few months and --strip-cwd-prefix is included in fd's documentation (it's not currently there) and bump up the required fd version to 8.3.0?

@PatrickF1
Copy link
Owner

Oh, actually could you add a test for when fd is > 8.3 and when it's < 8.3?

@kidonng
Copy link
Contributor Author

kidonng commented Dec 1, 2021

Oh, actually could you add a test for when fd is > 8.3 and when it's < 8.3?

...using the actual program or a mocked fd? Either way it will be too much for this simple check. I don't like tests for the sake of testing.

@PatrickF1
Copy link
Owner

Oh a second thought, I kind of like the idea of leaving in the ./ always and instead removing the logic that appends / to the path if it's a directory. Advantages

  • less code
  • now users can just hit enter to cd or execute if it's an executable
  • works with both pre 8.3 and post 8.3

@kidonng
Copy link
Contributor Author

kidonng commented Dec 1, 2021

No, it will look ugly with a base directory (base/./path). Also note that fd won't show the prefix it is connected to a tty, because of the same reason: the prefixes are just plain noises to human eyes.

@PatrickF1
Copy link
Owner

You're right--it is pretty ugly. Ok we'll stick with this version!

@PatrickF1 PatrickF1 merged commit 6b592e4 into PatrickF1:main Dec 1, 2021
PatrickF1 added a commit that referenced this pull request Mar 19, 2022
In #211, we updated fzf.fish to conditionally pass --strip-cwd-prefix to prevent fd >= 8.3.0 from prepending ./ to the paths sent into fzf. Now that fd 8.3.0 has been out for almost 4 months and most users should have updated to it by now, let's remove this special logic supporting older fd versions to reduce code complexity. This also means the minimum supported version of fd is now 8.3.0.
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