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

feat: Add option to always include cwd prefix #1445

Merged
merged 3 commits into from May 8, 2024

Conversation

tmccombs
Copy link
Collaborator

@tmccombs tmccombs commented Dec 6, 2023

Fixes: #1243
Fixes: #1331

  • add test
  • update changelog

src/cli.rs Outdated
overrides_with("strip_cwd_prefix"),
long_help
)]
pub cwd_prefix: bool,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Alternatively, I could change the --strip-cwd-prefix option to take an enum value that is one of "always", "never", and "auto" (current default behavior). Although in that case I would want to treat it being given without a value as always. I'm not sure if clap can handle that.

@tmccombs tmccombs requested a review from sharkdp December 6, 2023 08:15
@tmccombs tmccombs marked this pull request as draft December 6, 2023 08:15
@tavianator
Copy link
Collaborator

Not sure if this was mentioned in the linked issues, but it is already possible to get the leading ./ by specifying . as the search path explicitly, e.g.

$ fd . .
./cli.rs
./config.rs
...
$ fd --search-path .
./cli.rs
./config.rs
...

@tmccombs tmccombs force-pushed the cwd-prefix branch 2 times, most recently from 6567b29 to 9cbf494 Compare December 11, 2023 08:17
doc/fd.1 Outdated Show resolved Hide resolved
doc/fd.1 Outdated Show resolved Hide resolved
-X/--exec-batch, or -0/--print0 are given, to reduce the risk of a
path starting with '-' being treated as a command line option. Use
this flag to change this behavior. If htis flag is used without a value,
it is equivalent to passing "always". Possible values are:
Copy link
Owner

Choose a reason for hiding this comment

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

I would have expected "auto" here, since it is described as the default behavior below?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Making it equivalent to always makes it simpler to turn it on. And I think that having --strip-cwd-prefix without a specific when value meaning "auto" instead of "always" could be confusing since it reals like it is always stripping the prefix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Also, using always if this is used without an argument is more backwards compatible with existing usage of the argument.

Copy link
Owner

Choose a reason for hiding this comment

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

I think the thing that confused me was that "auto" is described as "Use the default behavior" below. And I misinterpreted that as "auto = do whatever you do when no argument is given". After re-reading everything, it still seems a bit strange to me, but I agree that always makes sense as a default.

@sharkdp
Copy link
Owner

sharkdp commented May 6, 2024

Thank you for the update

@tmccombs tmccombs merged commit 6becb66 into sharkdp:master May 8, 2024
17 checks passed
@tmccombs tmccombs deleted the cwd-prefix branch May 8, 2024 03:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

force cwd-prefix to be on [BUG] --strip-cwd-prefix docs out of date
3 participants