feat: Icon generation fix - #9
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an opt-in “generated asset catalog” mode so VersionIcon can write configuration-specific app icons into a separate .xcassets output directory, avoiding modifications to tracked source icon assets.
Changes:
- Added
--outputAssetCatalog <path>CLI option and threaded it throughScriptSetup. - Updated app icon folder resolution to support generating/syncing an output
.appiconsetbased onAppIconOriginal. - Added a regression test validating source icons remain untouched when generating multiple configurations; documented usage in the README.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/VersionIconTests/VersionIconTests.swift | Adds regression coverage for generated-catalog mode ensuring source icons stay unchanged across multiple runs/configs. |
| Sources/VersionIcon/Support/Models.swift | Extends ScriptSetup to carry the optional generated asset catalog output path. |
| Sources/VersionIcon/Support/AppSetup.swift | Implements output catalog mode by preparing/syncing a generated .appiconset and using it as the destination folder. |
| Sources/VersionIcon/main.swift | Exposes --outputAssetCatalog CLI option and passes it into ScriptSetup. |
| README.md | Documents the new generated asset catalog workflow and the new CLI parameter. |
| pr_description.md | Adds PR summary notes (currently includes a screenshot placeholder). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…atalog Previously the guard only rejected an output path resolving to the exact same appiconset folder as appIconOriginal, so pointing --outputAssetCatalog at the same tracked .xcassets catalog (under a different appiconset name) would still dirty tracked assets. Also drops the leftover screenshot TODO from pr_description.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
ipek
left a comment
ipek
left a comment
There was a problem hiding this comment.
I'm a little confused, you're adding the option to use xcassets but guard against the option to use AppIcon assets, why? Does dev need to create and maintain separate assets catalog just for generated app icons? How about it just defaults or guides dev to usual app assets? Let's discuss before merging.
There was a problem hiding this comment.
Done — removed pr_description.md in 87a713e. It was PR-only metadata and is no longer part of the repository.
| By default, VersionIcon keeps the historical behavior and writes generated images into the `--appIcon` asset. For new projects, use `--outputAssetCatalog` to keep source assets immutable: | ||
|
|
||
| ```shell | ||
| "Pods/VersionIcon/Bin/VersionIcon" \ |
There was a problem hiding this comment.
Is this Pods path correct? We should be dropping CocoaPods in favour of SPM.
There was a problem hiding this comment.
Updated the new generated-catalog example in 87a713e to use the SPM-neutral VERSION_ICON_BIN and VERSION_ICON_RESOURCES variables. The README now explains that the executable can be built with Swift Package Manager and that the resources live in the package's Bin directory. The repository still contains the legacy CocoaPods installation documentation and podspec for existing users; removing that distribution path is a separate migration decision.
Summary: - Normalize repeated value-bearing command-line flags so the last value wins. - Add regression coverage and rebuild the distributed executable. - Clarify generated asset catalog ownership and use an SPM-neutral README example. - Remove the PR-only description file. Rationale: - The PR branch referenced a missing helper and could not build. - The generated catalog is separate output, while AppIconOriginal remains the source asset. Tests: - swift test - swift build -c release - swiftformat changed Swift files --lint - git diff --check SwiftLint remains blocked by pre-existing repository violations.
|
@ipek Thanks for raising this. The guard is not against using normal AppIcon assets; it prevents --outputAssetCatalog from pointing into the same source .xcassets catalog as AppIconOriginal. VersionIcon still reads AppIconOriginal from the project's usual asset catalog, while it creates and maintains the generated catalog and app-icon sets itself. Developers only need the one-time Xcode wiring to include that output and select the generated app-icon set; the generated files do not need manual maintenance. I kept the option opt-in because existing projects retain their current behavior without it. The README now makes this source/output distinction explicit. Generating into DerivedData by default would be a broader Xcode integration change, so I left that for a separate discussion. |
Why
Previously, VersionIcon wrote generated icons straight into the tracked
--appIconasset catalog. Every configuration switch (e.g. Debug ↔ Release) re-wrote those files, leaving the source assets "dirty" after almost any build and polluting git diffs.What changed
--outputAssetCatalog <path>option. When set, VersionIcon generates the app icon set into a separate, build-only.xcassetscatalog instead of touching the sourceAppIconOriginal-derived assets.Contents.jsonare created/synced automatically fromAppIconOriginalon demand.Bin/VersionIconbinary.Result
Projects that adopt
--outputAssetCatalogno longer see their tracked icon assets modified/marked dirty on every configuration build.I am testing it on Memoaire project: