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

Make -u idempotent (always search all files/dirs) #840

Closed
andreavaccari opened this issue Aug 29, 2021 · 16 comments · Fixed by #986
Closed

Make -u idempotent (always search all files/dirs) #840

andreavaccari opened this issue Aug 29, 2021 · 16 comments · Fixed by #986

Comments

@andreavaccari
Copy link

fd -u is an alias for fd -Iand fd -uuis an alias for fd -IH. The former is unnecessary given that it's an alias to another one-letter flag (I imagine it's there for historical reasons?) and the latter is cumbersome if someone wanted to use the long form (--unrestricted). This setup also makes the behavior of the flag ambiguous if new default filters were added in the future. In that case, would I disable all filters with -uu or would I need to use -uuu?

Given the project's focus on ergonomics and usability, I'd suggest to make -u always search all files and to deprecate -uu. This would be the equivalent of ls -a, giving the user the guarantee that no filters will be applied by default. The main cons of this suggestion is that it's a breaking change and a subtle one: the command would continue to work with -u but could return a different set of results.

@tavianator
Copy link
Collaborator

I worry about backwards incompatibility with this change. I expect many people have aliases and/or scripts already using -u which would change behaviour.

@sharkdp
Copy link
Owner

sharkdp commented Sep 3, 2021

The main reason for -u to exist is compatibility with ripgrep. It uses -u and -uu in the same way that fd does. ripgrep indeed has -uuu in addition (search within binary files) which doesn't make sense for fd. This should probably be documented.

@andreavaccari
Copy link
Author

I don't use ripgreg (although I want to), so I wasn't aware of that convention. I do agree it's desirable to maintain backward compatibility and ideally match options used by other tools. With that said, I noticed there's a pinned discussion on whether to show git-ignored files by default.

In using fd, I've found myself tweaking a search pattern because I wasn't finding a result, to then realize it was being filtered out by default. My intuition is that it should be easy to search all files (ideally by default, or with a simple flag), and to then filter results as needed.

Alternatively, it may be enough to show a info message that some files were filtered.

@tavianator
Copy link
Collaborator

@andreavaccari See #612

@andreavaccari
Copy link
Author

@tavianator thanks for the pointer, I'm aware of of this issue and read your comments. I appreciate the desire to maintain backward compatibility and opinionated simplicity. On the other hand, it's more intuitive to find too many results and narrow them down with flags, than to find too few and and open up the filters. Short of that, I like your idea to add a status message that files were ignored, and I still think it'd be beneficial to have a one-letter flag that guarantees you'll search all files.

I love opinionated tools because they are easy to use and "obvious". However, the fact that different projects use .gitignore in different ways and that hidden files are, well, hidden, make fd's default search scope neither universally easy to use nor obvious.

@sharkdp
Copy link
Owner

sharkdp commented Nov 14, 2021

I have to admit that I never use a single -u (and don't know by heart what that would mean), but always -uu. So I think I'd be okay with a breaking change here that ignores multiple uses of -u (but maybe still allows them) and changes the meaning of -u to: "completely unrestricted", i.e. equivalent to -HI.

@andreavaccari
Copy link
Author

This seems like a reasonable solution! Like you, I also never use a single -u and cannot recall what it does, so I believe simplifying the API here would still address most use cases while reducing the cognitive load. There are two thoughts I wanted to add here:

  • Would it make sense to open an issue in the ripgrep repo to see if we can introduce the same change there?
  • When -u is not used, and fd is not piped, would it make sense to print an info message to remind the user that there may be matches in ignored files?

@sharkdp
Copy link
Owner

sharkdp commented Nov 16, 2021

Would it make sense to open an issue in the ripgrep repo to see if we can introduce the same change there?

I can't imagine that they want to change their behavior.

When -u is not used, and fd is not piped, would it make sense to print an info message to remind the user that there may be matches in ignored files?

yes, please see a similar proposal here #612 (comment)

@andreavaccari
Copy link
Author

Thank you, I'll add my thoughts on the second point there.

@pradkrish
Copy link

is this issue still open? If I understand correctly, is it about making the switches -u and -HI equivalent?

@sharkdp
Copy link
Owner

sharkdp commented Nov 23, 2021

is this issue still open? If I understand correctly, is it about making the switches -u and -HI equivalent?

yes and yes

@pradkrish
Copy link

Thanks. I am new to Rust development and I plan to learn by contributing to this project. Any pointers on where to start looking to fix this will be very helpful. :)

@sharkdp
Copy link
Owner

sharkdp commented Dec 28, 2021

In src/app.rs. Search for the unrestricted option (short: u).

@jacksontheel
Copy link
Contributor

@pradkrish Are you still hoping to implement this change? I am going to start working on it, if you're no longer wanting to.

@sharkdp
Copy link
Owner

sharkdp commented Mar 18, 2022

@jacksontheel I think it's safe to say that you can go forward with this.

@andreavaccari
Copy link
Author

@sharkdp @jacksontheel Thank you for working on this! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants