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

HiddenFileFilter behaviour doesn't match its description #22

Open
jroper opened this issue Jul 27, 2016 · 4 comments
Open

HiddenFileFilter behaviour doesn't match its description #22

jroper opened this issue Jul 27, 2016 · 4 comments
Labels
Bug uncategorized Used for Waffle integration

Comments

@jroper
Copy link
Member

jroper commented Jul 27, 2016

See

/** A [[FileFilter]] that selects files that are hidden according to `java.io.File.isHidden` or if they start with a dot (`.`). */

The comment there is:

A [[FileFilter]] that selects files that are hidden according to java.io.File.isHidden or if they start with a dot (.).

That sounds like reasonable behaviour for the filter, but the implementation doesn't match that at all, it selects files that are hidden AND whose names aren't EQUAL to .. It needs to be changed to OR and startsWith dot to match.

Obviously the implementation has a bug, but the fix I think is less obvious - not excluding files that start with a dot probably hasn't caused anyone any issues, otherwise this issue would be reported and fixed. The question is, how many builds will break if the behaviour is changed to exclude files that start with a dot? I wouldn't be surprised if some builds do break.

So the question is, should the behaviour be updated to match the description, or should the description be modified to match the behaviour (and the check for the file name being equal to . be removed altogether)?

@dwijnand
Copy link
Member

Note the javadoc for java.io.File.isHidden (which I didn't know before) (link):

 * Tests whether the file named by this abstract pathname is a hidden
 * file.  The exact definition of <em>hidden</em> is system-dependent.  On
 * UNIX systems, a file is considered to be hidden if its name begins with
 * a period character (<code>'.'</code>).  On Microsoft Windows systems, a file is
 * considered to be hidden if it has been marked as such in the filesystem.

Which leaves me even more perplexed as to what should or shouldn't be changed implementation-wise.

Definitely whatever happens here the docs should be updated to reflect the implementation.

@eed3si9n
Copy link
Member

The question is, how many builds will break if the behaviour is changed to exclude files that start with a dot?

In light of what Dale posted, the current implementation should exclude the files starting with a dot if you're on Mac and Linux.

So the question is, should the behaviour be updated to match the description, or should the description be modified to match the behaviour (and the check for the file name being equal to . be removed altogether)?

Assuming that (file.getName != ".") is compensating for Java's platform specificity, my vote would be fix the implementation to change to startsWith, effective sbt 1.x or 0.13.

@eed3si9n eed3si9n added the Bug label Jul 27, 2016
@jroper
Copy link
Member Author

jroper commented Jul 28, 2016

@eed3si9n That sounds fine to me. I don't think the fix is urgent - I couldn't find any evidence that anyone has ever noticed it before, and it's probably existed since forever, so 1.x is fine.

@eed3si9n eed3si9n added the uncategorized Used for Waffle integration label Sep 18, 2018
@eatkins
Copy link
Contributor

eatkins commented Sep 27, 2018

I actually modified this filter for performance reasons in #178: 04fe874
It would be a bad idea to change to startsWith because Windows has a different definition of hidden files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug uncategorized Used for Waffle integration
Projects
None yet
Development

No branches or pull requests

4 participants