FileManager: Implement missing Path and URL Enumerator functionality. #1548
Conversation
- NSURLDirectoryEnumerator: Implement .skipsPackageDescendants and .skipsHiddenFiles options when recursing through sub-directories. - NSPathDirectoryEnumerator: Implement directoryAttributes, fileAttributes, level and skipDescendants.
|
@swift-ci please test |
Foundation/FileManager.swift
Outdated
| switch Int32(current.pointee.fts_info) { | ||
| case FTS_D: | ||
| if _options.contains(.skipsSubdirectoryDescendants) { | ||
| if skipHidden || _options.contains(.skipsSubdirectoryDescendants) || (_options.contains(.skipsPackageDescendants) && filename.hasSuffix(".app")) { |
There was a problem hiding this comment.
The definitions of "isHidden" and "isPackage" should be abstracted away into some private functions or something. Eventually this abstraction should be URLResourceValues-based, but internal functions should suffice for now.
Perhaps even better, you could have a single function taking the path or filename and the enumeration options that decides whether the file should be hidden. This avoids the variable naming nit above.
That all said, since packages aren't an actual concept on Linux, so I feel that the option should be a no-op.
The . prefix definition of "hidden" seems fine, though incomplete on Darwin.
There was a problem hiding this comment.
What else marks a file as hidden on Darwin?
There was a problem hiding this comment.
There are a mismash of conditions, including a dot prefix, the UF_HIDDEN chflag (which I expect exists on Linux as well?) and a very old "hidden" bit in the "FinderFlags", which today is usually implemented as an extended attribute.
Foundation/FileManager.swift
Outdated
| } | ||
|
|
||
| _current = fts_read(stream) | ||
| while let current = _current { | ||
| let filename = NSString(bytes: current.pointee.fts_path, length: Int(strlen(current.pointee.fts_path)), encoding: String.Encoding.utf8.rawValue)!._swiftObject | ||
| let skipHidden = (_options.contains(.skipsHiddenFiles) && filename[filename._startOfLastPathComponent] == ".") |
There was a problem hiding this comment.
Nit on the variable name: I expected it to just have the value of _options.contains(.skipsHiddenFiles). I'd suggest skipBecauseHidden or something like that. But see suggested refactoring down below.
- Dont match .skipsPackageDescendants on non-Darwin platforms.
Foundation/FileManager.swift
Outdated
| skipDescendants = true | ||
| } | ||
| #if canImport(Darwin) | ||
| if options.contains(.skipsPackageDescendants) && filename.hasSuffix(".app") { |
There was a problem hiding this comment.
Let's not do this .app stuff at all, even on Darwin. There are a plethora of extensions that can represent a package, and even more non-filename-based conditions. In this instance, I'd say it's better to have consistent behavior between platforms than have an inadequate Darwin behavior.
I know this means that the API's implementation does nothing, but I don't think there's a better alternative at the moment.
There was a problem hiding this comment.
Ive removed the .skipsPackageDescendants check completely.
Foundation/FileManager.swift
Outdated
| } | ||
| #endif | ||
| } | ||
| if options.contains(.skipsHiddenFiles) && (filename[filename._startOfLastPathComponent] == ".") { |
There was a problem hiding this comment.
Seems like this could be checked first, letting us skip the isDir block if true, since it overrides the previous settings. But not a big deal, since the skipDescendants check is not expensive.
There was a problem hiding this comment.
I guessed that ._startOfLastPathComponent would be the most expensive. Also when skipDescendants is ignored later its not computed because isDir is false. But none of these checks should touch the fs anyway.
Foundation/FileManager.swift
Outdated
|
|
||
| switch Int32(current.pointee.fts_info) { | ||
| case FTS_D: | ||
| if skipHidden || _options.contains(.skipsSubdirectoryDescendants) || (_options.contains(.skipsPackageDescendants) && filename.hasSuffix(".app")) { | ||
| let (showFile, skipDescendants) = match(filename: filename, to: _options, isDir: true) |
There was a problem hiding this comment.
The tuple return is interesting… I'll allow it. :-)
The main concern is that if the skipDescendants check ended up touching the file system or doing some other expensive work, it would be wasted work if you just assign it to _.
|
@swift-ci please test |
|
@swift-ci please test and merge |
1 similar comment
|
@swift-ci please test and merge |
NSURLDirectoryEnumerator: Implement .skipsPackageDescendants and
.skipsHiddenFiles options when recursing through sub-directories.
NSPathDirectoryEnumerator: Implement directoryAttributes,
fileAttributes, level and skipDescendants.