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

Split executable and empty file "types" into seperate "prop" flag #847

Closed
wants to merge 1 commit into from

Conversation

tmccombs
Copy link
Collaborator

@tmccombs tmccombs commented Sep 5, 2021

But keep the old file types for backwards compatibility.

Fixes #823

This is a work in progress. I haven't added anything to the changelog yet, and the documentation could probably be a little better. I'm also not entirely sure if this is something we want to do.

Note that the new --prop options behave slightly differently than the old file types. In particular:

  • If no --type option is given, then empty defaults to just empty files, not files and directories. That makes it more consistent with the way executable works, seems more useful to me, and is simpler to implement, but I could be convinced to change this to default to files and directories.
  • The executables property no longer always includes files, so you can search for just executable directories. I'm not really sure when that would be useful, but it seems more consistent and logical to me.

But keep the old file types for backwards compatibility.

Fixes sharkdp#823
@tmccombs tmccombs requested a review from sharkdp September 5, 2021 07:18
@sharkdp
Copy link
Owner

sharkdp commented Oct 12, 2021

Thank you very much for exploring this path. I will have to think a bit more about this. I really like that we currently only have one option instead of two. On the other hand, if the behavior is really inconsistent, we should probably sacrifice simplicity for correctness.

Does this PR itself introduce any breaking behavior, i.e. do you plan to remove -tx?

@tmccombs
Copy link
Collaborator Author

iirc, it doesn't actually remove -tx to preserve backwards compatibility, but it does remove documentation from the help and man page.

@sharkdp
Copy link
Owner

sharkdp commented Nov 26, 2021

Closing for now, see #823

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.

Make --type empty its own option
2 participants