Hi Miguel, Thank you for reviewing. A better unit test would help of course ;-) but testing links can be a pain as the JVM requires admin rights on Windows 10 at least. This might not be an issue for GitHub but be aware of it if you run builds locally.
The class per its javadoc filters for symbolic links on files. Gary On Fri, Apr 14, 2023, 10:05 Gary Gregory <garydgreg...@gmail.com> wrote: > Let's assume this is for IO, not VFS; in the future, please prefix email > subjects with > [IO] or [VFS] or whatever Commons component is being discussed. > > TY! > Gary > > On Thu, Apr 13, 2023, 01:59 Miguel Muñoz <swingguy1...@gmail.com> wrote: > >> It's not clear what exactly this filter is supposed to do. From its >> behavior, I gather that it's supposed to distinguish files from >> directories, treating Symbolic links as ordinary files. There are two >> reasons why this isn't clear. >> >> 1. This behavior (usually) seems identical to that of a filter that just >> relies on File.isDirectory(). >> 2. The current behavior is inconsistent: >> >> I made a test directory with four entries: a file, a directory, and a >> symbolic link to each. >> >> I wrote code using SymbolicLinkFileFilter.INSTANCE as a FileFilter, a >> FileNameFilter, and a PathFilter. I created a second filter that relied on >> File.isDirectory(). The two filters behaved the same for three of the >> files, but differed for the symbolic link to the directory. (I'm not sure >> if the filter should treat that as a file or a directory, since it's not a >> directory, but it resolves as one.) >> >> Used as a FileFilter and a FileNameFilter, the accept() methods returned >> false. Used as a PathFilter, it returned FileVisitResult.CONTINUE, which >> essentially means true. >> >> Looking through the code, the difference in behavior is apparently because >> the call to resolve the PathFilter is the only one that calls >> Files.isSymbolicLink(). >> >> I could fix this, but I don't know which behavior is correct. >> >> I'll send you my test code if you want it. >> >> — Miguel Muñoz >> >