mxcl
Fix Path.Finder use case when a hidden file and a regular subdirectory are in the same level
When using Path.swift to build a tool, I noticed it had a bug related to its implementation around FileManager.DirectoryEnumerator. Here is a sample use case, which I added a test case to cover.
Imagine you have the following file tree:
.
├── .a
│ └── foo.txt
├── .foo.txt
└── b
├── .bar.txt
├── c
│ └── bar.txt
└── foo.txt
When running find() on root, including hidden files, one would expect to get all files and directories - and this is working fine. However, when setting .hidden(false), only the directory b is returned - and not b/c, b/c/bar.txt, nor b/foo.txt.
This happens because of these lines in the next() method in Path.Finder:
if !hidden, path.basename().hasPrefix(".") {
enumerator.skipDescendents()
continue
}
When finding the first hidden file or directory, skipDescendants() is called. And according to the docs, this is what it does:
Causes the receiver to skip recursion into the most recently obtained subdirectory.
This means that, when the first hidden file is found, and skipDescendants() is called, all files and directories residing alongside the hidden file are skipped.
This PR adds a check, and only calls skipDescendants() if the path in question is a directory - if it's just a file, then nothing should be skipped, as other sibling files or directories might be "waiting" to be enumerated.
Additionally, I changed the FileManager.DirectoryEnumerator initialization to use a more modern API, that can receive a list of URLResourceKeys, and passes .isDirectoryKey - this way, there's no need to do an extra disk read for each isDirectory check, as they're fetched on the enumeration stage and only read via a URL's URLResourceValues.
@mxcl glad you liked it and learned a new API - I had no idea resourceValues were a thing on URLs either. I only knew about FileManager's contentsOfDirectory... method before, and met FileManager.DirectoryEnumerator thanks to this repo 😄
Once CI passes - may I merge it? Do you think you could publish a new release?
We have to continue to support the same set of Swifts or we need to bump the major. Swift 5.0 is not that old. But I dunno, what do you think?
CI/CD needs to pass. If you look at my contribution graph you can already see I basically only work on open source 7 days a week. CI/CD shit is hours and hours of tedious busywork.
We need it to pass or we need to bump the major version.
I know I know, I can deal with it. For some reason I forgot that CI could be changed in this PR itself. What Swift version could be set as the minimum? I would suggest 5.5 + bumping the package major version.
I know I know, I can deal with it.
like I don't feel that is fair either. But I really have no time to mess with this :(
What Swift version could be set as the minimum? I would suggest 5.5 + bumping the package major version.
Ideally we don’t bump, we promise to work with the version matrix we test against. However if it cannot be helped I can make a judgement call. I get that very few people use these old Swifts anymore, but that is the promise we make with semantic versioning.
Like my new project pkgx should be able to provide swift versions all the way back. I don't think we do yet tho.