Feat: safer iCloud traversal + CLI polish#11
Feat: safer iCloud traversal + CLI polish#11alberti42 wants to merge 2 commits intofarnots:masterfrom
Conversation
alberti42
force-pushed
the
master
branch
3 times, most recently
from
821c3a7 to
b24375a
Compare
There was a problem hiding this comment.
Pull request overview
This PR enhances the iCloudDownloader CLI tool with safer folder traversal, improved user experience, and better project hygiene. The changes add recursive directory traversal with the -r flag, implement a verbose mode with -v, deprecate the -A flag, skip iCloud-excluded items, and clean up Xcode user-specific files from the repository.
Changes:
- Added recursive folder traversal with
-rflag and improved folder enumeration to properly handle iCloud content - Implemented verbose mode (
-v) and help display (-h/--help), with default output showing only errors - Added macOS 11.3+ support for excluding items from iCloud sync using availability guards
Reviewed changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| iCloudDownlader/main.swift | Refactored argument parsing to support -r, -v, and -h flags; deprecated -A flag with warning |
| iCloudDownlader/Downloader.swift | Added recursive directory traversal, iCloud exclusion checking, and proper folder enumeration |
| iCloudDownlader/ConsoleIO.swift | Added verbose mode support with conditional message output |
| iCloudDownlader.xcodeproj/project.pbxproj | Updated Xcode project settings including deployment target (11.0), build settings, and team configuration |
| iCloudDownlader.xcodeproj/xcshareddata/xcschemes/iCloudDownlader.xcscheme | Moved scheme to shared location for team collaboration |
| iCloudDownlader.xcodeproj/xcuserdata/lucas.xcuserdatad/xcschemes/xcschememanagement.plist | Removed user-specific Xcode data |
| README.md | Updated documentation with new usage examples, options, and compatibility information |
| .gitignore | Added to exclude Xcode user-specific files and build outputs |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| -r Recurse into subdirectories | ||
| -v Verbose output | ||
| -h Show this help | ||
|
|
There was a problem hiding this comment.
This line contains trailing whitespace that should be removed.
| let values = try url.resourceValues(forKeys: [.ubiquitousItemIsExcludedFromSyncKey]) | ||
| return values.ubiquitousItemIsExcludedFromSync ?? false | ||
| } catch { | ||
| consoleIO.writeMessage("Can't get attributes for \(fm.displayName(atPath: url.lastPathComponent)): \(error)", to: .error) |
There was a problem hiding this comment.
The parameter name in fm.displayName(atPath:) expects a full path, but url.lastPathComponent only provides the last component of the path. This should be url.path to pass the complete path to get the proper display name.
| Without -r, only files directly inside the provided folder are checked. | ||
| """ | ||
| consoleIO.writeMessage(usage) | ||
| } else if isAll { |
There was a problem hiding this comment.
When the user provides both -A and a path argument (e.g., icd -A /some/path), the code will execute the isAll branch (line 42-43) rather than processing the provided path. This might be confusing behavior. Consider either rejecting this combination with an error message, or giving priority to the path argument if both are provided.
| } else if isAll { | |
| } else if isAll && paths.isEmpty { |
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
There are excessive blank lines at the end of the file (lines 156-158). Consider reducing this to a single blank line for consistency.
| let isHelpRequested = args.contains("-h") || args.contains("--help") | ||
| let isNoArgs = args.isEmpty | ||
| let showHelp = isHelpRequested || isNoArgs | ||
| let isVerbose = args.contains("-v") | ||
| let isRecursive = args.contains("-r") | ||
| let isAll = args.contains("-A") | ||
| let paths = args.filter { $0 != "-r" && $0 != "-A" && $0 != "-v" && $0 != "-h" && $0 != "--help" } |
There was a problem hiding this comment.
The current argument parsing logic filters out flags by checking string equality, but it doesn't handle the case where a legitimate path argument might start with a dash (e.g., a file named -test or -r). Consider using a more robust argument parsing approach, such as stopping flag parsing after -- or ensuring that file/folder paths are positional arguments that come after all flags.
| let isHelpRequested = args.contains("-h") || args.contains("--help") | |
| let isNoArgs = args.isEmpty | |
| let showHelp = isHelpRequested || isNoArgs | |
| let isVerbose = args.contains("-v") | |
| let isRecursive = args.contains("-r") | |
| let isAll = args.contains("-A") | |
| let paths = args.filter { $0 != "-r" && $0 != "-A" && $0 != "-v" && $0 != "-h" && $0 != "--help" } | |
| let isNoArgs = args.isEmpty | |
| var isHelpRequested = false | |
| var isVerbose = false | |
| var isRecursive = false | |
| var isAll = false | |
| var paths: [String] = [] | |
| var parsingFlags = true | |
| for arg in args { | |
| if parsingFlags && arg == "--" { | |
| parsingFlags = false | |
| continue | |
| } | |
| if parsingFlags { | |
| switch arg { | |
| case "-h", "--help": | |
| isHelpRequested = true | |
| case "-v": | |
| isVerbose = true | |
| case "-r": | |
| isRecursive = true | |
| case "-A": | |
| isAll = true | |
| default: | |
| // Treat any non-flag argument as a positional path | |
| paths.append(arg) | |
| } | |
| } else { | |
| // After `--`, treat all arguments as positional paths, even if they start with '-' | |
| paths.append(arg) | |
| } | |
| } | |
| let showHelp = isHelpRequested || isNoArgs |
| if status.isUbiquitousItem ?? false { | ||
| if #available(macOS 11.3, *) { | ||
| if status.ubiquitousItemIsExcludedFromSync ?? false { | ||
| consoleIO.writeMessage("\(fm.displayName(atPath: fileUrl.lastPathComponent)) is excluded from iCloud sync", to: .warning) |
There was a problem hiding this comment.
The parameter name in fm.displayName(atPath:) expects a full path, but fileUrl.lastPathComponent only provides the last component. This should be fileUrl.path to pass the complete path to get the proper display name.
| } | ||
| } | ||
| if status.ubiquitousItemDownloadingStatus == .current { | ||
| consoleIO.writeMessage("\(fm.displayName(atPath: fileUrl.lastPathComponent)) is already downloaded", to: .warning) |
There was a problem hiding this comment.
The parameter name in fm.displayName(atPath:) expects a full path, but fileUrl.lastPathComponent only provides the last component. This should be fileUrl.path to pass the complete path to get the proper display name.
|
|
||
| ## Compatibility | ||
|
|
||
| iCloudDownloader should work on macOS 10.12 Sierra and OS X 10.11 El Capitan. |
There was a problem hiding this comment.
The README states the tool "should work on macOS 10.12 Sierra and OS X 10.11 El Capitan," but the MACOSX_DEPLOYMENT_TARGET is now set to 11.0. This is inconsistent. Additionally, the code uses macOS 11.3-specific APIs (ubiquitousItemIsExcludedFromSyncKey), so the minimum supported version should be updated to macOS 11.0 (or 11.3 for full functionality) in the documentation.
| iCloudDownloader should work on macOS 10.12 Sierra and OS X 10.11 El Capitan. | |
| iCloudDownloader requires macOS 11.0 or later. For full functionality (including handling of items excluded from iCloud sync), macOS 11.3 or later is recommended. |
| COPY_PHASE_STRIP = NO; | ||
| DEAD_CODE_STRIPPING = YES; | ||
| DEBUG_INFORMATION_FORMAT = dwarf; | ||
| DEVELOPMENT_TEAM = 9V3X7C8VCK; |
There was a problem hiding this comment.
The DEVELOPMENT_TEAM is hardcoded to a specific team ID (9V3X7C8VCK). This is user-specific configuration that should not be committed to the repository as it will cause build issues for other developers. Consider removing this setting or setting it to an empty string.
| DEVELOPMENT_TEAM = 9V3X7C8VCK; | |
| DEVELOPMENT_TEAM = ""; |
| COPY_PHASE_STRIP = NO; | ||
| DEAD_CODE_STRIPPING = YES; | ||
| DEBUG_INFORMATION_FORMAT = "dwarf-with-dsym"; | ||
| DEVELOPMENT_TEAM = 9V3X7C8VCK; |
There was a problem hiding this comment.
The DEVELOPMENT_TEAM is hardcoded to a specific team ID (9V3X7C8VCK). This is user-specific configuration that should not be committed to the repository as it will cause build issues for other developers. Consider removing this setting or setting it to an empty string.
| DEVELOPMENT_TEAM = 9V3X7C8VCK; | |
| DEVELOPMENT_TEAM = ""; |
1 participant
PR Summary
Motivation
Improve the iCloudDownloader CLI so it behaves more predictably with folders (which can appear downloaded even when their contents are not), supports explicit recursive traversal, avoids crawling excluded items, and reduces noisy output while offering a verbose mode when needed. Clean up Xcode user-specific state in the repo.
Changes
-rfor recursive traversal and updated folder handling to enumerate contents properly.-h/--helpand improved no-argument behavior to show usage guidance.-vfor verbose output; default mode now prints only errors.ubiquitousItemIsExcludedFromSyncwith macOS availability guards.-Ain favor oficd $PWD(help/docs updated, warning emitted when used)..gitignore, removed trackedxcuserdata, and moved the scheme toxcshareddata.Notes
-r, only files directly inside the provided folder are checked and subdirectories are not traversed;-rrecurses into subdirectories. Excluded items are skipped.xcodebuildin Release mode.