fix unlocalizable leftovers - #179
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughThe change localizes Finder and macOS interface strings, adds Core localization resources, and replaces archive status messages with typed processing activities. ChangesLocalization and archive status reporting
Estimated code review effort: 3 (Moderate) | ~20 minutes Mergeability Score: ⚪ Minimal · up to The PR expands localization coverage and keeps archive status messages user-facing and localized; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant ArchiveLoader
participant ArchiveEngine
participant ArchiveState
ArchiveLoader->>ArchiveEngine: Report ProcessingActivity
ArchiveEngine->>ArchiveState: Send processing progress and activity
ArchiveState->>ArchiveState: Convert activity to localized status text
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Modules/Sources/Core/ArchiveState.swift`:
- Around line 446-450: Update receiveStatusUpdates so .processing values are not
written directly as raw English statusText; emit semantic status data and
localize it at the UI boundary, preserving diagnostic details separately if
needed. Align ArchiveLoader’s processing emissions (including engine loaded,
entries found, and entry extracted) with this semantic representation, while
keeping the existing localized .done and .error handling.
- Around line 193-194: Update the Core package resources and all localized
status-text calls in ArchiveState, including the password-protected message and
the cited locations, so they resolve through Core’s own resource bundle rather
than bundle: .main. Add or reuse the Core bundle/catalog resource and pass that
bundle to each String(localized:) call, preserving the existing fallback text
and localization comments.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ab625e64-6781-4013-ba5d-4f5a532b215d
📒 Files selected for processing (8)
FinderExtension/FinderSync.swiftFinderExtension/Localizable.xcstringsMacPacker/Core/Sandbox/FolderAccessStore.swiftMacPacker/Core/UrlHandling/AppUrlHandler.swiftMacPacker/Features/Debug/QuickLookHarnessWindowController.swiftMacPacker/Features/Settings/AdvancedSettingsView.swiftMacPacker/Features/Settings/DebugSettingsView.swiftModules/Sources/Core/ArchiveState.swift
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Modules/Package.swift (1)
371-372: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winDeclare the package default localization.
Modules/Package.swiftprocesses a localized Core catalog, but itsPackage(...)declaration has nodefaultLocalization. AdddefaultLocalization: "en"so the package declares its development locale consistently for localized resources. Apple documents this manifest setting for localized package resources. (developer.apple.com)Proposed manifest change
let package = Package( name: "Modules", + defaultLocalization: "en", platforms: [ .macOS(.v14) ],🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Modules/Package.swift` around lines 371 - 372, Update the Package(...) declaration in Modules/Package.swift to include defaultLocalization: "en", preserving the existing localized resource processing and other manifest settings.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Modules/Package.swift`:
- Around line 371-372: Update the Package(...) declaration in
Modules/Package.swift to include defaultLocalization: "en", preserving the
existing localized resource processing and other manifest settings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1fc36b38-6a62-491d-8906-f33b54bf9be0
📒 Files selected for processing (5)
Modules/Package.swiftModules/Sources/Core/ArchiveLoader.swiftModules/Sources/Core/ArchiveState.swiftModules/Sources/Core/Engine/ArchiveEngine.swiftModules/Sources/Core/Localizable.xcstrings
🚧 Files skipped from review as they are similar to previous changes (1)
- Modules/Sources/Core/ArchiveState.swift
|
Hi @pincetgore , it took me a while to go through this PR, simply because it doesn't even compile. Changes that eventually become PRs should compile and all tests should be green 😉 Regardless of that, this uncovered an issue that I will fix as part of this PR. So I will hopefully make the necessary changes soon, now that I finally understood what's happening here. |
CSevenZip declared `path: "."`, making all of Modules/ its resource
territory. SwiftPM auto-discovers .xcstrings anywhere under a target's
path and flattens resources into a single bundle namespace, so the target
silently absorbed ArchivePreviewUI's catalog. Adding a second catalog for
Core made the basenames collide:
error: 'modules': multiple resources named 'Localizable.xcstrings'
in target 'CSevenZip'
Point the target at Sources/CSevenZip and rebase the source list, public
headers path, and header search paths accordingly. Each module now emits
its own resource bundle; CSevenZip emits none.
DebugSettingsView sits behind #if DEBUG and the Quick Look harness is a development tool, so their labels are not meant for translation. Revert both to plain strings and remove the keys that had already been extracted into the app catalog. What stays localized is the user-facing text this branch set out to fix: the Finder "Nothing selected" item (now covering all 14 languages), the file- and folder-access panels, and the Advanced settings export alert. Also moves the "Open…" translator comment into the source, so the catalog entry is regenerated from code rather than drifting with whichever call site was extracted last.
The file- and folder-access panels carried two near-identical strings, so translators saw the same sentence twice. Collapse them into a single "%@ needs access to %@" entry and feed the app name from Constants rather than hard-coding it in the string.
The file panel said "Give access to MacPacker" and the folder panel said "Grant Access" for the same action. Settle on "Grant Access" for both and drop the now-unused key. Also references this PR from the localization changelog entry.
|
@pincetgore . I have merged the PR now. I have removed a couple of translations, though. They are not customer facing but only for debugging / development purposes. I don't want this to be translated. |
|
Thank you @sarensw I really appreciate your support and review! |
2 participants
After deeper investigation I found some leftovers
Summary
This branch improves localization coverage for user-visible strings and removes raw diagnostic messages from archive operation status text.
Changes
Finder extension
Nothing selectedPermission dialogs
Localized the user-facing text in file and folder access panels:
Settings, debug tools, and log export
Localized previously hard-coded UI strings in:
ArchiveState localization
CoreLocalizable.xcstringscatalog.String(localized:)calls inArchiveStateto usebundle: .moduleinstead ofbundle: .main, so Core resolves its strings from its own resource bundle.Semantic archive-loader status updates
EngineStatus.processingtext messages with semanticProcessingActivityvalues.ArchiveLoaderto emit structured activities for:ArchiveState.receiveStatusUpdatesto map semantic activities to localized status text at the UI boundary:loading...;building tree.....doneand.errorstatus handling.Validation
EngineStatus.processingemissions remain in Core.git diff --checksuccessfully.A complete SwiftPM build remains blocked by missing vendored 7-Zip submodule source files in the current checkout.
Summary by CodeRabbit
Localization
Bug Fixes