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 --type empty its own option #823

Closed
tsoutsman opened this issue Aug 10, 2021 · 8 comments · Fixed by #883
Closed

Make --type empty its own option #823

tsoutsman opened this issue Aug 10, 2021 · 8 comments · Fixed by #883

Comments

@tsoutsman
Copy link
Contributor

-t, --type <filetype>...              
        Filter the search by type (multiple allowable filetypes can be specified):
          'f' or 'file':         regular files
          'd' or 'directory':    directories
          'l' or 'symlink':      symbolic links
          'x' or 'executable':   executables
          'e' or 'empty':        empty files or directories
          's' or 'socket':       socket
          'p' or 'pipe':         named pipe (FIFO)

the help multiple allowable filetypes can be specified suggests that adding multiple type parameters will have an OR effect (if a file is any of the types then it will be included in the results), but this is not always the case. In particular, the empty type is inconsistent. When used with symlink, socket, or pipe, it works in accordance with the help, anything that is empty OR the other type will be included. However, if you use it with file, executable, or directory it will only include results if they are both empty AND the other type (e.g. fd --type empty --type directory only includes empty directories.

empty being a type prevents me from searching for empty symlinks, sockets, or pipes. (I'm not sure if these are possible but I digress)

This issue is touched upon in #714 :

You are right. executable and empty are special in that they mean: only show executable/empty results. Otherwise --type file --type empty would mean: "show me files OR empty directories/files", which would result in ALL files and empty directories. I don't think that's what most users would expect. See also: #246 (comment)

I think empty being made its own option would fix this issue, removing any confusion.

@tmccombs
Copy link
Collaborator

I don't think an empty socket or pipe makes sense. Does an empty symlink mean the target of the symlink is empty?

@tsoutsman
Copy link
Contributor Author

I don't think an empty socket or pipe makes sense. Does an empty symlink mean the target of the symlink is empty?

I'd assume so, but it's not so much about adding functionality by allowing to search for empty symlinks; rather, it's more about having a consistent behaviour. I find it kind of confusing because the help suggests a different behaviour to what is actually happening. A solution would be to document the special case in the help and leave it, but I don't think that will fix the underlying issue, which is, in my opinion, with the "flawed" design.

If you feel that it is fine, feel free to close the issue, this issue is based on my subjective opinion, so you and others may disagree.

@tmccombs
Copy link
Collaborator

So is this proposal to add a new --empty option that acts the same as the current --type empty, or is the behavior of the empty flag different? If it is different do we keep the empty file type? that seems like it would be confusing.

And what about executable that has similar behavior to empty, should that be a separate flag as well?

I'm not saying we shouldn't do this, just trying to get a clear idea of what would change.

I wonder if maybe there should be a different grouping that has things like empty, executable, and potentially text or binary. Since those are more properties of the file rather than "filetype"s.

@tsoutsman
Copy link
Contributor Author

Yea the proposal is to add a new --empty flag that would act pretty much identically to the current --type empty. I just think that empty being put under the --type option is wrong.

I didn't want to suggest executable as I looked through a bit of the source code of fd and it seems like directories are classed as executable if they contain executables. I doubt that if someone is using the --executable flag they are looking for directories. Though, in a sense, it is a similar issue to empty.

I think some sort of property option would be a good idea, as, similar to file types, you could group the modifiers in the help menu, and it would also probably make the implementation cleaner and more understandable.

@tmccombs
Copy link
Collaborator

it seems like directories are classed as executable if they contain executables

No, it tests if the directory has the execute permission. Which on directories means that you can access files within the directory. It does seem kind of odd to me that suppling a type of executable implies --type file, so you can't filter to executable directories.

I've started a branch to address this, I'll post a PR shortly.

tmccombs added a commit to tmccombs/fd that referenced this issue Sep 5, 2021
But keep the old file types for backwards compatibility.

Fixes sharkdp#823
@sharkdp
Copy link
Owner

sharkdp commented Nov 14, 2021

Thank you for reporting and discussing this, @tsoutsman and @tmccombs.

The current behavior of the --type option was carefully designed and is not an oversight. I am fully aware of the (potentially confusing?) behavior when involving empty/executable. I also fully agree that the documentation should be improved.

But I don't really see that we need a new option for this. I haven't seen any valid use cases which we can not solve with the current option. And I think a certain simplificity in CLI design is a very valuable feature of fd.

I am proposing that we change the --help text and man page to the following text:

    -t, --type <filetype>...
            Filter the search by type:
              'f' or 'file':         regular files
              'd' or 'directory':    directories
              'l' or 'symlink':      symbolic links
              's' or 'socket':       socket
              'p' or 'pipe':         named pipe (FIFO)
            
              'x' or 'executable':   executables
              'e' or 'empty':        empty files or directories
            
            This option can be specified more than once to include multiple file types.
            Searching for '--type file --type symlink' will show both regular files as well as
            symlinks. Note that the 'executable' and 'empty' filters work differently: '--type
            executable' implies '--type file' by default. And '--type empty' searches for
            empty files and directories, unless either '--type file' or '--type directory' is
            specified in addition.
            
            Examples:
              - Only search for files:
                  fd --type file …
                  fd -tf …
              - Find both files and symlinks
                  fd --type file --type symlink …
                  fd -tf -tl …
              - Find executable files:
                  fd --type executable
                  fd -tx
              - Find empty files:
                  fd --type empty --type file
                  fd -te -tf
              - Find empty directories:
                  fd --type empty --type directory
                  fd -te -td

I think those five examples cover a huge percentage of use cases. And I also believe that the current syntax is actually quite intuitive... unless someone tries to interpret it as a strict OR - which it is not.

I agree that it's not fully satisfactory, but I still like this better than introducing a completely new command line option.

What do you think @tmccombs @tsoutsman?

sharkdp added a commit that referenced this issue Nov 14, 2021
sharkdp added a commit that referenced this issue Nov 14, 2021
@sharkdp
Copy link
Owner

sharkdp commented Nov 14, 2021

I'm actually going ahead with merging #883, as it certainly doesn't make things worse. I'm still happy to discuss this futher and consider an approach like #847.

@sharkdp sharkdp reopened this Nov 14, 2021
@sharkdp
Copy link
Owner

sharkdp commented Nov 26, 2021

I'm closing this and #847 for now, let me know if someone wants to discuss this further.

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