Conversation
- Extract ~160 user-facing strings to Localizable.strings - SwiftUI Text() auto-localizes via standard .lproj lookup - Notification strings wrapped in String(localized:) - Language picker in Settings > General with restart prompt - Makefile copies .lproj dirs into app bundle - Info.plist declares CFBundleLocalizations Closes #23
|
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: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds app localizations (en, es, fr): declares them in Info.plist, includes .lproj resource directories in the bundle, adds three Localizable.strings files, and localizes UI/notification/helper strings plus a language picker with restart flow in Settings. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 6
🧹 Nitpick comments (1)
Sources/SettingsView.swift (1)
1832-1837: Restart process uses shell command — works but fragile.The shell command handles paths with spaces via quoting. Consider using
Processdirectly withopenas the executable to avoid shell parsing edge cases.More robust alternative
Button(String(localized: "Restart Now")) { let path = Bundle.main.bundlePath - let task = Process() - task.launchPath = "/bin/sh" - task.arguments = ["-c", "sleep 1 && open \"\(path)\""] - task.launch() + DispatchQueue.main.asyncAfter(deadline: .now() + 1) { + NSWorkspace.shared.open(URL(fileURLWithPath: path)) + } NSApp.terminate(nil) }Note: The async approach won't survive
terminate. The current shell approach is valid for this use case.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SettingsView.swift` around lines 1832 - 1837, Replace the shell-invoked Process that sets launchPath = "/bin/sh" and arguments = ["-c", "sleep 1 && open \"\(path)\""] with a direct Process invocation of the open executable to avoid shell parsing: create a Process (the existing task variable), set its executableURL/launchPath to "/usr/bin/open" and its arguments to [path], start it (task.run() or task.launch()) and then call NSApp.terminate(nil); also wrap run/launch in a try/catch or handle errors so failures to launch open are logged instead of crashing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Resources/en.lproj/Localizable.strings`:
- Around line 17-18: Remove the two unused localization entries from
Localizable.strings: delete the lines for the keys "+%lld more" and "+ %lld
more" (the exact string keys "+%lld more" and "+ %lld more" in
Resources/en.lproj/Localizable.strings) so they are no longer present in the
file; ensure you run a quick grep/scan to confirm no remaining references before
committing.
In `@Sources/NotificationManager.swift`:
- Around line 154-156: The localization currently embeds the network name
directly into the key (the assignment to body using newNetwork!), causing unique
keys per network and breaking localization; change to use a stable localized
format string (e.g., "Switched to %@. Checking VPN status..." or a localized
string with a placeholder) and then inject newNetwork via String(format:) or
String.localizedStringWithFormat when computing body (referencing the body
variable and newNetwork) so the localization key remains constant and only the
placeholder is replaced at runtime.
- Around line 109-114: The localized strings use interpolated values which won't
match the .strings format keys; update the construction of suffix and body in
NotificationManager.swift to use format-style localization keys and pass the
values as arguments (e.g., use "%lld" for routesRemaining and "%@" for
wasInterface) instead of string interpolation—replace the interpolated literals
for suffix and body with format keys and supply routesRemaining and wasInterface
(or suffix) as parameters to the localized formatting API used (e.g.,
String(localized:/*format key*/, routesRemaining) or String(format:
NSLocalizedString(/*key*/, comment: ""), ...)) so keys match the .strings
entries and avoid force-unwrapped interpolation in body.
- Around line 129-131: The current code builds localized strings by
interpolating variables inside the localization key (String(localized: "\(count)
route(s) applied successfully.")), which prevents proper
localization/pluralization; change to use a format-style localized string with
arguments (e.g., String(localized: "%d route(s) applied successfully.", count)
and similarly for the failedCount part) or use distinct localized keys that
accept numeric arguments so the localization system receives a stable key and
the count as a parameter; update the occurrences that assign to body (and the
concatenation with failedCount) to use these format-argument localized calls
(refer to the body variable and the count/failedCount usages).
- Around line 309-310: The localization uses an interpolated Swift string for
the notification body which prevents using a proper localization key and
pluralization; update the body to use a localized format with a placeholder and
pass updatedCount as an argument (e.g. replace body: String(localized:
"\(updatedCount) route(s) updated") with a format-based localization call), and
ensure the title and body in NotificationManager.swift use proper localization
keys/placeholders (title: String(localized: "DNS Refresh Complete") can remain
if it's a key) so the translator can provide correct strings and plurals for
updatedCount.
In `@Sources/SettingsView.swift`:
- Around line 1830-1842: The alert message string is not localized; update the
Text(...) inside the .alert tied to showingRestartAlert in SettingsView to use
the app's localization utilities (e.g., wrap the message with String(localized:)
or use a LocalizedStringKey) so it matches the localized title and buttons
(refer to the .alert block that contains the "Restart Required" title and
"Restart Now"/"Later" buttons).
---
Nitpick comments:
In `@Sources/SettingsView.swift`:
- Around line 1832-1837: Replace the shell-invoked Process that sets launchPath
= "/bin/sh" and arguments = ["-c", "sleep 1 && open \"\(path)\""] with a direct
Process invocation of the open executable to avoid shell parsing: create a
Process (the existing task variable), set its executableURL/launchPath to
"/usr/bin/open" and its arguments to [path], start it (task.run() or
task.launch()) and then call NSApp.terminate(nil); also wrap run/launch in a
try/catch or handle errors so failures to launch open are logged instead of
crashing.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 85f1cc49-06c5-4345-a31f-418c777f970c
📒 Files selected for processing (7)
Info.plistMakefileResources/en.lproj/Localizable.stringsResources/es.lproj/Localizable.stringsResources/fr.lproj/Localizable.stringsSources/NotificationManager.swiftSources/SettingsView.swift
Comment on lines
+17
to
+18
| "+%lld more" = "+%lld more"; | ||
| "+ %lld more" = "+ %lld more"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check usage of both "+%lld more" variants in the codebase
rg -n '"\+%lld more"' --type swift
rg -n '"\+ %lld more"' --type swiftRepository: GeiserX/VPN-Bypass
Length of output: 44
Remove unused localization keys at lines 17-18.
Both "+%lld more" and "+ %lld more" are not referenced anywhere in the codebase and should be removed from Localizable.strings.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Resources/en.lproj/Localizable.strings` around lines 17 - 18, Remove the two
unused localization entries from Localizable.strings: delete the lines for the
keys "+%lld more" and "+ %lld more" (the exact string keys "+%lld more" and "+
%lld more" in Resources/en.lproj/Localizable.strings) so they are no longer
present in the file; ensure you run a quick grep/scan to confirm no remaining
references before committing.
Comment on lines
109
to
+114
| let suffix = routesRemaining > 0 | ||
| ? "\(routesRemaining) route(s) could not be removed." | ||
| : "Routes cleared." | ||
| ? String(localized: "\(routesRemaining) route(s) could not be removed.") | ||
| : String(localized: "Routes cleared.") | ||
| let body = wasInterface != nil | ||
| ? "Disconnected from \(wasInterface!). \(suffix)" | ||
| : "VPN connection lost. \(suffix)" | ||
| ? String(localized: "Disconnected from \(wasInterface!). \(suffix)") | ||
| : String(localized: "VPN connection lost. \(suffix)") |
There was a problem hiding this comment.
Same localization key mismatch issue.
All these interpolated strings won't match .strings keys:
- Line 110:
"\(routesRemaining) route(s)..."vs"%lld route(s)..." - Line 113:
"Disconnected from \(wasInterface!)..."vs"Disconnected from %@..." - Line 114:
"VPN connection lost. \(suffix)"vs"VPN connection lost. %@"
Proposed fix
let suffix = routesRemaining > 0
- ? String(localized: "\(routesRemaining) route(s) could not be removed.")
- : String(localized: "Routes cleared.")
+ ? String(format: String(localized: "%lld route(s) could not be removed."), routesRemaining)
+ : String(localized: "Routes cleared.")
let body = wasInterface != nil
- ? String(localized: "Disconnected from \(wasInterface!). \(suffix)")
- : String(localized: "VPN connection lost. \(suffix)")
+ ? String(format: String(localized: "Disconnected from %@. %@"), wasInterface!, suffix)
+ : String(format: String(localized: "VPN connection lost. %@"), suffix)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let suffix = routesRemaining > 0 | |
| ? "\(routesRemaining) route(s) could not be removed." | |
| : "Routes cleared." | |
| ? String(localized: "\(routesRemaining) route(s) could not be removed.") | |
| : String(localized: "Routes cleared.") | |
| let body = wasInterface != nil | |
| ? "Disconnected from \(wasInterface!). \(suffix)" | |
| : "VPN connection lost. \(suffix)" | |
| ? String(localized: "Disconnected from \(wasInterface!). \(suffix)") | |
| : String(localized: "VPN connection lost. \(suffix)") | |
| let suffix = routesRemaining > 0 | |
| ? String(format: String(localized: "%lld route(s) could not be removed."), routesRemaining) | |
| : String(localized: "Routes cleared.") | |
| let body = wasInterface != nil | |
| ? String(format: String(localized: "Disconnected from %@. %@"), wasInterface!, suffix) | |
| : String(format: String(localized: "VPN connection lost. %@"), suffix) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/NotificationManager.swift` around lines 109 - 114, The localized
strings use interpolated values which won't match the .strings format keys;
update the construction of suffix and body in NotificationManager.swift to use
format-style localization keys and pass the values as arguments (e.g., use
"%lld" for routesRemaining and "%@" for wasInterface) instead of string
interpolation—replace the interpolated literals for suffix and body with format
keys and supply routesRemaining and wasInterface (or suffix) as parameters to
the localized formatting API used (e.g., String(localized:/*format key*/,
routesRemaining) or String(format: NSLocalizedString(/*key*/, comment: ""),
...)) so keys match the .strings entries and avoid force-unwrapped interpolation
in body.
Comment on lines
+129
to
+131
| var body = String(localized: "\(count) route(s) applied successfully.") | ||
| if failedCount > 0 { | ||
| body += " \(failedCount) failed." | ||
| body += " " + String(localized: "\(failedCount) failed.") |
There was a problem hiding this comment.
Localization key mismatch continues here.
Proposed fix
- var body = String(localized: "\(count) route(s) applied successfully.")
+ var body = String(format: String(localized: "%lld route(s) applied successfully."), count)
if failedCount > 0 {
- body += " " + String(localized: "\(failedCount) failed.")
+ body += " " + String(format: String(localized: "%lld failed."), failedCount)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/NotificationManager.swift` around lines 129 - 131, The current code
builds localized strings by interpolating variables inside the localization key
(String(localized: "\(count) route(s) applied successfully.")), which prevents
proper localization/pluralization; change to use a format-style localized string
with arguments (e.g., String(localized: "%d route(s) applied successfully.",
count) and similarly for the failedCount part) or use distinct localized keys
that accept numeric arguments so the localization system receives a stable key
and the count as a parameter; update the occurrences that assign to body (and
the concatenation with failedCount) to use these format-argument localized calls
(refer to the body variable and the count/failedCount usages).
Comment on lines
+154
to
+156
| let body = newNetwork != nil | ||
| ? String(localized: "Switched to \(newNetwork!). Checking VPN status...") | ||
| : String(localized: "Network changed. Checking VPN status...") |
There was a problem hiding this comment.
Localization key mismatch.
Proposed fix
let body = newNetwork != nil
- ? String(localized: "Switched to \(newNetwork!). Checking VPN status...")
- : String(localized: "Network changed. Checking VPN status...")
+ ? String(format: String(localized: "Switched to %@. Checking VPN status..."), newNetwork!)
+ : String(localized: "Network changed. Checking VPN status...")🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/NotificationManager.swift` around lines 154 - 156, The localization
currently embeds the network name directly into the key (the assignment to body
using newNetwork!), causing unique keys per network and breaking localization;
change to use a stable localized format string (e.g., "Switched to %@. Checking
VPN status..." or a localized string with a placeholder) and then inject
newNetwork via String(format:) or String.localizedStringWithFormat when
computing body (referencing the body variable and newNetwork) so the
localization key remains constant and only the placeholder is replaced at
runtime.
Comment on lines
+309
to
+310
| title: String(localized: "DNS Refresh Complete"), | ||
| body: String(localized: "\(updatedCount) route(s) updated"), |
There was a problem hiding this comment.
Localization key mismatch.
Proposed fix
- title: String(localized: "DNS Refresh Complete"),
- body: String(localized: "\(updatedCount) route(s) updated"),
+ title: String(localized: "DNS Refresh Complete"),
+ body: String(format: String(localized: "%lld route(s) updated"), updatedCount),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/NotificationManager.swift` around lines 309 - 310, The localization
uses an interpolated Swift string for the notification body which prevents using
a proper localization key and pluralization; update the body to use a localized
format with a placeholder and pass updatedCount as an argument (e.g. replace
body: String(localized: "\(updatedCount) route(s) updated") with a format-based
localization call), and ensure the title and body in NotificationManager.swift
use proper localization keys/placeholders (title: String(localized: "DNS Refresh
Complete") can remain if it's a key) so the translator can provide correct
strings and plurals for updatedCount.
Comment on lines
+1830
to
+1842
| .alert(String(localized: "Restart Required"), isPresented: $showingRestartAlert) { | ||
| Button(String(localized: "Restart Now")) { | ||
| let path = Bundle.main.bundlePath | ||
| let task = Process() | ||
| task.launchPath = "/bin/sh" | ||
| task.arguments = ["-c", "sleep 1 && open \"\(path)\""] | ||
| task.launch() | ||
| NSApp.terminate(nil) | ||
| } | ||
| Button(String(localized: "Later"), role: .cancel) { } | ||
| } message: { | ||
| Text("The app needs to restart to apply the language change.") | ||
| } |
There was a problem hiding this comment.
Alert message not localized.
Line 1841 uses a raw string while the title and buttons use String(localized:).
Proposed fix
} message: {
- Text("The app needs to restart to apply the language change.")
+ Text(String(localized: "The app needs to restart to apply the language change."))
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| .alert(String(localized: "Restart Required"), isPresented: $showingRestartAlert) { | |
| Button(String(localized: "Restart Now")) { | |
| let path = Bundle.main.bundlePath | |
| let task = Process() | |
| task.launchPath = "/bin/sh" | |
| task.arguments = ["-c", "sleep 1 && open \"\(path)\""] | |
| task.launch() | |
| NSApp.terminate(nil) | |
| } | |
| Button(String(localized: "Later"), role: .cancel) { } | |
| } message: { | |
| Text("The app needs to restart to apply the language change.") | |
| } | |
| .alert(String(localized: "Restart Required"), isPresented: $showingRestartAlert) { | |
| Button(String(localized: "Restart Now")) { | |
| let path = Bundle.main.bundlePath | |
| let task = Process() | |
| task.launchPath = "/bin/sh" | |
| task.arguments = ["-c", "sleep 1 && open \"\(path)\""] | |
| task.launch() | |
| NSApp.terminate(nil) | |
| } | |
| Button(String(localized: "Later"), role: .cancel) { } | |
| } message: { | |
| Text(String(localized: "The app needs to restart to apply the language change.")) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/SettingsView.swift` around lines 1830 - 1842, The alert message
string is not localized; update the Text(...) inside the .alert tied to
showingRestartAlert in SettingsView to use the app's localization utilities
(e.g., wrap the message with String(localized:) or use a LocalizedStringKey) so
it matches the localized title and buttons (refer to the .alert block that
contains the "Restart Required" title and "Restart Now"/"Later" buttons).
- Change SettingsCard, SettingsToggleRow, StatusRow, NotificationChip, TabItem, RoutingModeButton to accept LocalizedStringKey instead of String, so SwiftUI auto-localization works through wrapper components - Fix MenuBarViews modeButton and StatBadge with same pattern - Fix ternary expressions in MenuBarViews that erased literals to String - Use String(localized:) for helperStateSubtitle/helperActionLabel - Fix language picker to use dedicated UserLanguageOverride key instead of reading AppleLanguages (which contains region codes like en-US) - Add localization resources to Package.swift so SwiftPM builds include .lproj directories
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Sources/MenuBarViews.swift (1)
342-343:⚠️ Potential issue | 🟡 MinorIncomplete localization coverage in this file.
Several UI strings remain hardcoded while others are localized. Examples: "Refresh Routes" (342), "Clear" (361), "Setting Up..." (414), "No VPN Connection" (435), "Active Services" (488), "routes" (249).
Consider localizing remaining user-visible strings for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/MenuBarViews.swift` around lines 342 - 343, Several user-facing strings in MenuBarViews.swift are hardcoded (e.g., Text("Refresh Routes"), "Clear", "Setting Up...", "No VPN Connection", "Active Services", "routes"); replace these literals with localized keys using LocalizedStringKey or NSLocalizedString (for example use Text(LocalizedStringKey("Refresh_Routes")) or Text(NSLocalizedString("Refresh_Routes", comment: ""))) and add corresponding entries to Localizable.strings; update any SwiftUI Text initializers (e.g., the Text instances and string interpolations in the view structs inside MenuBarViews.swift) to use the same keys so all visible strings are covered consistently.
🧹 Nitpick comments (1)
Sources/SettingsView.swift (1)
1834-1839: Shell command with interpolated path is fragile.If the bundle path contains quotes or shell metacharacters, this command could fail or behave unexpectedly. Consider using
NSWorkspacefor a more robust restart.Safer restart approach
- Button(String(localized: "Restart Now")) { - let path = Bundle.main.bundlePath - let task = Process() - task.launchPath = "/bin/sh" - task.arguments = ["-c", "sleep 1 && open \"\(path)\""] - task.launch() - NSApp.terminate(nil) - } + Button(String(localized: "Restart Now")) { + let bundleURL = Bundle.main.bundleURL + DispatchQueue.main.asyncAfter(deadline: .now() + 0.5) { + NSWorkspace.shared.openApplication( + at: bundleURL, + configuration: NSWorkspace.OpenConfiguration() + ) { _, _ in } + } + DispatchQueue.main.asyncAfter(deadline: .now() + 0.6) { + NSApp.terminate(nil) + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SettingsView.swift` around lines 1834 - 1839, The current code builds and runs a shell command using an interpolated bundle path (variables: path, task, task.arguments) which is fragile; replace the Process + "/bin/sh" invocation with NSWorkspace to open the bundle path safely: construct a URL from Bundle.main.bundlePath using URL(fileURLWithPath: Bundle.main.bundlePath) and call NSWorkspace.shared.open(...) to open the app bundle (or use NSWorkspace.shared.launchApplication(at:options:configuration:) if relaunch behavior is needed), then call NSApp.terminate(nil); remove the shell invocation and related Process setup (task.launchPath, task.arguments, task.launch()) so you rely on Cocoa APIs instead of a quoted shell string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@Sources/MenuBarViews.swift`:
- Around line 342-343: Several user-facing strings in MenuBarViews.swift are
hardcoded (e.g., Text("Refresh Routes"), "Clear", "Setting Up...", "No VPN
Connection", "Active Services", "routes"); replace these literals with localized
keys using LocalizedStringKey or NSLocalizedString (for example use
Text(LocalizedStringKey("Refresh_Routes")) or
Text(NSLocalizedString("Refresh_Routes", comment: ""))) and add corresponding
entries to Localizable.strings; update any SwiftUI Text initializers (e.g., the
Text instances and string interpolations in the view structs inside
MenuBarViews.swift) to use the same keys so all visible strings are covered
consistently.
---
Nitpick comments:
In `@Sources/SettingsView.swift`:
- Around line 1834-1839: The current code builds and runs a shell command using
an interpolated bundle path (variables: path, task, task.arguments) which is
fragile; replace the Process + "/bin/sh" invocation with NSWorkspace to open the
bundle path safely: construct a URL from Bundle.main.bundlePath using
URL(fileURLWithPath: Bundle.main.bundlePath) and call
NSWorkspace.shared.open(...) to open the app bundle (or use
NSWorkspace.shared.launchApplication(at:options:configuration:) if relaunch
behavior is needed), then call NSApp.terminate(nil); remove the shell invocation
and related Process setup (task.launchPath, task.arguments, task.launch()) so
you rely on Cocoa APIs instead of a quoted shell string.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: be95c1fa-9ade-484d-a5b8-c8e9cd9af9d7
📒 Files selected for processing (3)
Package.swiftSources/MenuBarViews.swiftSources/SettingsView.swift
- Wrap HelperState.statusText returns with String(localized:) so the helper card title line localizes - Change LinkButton, LinkRow, RouteStatCard to accept LocalizedStringKey - Add new helper status keys to all three .strings files
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Sources/HelperManager.swift`:
- Around line 23-28: The status text currently localizes wrapper strings but
leaves the underlying error messages and several wrapper phrases in English;
update all places that construct the .failed(...) case and the wrapper strings
("Helper not ready (...)") to use the localization API (e.g., wrap user-facing
literals with String(localized: "...") or the project's localization helper) so
the entire message is localized. Specifically, change the code paths that create
HelperManager.Status.failed(...) (the four .failed(...) call sites referenced in
the review) to pass localized error messages instead of raw English strings, and
replace the hard-coded wrapper strings around statusText (the "Helper not ready
(...)" occurrences) with localized equivalents so the final composed strings are
fully localized.
In `@Sources/SettingsView.swift`:
- Around line 1086-1093: The Picker currently uses an empty label (Picker("",
selection: $selectedLanguage)) which is inaccessible to VoiceOver; update the
Picker to provide an explicit accessibility label by replacing the empty label
with a descriptive Text (e.g., Text("Language")) or by keeping the existing
empty label and adding .accessibilityLabel("Language") to the Picker bound to
$selectedLanguage so screen readers can announce the control; ensure the chosen
label matches the visible sibling label and update any localized strings if
needed.
- Around line 1833-1839: The Restart button currently interpolates
Bundle.main.bundlePath into a shell command and calls task.launch() then
NSApp.terminate(nil); change it to invoke /usr/bin/open directly as the
Process.launchPath and pass the bundle path as a separate argument (do not use
"/bin/sh -c" or string interpolation), and only terminate the app after the
Process successfully starts (use Process.run()/start with error handling or
check for thrown errors from run())—update the code around the Button action
that defines path, task, task.launchPath, task.arguments, task.launch(), and
NSApp.terminate(nil) accordingly.
🪄 Autofix (Beta)
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f6fccce3-abef-46a0-93aa-f4605d256a95
📒 Files selected for processing (5)
Resources/en.lproj/Localizable.stringsResources/es.lproj/Localizable.stringsResources/fr.lproj/Localizable.stringsSources/HelperManager.swiftSources/SettingsView.swift
✅ Files skipped from review due to trivial changes (3)
- Resources/fr.lproj/Localizable.strings
- Resources/es.lproj/Localizable.strings
- Resources/en.lproj/Localizable.strings
Comment on lines
+23
to
+28
| case .missing: return String(localized: "Not Installed") | ||
| case .checking: return String(localized: "Checking...") | ||
| case .installing: return String(localized: "Installing...") | ||
| case .outdated(let installed, let expected): return String(localized: "Update Required (v\(installed) → v\(expected))") | ||
| case .ready: return String(localized: "Helper Installed") | ||
| case .failed(let msg): return String(localized: "Error: \(msg)") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the file structure and the lines in question
wc -l Sources/HelperManager.swiftRepository: GeiserX/VPN-Bypass
Length of output: 93
🏁 Script executed:
# Read the relevant sections of the file
# Lines 23-28 (the changed code)
sed -n '20,30p' Sources/HelperManager.swift
# Lines around 326, 352, 380, 416, 446 (usage sites)
echo "=== Line 326 context ===" && sed -n '320,330p' Sources/HelperManager.swift
echo "=== Line 352 context ===" && sed -n '346,356p' Sources/HelperManager.swift
echo "=== Line 380 context ===" && sed -n '374,384p' Sources/HelperManager.swift
echo "=== Line 416 context ===" && sed -n '410,420p' Sources/HelperManager.swift
echo "=== Line 446 context ===" && sed -n '440,450p' Sources/HelperManager.swift
# Lines around 88, 114, 133, 146 (where .failed is called)
echo "=== Line 88 context ===" && sed -n '82,92p' Sources/HelperManager.swift
echo "=== Line 114 context ===" && sed -n '108,118p' Sources/HelperManager.swift
echo "=== Line 133 context ===" && sed -n '127,137p' Sources/HelperManager.swift
echo "=== Line 146 context ===" && sed -n '140,150p' Sources/HelperManager.swiftRepository: GeiserX/VPN-Bypass
Length of output: 4717
🏁 Script executed:
# Also check the beginning of the file to understand the HelperState type definition
head -50 Sources/HelperManager.swiftRepository: GeiserX/VPN-Bypass
Length of output: 1629
Localization is incomplete and will create mixed-language output.
The statusText now localizes the "Error:" wrapper, but the error messages passed to .failed() remain English (lines 88, 114, 133, 146). Additionally, the wrapper strings like "Helper not ready (...)" at lines 326, 352, 380, 416, and 446 are still English. Users will see partial translations like Helper not ready (Error: Installation failed) instead of fully localized messages.
Localize the error messages passed to .failed() and the wrapper strings to provide consistent user-facing text.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Sources/HelperManager.swift` around lines 23 - 28, The status text currently
localizes wrapper strings but leaves the underlying error messages and several
wrapper phrases in English; update all places that construct the .failed(...)
case and the wrapper strings ("Helper not ready (...)") to use the localization
API (e.g., wrap user-facing literals with String(localized: "...") or the
project's localization helper) so the entire message is localized. Specifically,
change the code paths that create HelperManager.Status.failed(...) (the four
.failed(...) call sites referenced in the review) to pass localized error
messages instead of raw English strings, and replace the hard-coded wrapper
strings around statusText (the "Helper not ready (...)" occurrences) with
localized equivalents so the final composed strings are fully localized.
- Add accessible label to language Picker with .labelsHidden() - Replace /bin/sh -c shell interpolation with direct /usr/bin/open -n invocation for app restart, avoiding shell parsing edge cases
There was a problem hiding this comment.
♻️ Duplicate comments (1)
Sources/SettingsView.swift (1)
1847-1849:⚠️ Potential issue | 🟡 MinorAlert message not localized.
Title and buttons use
String(localized:), but the message body is still a raw string.Proposed fix
} message: { - Text("The app needs to restart to apply the language change.") + Text(String(localized: "The app needs to restart to apply the language change.")) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SettingsView.swift` around lines 1847 - 1849, The alert message in SettingsView is a raw string; replace the Text("The app needs to restart to apply the language change.") with a localized version using the same localization approach as the title/buttons (e.g., Text(String(localized: "The app needs to restart to apply the language change."))) so the message is localized; update the string key as appropriate in your Localizable resources and keep this change near the alert's message closure in SettingsView.
🧹 Nitpick comments (1)
Sources/SettingsView.swift (1)
1839-1844: Consider logging restart failure.Error is caught but silently discarded. The UX is acceptable (app stays open), but logging could aid debugging.
Optional enhancement
} catch { - // Fall through — app stays open + // Fall through — app stays open + print("Failed to restart app: \(error)") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Sources/SettingsView.swift` around lines 1839 - 1844, The catch block after attempting to restart the app (the do { try task.run(); NSApp.terminate(nil) } catch { ... }) currently swallows errors silently; update the catch to log the thrown error (e.g., via os_log/NSLog or the app's logger) with context like "Failed to run restart task" so failures to run task.run() or terminate can be diagnosed; keep the UX unchanged (app stays open) but ensure the error and any relevant task/command info are included in the log message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@Sources/SettingsView.swift`:
- Around line 1847-1849: The alert message in SettingsView is a raw string;
replace the Text("The app needs to restart to apply the language change.") with
a localized version using the same localization approach as the title/buttons
(e.g., Text(String(localized: "The app needs to restart to apply the language
change."))) so the message is localized; update the string key as appropriate in
your Localizable resources and keep this change near the alert's message closure
in SettingsView.
---
Nitpick comments:
In `@Sources/SettingsView.swift`:
- Around line 1839-1844: The catch block after attempting to restart the app
(the do { try task.run(); NSApp.terminate(nil) } catch { ... }) currently
swallows errors silently; update the catch to log the thrown error (e.g., via
os_log/NSLog or the app's logger) with context like "Failed to run restart task"
so failures to run task.run() or terminate can be diagnosed; keep the UX
unchanged (app stays open) but ensure the error and any relevant task/command
info are included in the log message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 65a1cd16-635b-438b-bed0-4f5edfa74956
📒 Files selected for processing (1)
Sources/SettingsView.swift
The .failed() messages flow through statusText's "Error: %@" into the helper card, so Spanish/French users would see mixed-language text like "Erreur : Cannot connect to helper after reinstall".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Text()calls auto-localize via standard.lproj/Localizable.stringslookup — no code changes needed for most UI stringsString(localized:).lprojdirs into the app bundle;Info.plistdeclaresCFBundleLocalizationsCloses #23
How it works
.lprojResources/en.lproj/Localizable.strings→Resources/<code>.lproj/Localizable.strings, translate, and add the code toInfo.plistCFBundleLocalizations+ the picker inSettingsView.swiftFor reviewers
Resources/fr.lproj/Localizable.stringsfor French accuracyTest plan
Summary by CodeRabbit
New Features
Chores