Repository navigation
i18n: extract all user-facing UI strings to resources (umbrella) - #1196
torlando-tech wants to merge 31 commits into
Conversation
- New detekt rule flags hardcoded user-facing strings at UI sinks (Text/labels/contentDescription, Toast/Snackbar, notification builders and channels, clipboard labels, enum display fields). Sink table shared with the ticket-23 equivalence gate lives in UiSinkTable.kt. - Fix BleLoggingTagRule: package pattern pointed at dead com.lxmf.messenger.reticulum.ble.*; retargeted to the real network.columba.app.rns.host.ble (rule was inert; its tests fail on main once CI actually runs them). - CI: run :detekt-rules:test in the module-tests job; add a NoHardcodedUiString suppression ratchet step. - :app debug builds: enable pseudoLocales for truncation smoke. - Regenerate detekt baselines: 1803 app + 20 rns-host + 1 data + 1 rns-backend-py NoHardcodedUiString entries form the machine work list for the migration tickets; stale entries pruned. - Delete orphaned app/detekt-baseline-debug.xml (no lint config references it). Verified: :detekt-rules:test 67/67 green; full detekt green; negative test (new Text literal) fails :app:detekt as required.
…8n ticket 23)
Machine proof, on every PR forever, that no rendered UI text changes
unless the PR carries the 'string-changes-expected' label.
- Dumper CLI in :detekt-rules (dump/UiTextDumper.kt + ResResolver.kt +
Main.kt) reusing UiSinkTable, so it guards exactly what
NoHardcodedUiString guards. Normalization makes literal and resource
spellings compare equal: interpolation and %s/%1$s specs -> {},
stringResource/getString resolve through the owning module's
res/values XML (nonTransitiveRClass package mapping), plurals emit
one+other, +-chains join, opaque expressions emit {} (stable under
renames). @Preview/test exemptions match the rule.
- Gradle task :detekt-rules:uiTextDump (-PdumpRoot/-PdumpOut/
-PdumpFiles); JavaExec runs from repo root.
- CI job ui-text-equivalence: dumps base worktree and head restricted
to changed main-source .kt files; any diff fails with the diff in the
log; label bypasses. Dumper always runs from the head checkout, so
base trees predating the task still work.
- 11 dumper tests: extraction equivalence, plurals, concatenation,
cross-module R, getString, previews, assignments, enum fields,
ordinals, opaque leaves.
Verified end-to-end on PropagationNodesScreen.kt: renaming one word in
a Text( produced a one-line diff; pure extraction (literal -> identical
stringResource) produced a byte-identical dump; deleting a sink
produced a count-mismatch diff. ktlintCheck + detekt green.
Replace string-equality control flow with typed signals so display text can move to resources without breaking behavior: - BleGattServer: keepalive loop now breaks on NoConnectedCentralsException (new typed failure from notifyCentrals) instead of message sniffing. - CallViewModel: call-initiate retry triggers on new RnsError.CallManagerNotInitialized (tag 9) thrown by NativeRnsBackendImpl, not on a message substring; +retry tests. - PyxisFlashCore: RNode stage suppression keys off new ESPToolFlasher.FlashStage enum passed through ProgressCallback, not progress message text; flashRegion derives log name from stage. - RNodeWizardViewModel: pairing-state comparisons use new UsbPairingPhase enum alongside the display string; extracted requestUsbPermissionAndPair helper. No user-visible text changed (ticket 23 gate).
Verified-then-delete per ticket 03: - AnnounceStreamViewModel._initializationStatus: StateFlow had no UI consumer; deleted field + 5 assignments. Readiness tests now verify observeAnnounces start/no-start instead of pinning status strings. - BlePermissionManager.getPermissionRationale: no callers (UI uses BlePermissionBottomSheet's own rationale). - CallViewModel.callStateLabel: already absent from the tree; inventory entry was stale. - ReticulumError: zero references anywhere; deleted. No rendered text changed (ticket 23 gate).
:rns-host gets res/values/strings.xml (134 resources): ServiceNotificationManager channel + ongoing-notification text (channel ids and READY/ERROR: state tokens unchanged), and all flasher progress/error text (ESPToolFlasher, NordicDFUFlasher, RNodeFlasher, TncModeController, FirmwareDownloader, PyxisFlashCore). Flasher contract no longer carries English: new FlashText (Resource(@stringres, args) | Raw) replaces message strings in ProgressCallback/FlashState/DownloadCallback; app render sites resolve via FlashText.asString(). Per-stage region texts map from the FlashStage enum (ticket 02). :rns-api: BatteryProfile displayName/description become @stringres fields; FailedInterface JSON fallbacks move to the consumer (DebugViewModel) with resources. :rns-stats: TrafficSpeedChart default title -> resource. :rns-backend-kt: identity-import error resolved via appContext resource. Baseline: removed the 5 ServiceNotificationManager NoHardcodedUiString entries. Rendered text byte-identical (ticket 23 gate). Tests assert resource ids/args, not English.
…et 05) NotificationHelper (channels, announce/BLE notifications incl. hop-count plurals), CallNotificationHelper (channels, titles, Decline/Answer/End Call actions), NavTab labels -> @stringres labelRes (render sites use stringResource), and the double-back Toast move to strings.xml. Channel locale refresh: createNotificationChannel with an existing id replaces name/description with current-locale text on every app start (user-set importance preserved); both helpers are Hilt singletons built at app start via ColumbaApplication's injection graph. (Note: platform has no NotificationManager.updateNotificationChannel — verified against android-36 android.jar; re-create is the documented mechanism.) Baseline: removed 25 NoHardcodedUiString entries for these files; updated 3 BottomNavigationCard entries to the new labelRes literal. Rendered text byte-identical (ticket 23 gate).
…18n ticket 06)
Six ViewModels' user-facing strings move to resources behind typed reasons
(NewCallInvalidReason template): ContactToggleErrorReason, AddContactErrorReason,
BleConnectionsErrorReason, AnnounceErrorReason + shared UiMessage (Text/Plural)
for flows carrying localized text. e.message stays a %1$s detail arg (policy 4).
Compress-count strings become plurals; shared error_unknown; reply preview
"Message deleted" -> isDeletedPlaceholder flag + resource; currentPeerName
default "Unknown" -> null (never wire). Wire announce display names
("Unknown", "Unknown Peer") stay per ticket 03 policy 1.
Render sites (7 screens + ReplyComponents) resolve via stringResource/resolve().
Tests assert typed reasons, not English. Rendered text byte-identical
(ticket 23 gate). AdvancedCardTest failures are pre-existing on main
(global onNode(isToggleable()) matcher vs multiple Switches since 95b6b26).
RNodeWizardViewModel error/status state fields -> UiMessage (dead usbAssistedPairingStatus deleted); RNodeConfigValidator + DeviceNameValidator emit typed reason enums (RNodeFieldErrorReason, DeviceNameErrorReason) with messageResource() mapping at render sites — the shape util/validation (ticket 10) will adopt. FlasherViewModel/TcpClientWizardViewModel errors -> UiMessage; UiMessage gains Raw for exception pass-through. 87 byte-identical resources; interpolated values became placeholder args. Tests assert reason enums / resource ids, not English (ticket 23 gate).
IdentityManagerViewModel/IdentityUnlockViewModel/MigrationViewModel state messages -> UiMessage; IdentityFileReader/MigrationImporter/Exporter throw typed localized exceptions (IdentityFileInvalidException, MigrationLocalizedException) resolved at render sites. Version-mismatch and decrypt-failure texts become wrapper resources with placeholder args per ticket 03 verdict; crypto internals stay English args (policy 4). 68 byte-identical resources; tests assert UiMessage ids, not English (ticket 23 gate).
Ten ViewModels (NomadNetBrowser, InterfaceManagement, OfflineMapDownload, OfflineMaps, ApkSharing, DiscoveredInterfaces, Map, InterfaceStats, Debug, Onboarding) move user-facing strings to resources via UiMessage/typed enums. BrowserState.Error adopts wrapper-resource + %1$s detail pattern (policy 4); map marker labels separated from wire fallbacks; DebugInfo.error -> UiMessage with Raw pass-through. UiMessage.Text resolves nested UiMessage args. 65 byte-identical resources; 11 test files re-pinned to typed assertions (ticket 23 gate).
TimeUtils relative time -> plurals; InputValidator/DestinationHashValidator adopt typed reason enums + messageResource() (ticket 07 shape); permission rationales -> @stringres; PropagationFailureReason -> Reason enum + Explanation with pf: storage tokens (legacy English decodes to Raw); PropagationNodeManager/LocalHotspotManager/UpdateChecker/InterfaceConfigManager/ PageImageLoader/VoiceMessageRecorder/TileDownloadManager/MapLibreOfflineManager errors -> UiMessage (UiMessageException carries resource ids across exception hops); RNodeRegionalPreset/ImageCompressionPreset/MapStylePreference -> @stringres fields (region names verbatim per policy 6). ~150 byte-identical resources; 53 baseline entries removed (ticket 23 gate).
Render-level before/after proof for all 43 renderable ui/components files: semantics-tree text dump (Text/EditableText/ContentDescription, unmerged, document order) vs goldens/<component>.json; record mode -PrecordTexts (rides JAVA_TOOL_OPTIONS — AGP Robolectric fork drops Test.systemProperty). 14 test classes, 154 tests, 154 goldens captured from pre-migration code; sensitivity proven (reword -> fail -> revert -> green). Ticket 11 must keep these goldens unchanged.
CodecProfile/PresetTheme/SignalQualityInfo/AspectChip/INTERFACE_OPTIONS -> @stringres fields; all renderable ui/components/** Text/label/placeholder/ contentDescription/semantics -> resources (324 new: 316 strings + 8 plurals; hop/person/icon counts as plurals). MaterialDesignIcons keyword lists stay in code per policy. 221 baseline entries removed. All 154 ticket-24 goldens green with UNCHANGED golden files (text invariance proven at render level); quote-escaping bug in bg_location_body/icon_picker_no_results fixed in XML, goldens untouched (ticket 23 gate).
MessagingScreen (73: toasts, stateDescription sink, when-branch status labels, +N more files plural), MessageDetailScreen (when-keys stay, results migrate, hop plurals), ChatsScreen (30+ incl. Draft: prefix, chooser title). Toast prefixes migrated as policy-4 wrappers (Sync failed: %1$s, Failed to attach file: %1$s). 140 byte-identical resources; 106 baseline entries removed (ticket 23 gate).
ContactsScreen (tab-count plurals, toasts, dialogs), AnnounceDetailScreen (hop plurals, aspect when-branches), AnnounceStreamScreen (nodes-in-range plural), BlockedUsersScreen (Blocked %1$s / Unblock %1$s? placeholders), PropagationNodesScreen (hop plural), MapScreen (ClipData labels per policy 2, focus-marker fallback, formatTimeAgo -> context resources preserving the '1 hours ago' quirk under the invariance gate). 201 byte-identical resources (8 plurals); 164 baseline entries removed (ticket 23 gate).
InterfaceManagementScreen (type labels/descriptions via Context, peer/apps plurals), DiscoveredInterfacesScreen (announced-field labels @StringRes-keyed, when-branch results migrate keys stay, distance/hops/last-heard templates), InterfaceStatsScreen (31 badges/labels), UsbDeviceActionScreen (28). InterfaceConfigDialog/IfacConfigCard verified already complete from ticket 11. 212 byte-identical resources (3 plurals); 138 baseline entries removed (ticket 23 gate).
IdentityScreen (InfoRow labels, service-status when-branches, Error: prefix wrapper), IdentityManagerScreen (positional Icon contentDescriptions named, relative-time when-branch), MyIdentityScreen (multi-line bullet help dialogs -> single \n resources, share-text blocks byte-identical), IdentityUnlockScreen (embedded \n\n blocks collapsed), WelcomeScreen, IdentityQrCodeDialog. 201 byte-identical resources; 215 baseline entries removed; relative-time kept as exact strings (plurals would violate invariance gate) (ticket 23 gate).
SettingsScreen (ClipData labels per policy, snackbar/chooser text), crash dialogs (Exception:/Time: wrappers, Settings -> Advanced concat), ThemeEditor, ThemeManagement (escaped-quote template), NotificationSettings, AboutCard (13 positional InfoRow + 3 LinkButton named, proper nouns wrapped, copyright placeholder). 119 byte-identical resources; 123 baseline entries removed (ticket 23 gate).
All settings/cards/** except AboutCard: LocationSharingCard (interval chips format-only kept, telemetry templates), MessageDeliveryRetrievalCard (hop plurals), AutoAnnounceCard (hour(s) plural, Error: wrapper), SharedInstance- BannerCard (bullet blocks), MapSourcesCard (server-count plural), Bottom- NavigationCard (tab.labelRes as %1$s arg), AdvancedCard paragraphs, rest. 209 byte-identical resources (4 plurals incl. identical one/other forms under invariance gate); 245 baseline entries removed (ticket 23 gate).
rnode/** (DeviceDiscoveryStep 54, ReviewConfigStep 24, FrequencySlotStep 17, RNodeWizardScreen step-title when-branches -> @stringres, RegionSelectionStep, ModemPresetStep) + tcpclient/** (ReviewConfigureStep, ServerSelectionStep, TcpClientWizardScreen). MHz/dBm/duty/SF chips and Slot $slot kept format-only per policy; slot/technical tokens listed. 145 byte-identical resources (6 reuses from ticket 07); 126 baseline entries removed (ticket 23 gate).
flasher/**: DeviceDetectionStep (11 positional DeviceInfoRow named), FirmwareSelectionStep, FlashCompleteStep (numbered list -> single resource), RNodeFlasherScreen step/button when-branches -> @stringres, TncConfiguration- Step, FlashProgressStep, DeviceSelectionStep; FlasherStepIndicator StepInfo labels -> @stringres (beyond inventory, rendered text). v$version/VID-PID/MHz chips format-only kept. 129 byte-identical resources (nordic_flash_complete reused from rns-host); 116 baseline entries removed (ticket 23 gate).
OnboardingInterfaceType enum -> @stringres fields; PrivacyFeature textRes; all six pages + pager (crash paragraph concat -> single resource, share text placeholder). 67 byte-identical onboarding_* resources; 49 baseline entries removed. Also fixes LocationShareTextGoldenTest flake: golden fixture now pins addedTimestamp (factory live-clock value made the sheet's recency sort Alice/Bob order non-deterministic) (ticket 23 gate).
OfflineMapDownloadScreen (step when-branches, SummaryRow named, tiles plural), OfflineMapsScreen, NomadNetBrowserScreen (embedded \n blocks), ApkSharingScreen (step template), VoiceCall/IncomingCall (call-state when-branches), QrScannerScreen; plus full leftover sweep: MigrationScreen, BleConnectionStatus- Screen and every remaining flagged file. Baseline 220 -> 4 entries, all four DNT (Sentry breadcrumb + 3 animation labels). +209 resources (9 plurals; n==1 forms fix '1 tiles' grammar where the ticket explicitly requested plurals). Rule gap noted for ticket 22: if/elvis sink args not descended (VoiceCallScreen Mute/Earpiece/TALKING, QrScannerScreen) (ticket 23 gate).
NoHardcodedUiString now descends sink args through if/else, elvis, when-branch results, and sealed supertype constructors (conditions never descended, so wire-key comparisons stay non-sinks); 27 rule tests. Ratified DNT literals move to detekt-config doNotTranslateLiterals (telemetry breadcrumb, animation labels, persisted fallbacks, hardware product names) — zero-tolerance mechanism per ticket 01: baseline AND @Suppress count both zero repo-wide. 21 newly-extracted rule-gap strings (call mute/earpiece/TALKING, qr flash toggles, flasher source names); dead CallViewModel.getStatusText deleted. docs/translation-workflow.md hand-off (first-locale lint checklist, placeholder conventions, pseudo-locale howto). app_name + channel ids verified unchanged (ticket 23 gate).
Committed comments must not reference .scratch/ (local-only issue tracker, not in the repo). UiSinkTable KDoc and detekt-config now cite the CI job name and the committed docs page instead.
[High impact] The latest changes appear safe to merge; no new blocking issue was established. SummaryThis PR moves UI text into Android resources and adds checks to keep it there. The latest update:
No new, separate actionable issue was established. Reviews (7) · Last reviewed commit: "fix(detekt-rules): dump message-carrier ..." · Reviewed by Greptile |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- Sink gate: fetch the PR merge ref (depth 2) so base/head are actually
reachable in the shallow checkout; set -euo pipefail so a failed diff
fails the job instead of vacuously passing with an empty file list.
- Sink gate scope: when any values*/ resource changes, compare all
main-source Kotlin files (resource-only edits no longer bypass the gate).
- NoHardcodedUiString: descend braced branch blocks (final expression of
each { } branch of if/when in sink position); word check now covers
1-2 letter words (No/On/Hi) with technical tokens still exempt. 5 new
rule tests; no new findings on the current tree.
- IdentityManagerViewModel: restart-failure detail passes e.toUiMessage(...)
as the nested arg so UiMessage.Text resolves readable text instead of
Text(id=...) toString.
- rnode_wizard_short_term/long_term: literal single % (formatted=false) —
%% rendered doubled since callers pass no format args (was a text-
invariance violation vs main).
…aught (greploop r2)
Greptile P1: the widened word regex (\p{L}+) flags short static text in
format-only labels (v$version, 1h/1hr, 5 km, % LT, CR) — CI Code Quality
failed on 22 sites. Triage per site:
- technicalTokens += v, h, hr, km, lt, cr (units/abbreviations in
format-only labels; longer strings still flag via their other words).
- doNotTranslateLiterals += '"A"' (font-size preview glyph),
'"$b32Address.b32.i2p"' (I2P destination format).
- Genuine misses the braced-branch descent newly caught: QrScanner
contact-already-added dialog body (2 strings) extracted to resources;
filtered-count 'X of Y' now uses the existing iface_disc_filter_count
resource (was defined but unused).
- New rule tests: version prefix + unit chips exempt; short user-facing
words (No/On) still flagged.
Verified: :detekt-rules:test, :app:detekt --rerun-tasks --no-build-cache
0 findings, compile+resources+ktlint green, QrScanner/DiscoveredInterfaces
tests green.
The sink gate is the machine proof that the strings.xml move preserved rendered text 1:1 (the string-changes-expected label acknowledges the diff is non-empty; the diff itself is the artifact). Running it exposed four dumper bugs manufacturing false diffs, plus two blind spots: - ResResolver: \n/\t/\r now unescape to control chars (were literal 'n'/'t'/'r'); values wrapped in quotes have the quotes stripped per aapt semantics. - normalizeText: collapse %% to % in format strings (Android does at format time; Kotlin literals never contain %%). - FORMAT_SPEC_REGEX: drop space from the flag class — '% duty' is prose, not the Java space-flag specifier. - Dumper: resolve *Res-twin sink names (descriptionRes = R.string.x is the resource twin of description = "x"; 184 enum/constructor sites were invisible), named enum entry args, import-aliased cross-module R (RnsHostR.string.x), local vals holding resources, and format-argument substitution (Step %1\$d: %2\$s with (1, "…") now dumps the rendered text — swapped args would now be caught). Effect on the base-vs-head gate diff for this PR: 36 same-sink text diffs -> 8 (all verified artifacts: 's' suffix moved from Kotlin expr into resource, plural-vs-literal kind suffixes, new visibility); lost entries 184 -> 77 (remaining are positional->named kind renames and plurals); 357 entries gained are new visibility (ViewModel UiMessage sites the old dumper never saw). 12 new dumper tests.
Sink-gate diff reviewed: string move verified 1:1The Result: no rendered-text regressions. The only genuine text bug the migration ever produced ( Running the gate also exposed four bugs in the gate itself (fixed in After the fixes, the remaining base→head diff falls into four verified-harmless classes:
Spot-checked the risky classes by hand: every Follow-up ( |
…p r4) - Plural callers: the quantity selector (pluralStringResource(id, qty, *args) / getQuantityString(resId, qty, *args)) is not a format argument; substituting it shifted every text arg one place so a text change (Alice->Bob) produced no diff. - Local vals: a second, conflicting binding for the same name (shadowed val in a nested block) now poisons the entry to opaque instead of comparing against the wrong binding. - 3 regression tests (distinct count/text args, shadowed val, single val still resolves).
Resolve conflicts from 29 main commits (Sentry memory diagnostics #1213, unverified-sender warning #876, migration dispatcher #1205, Kotlin 2.4.21 toolchain, ktlint 14.2.0, and dependabot bumps): - CrashReportingPage/AdvancedCard/OptInDialog: take main's new crash_reporting_privacy_description + crash_reporting_title (the text changed to describe the new memory-diagnostics feature); drop the now -unused onboarding_crash_*/settings_advanced_crash_*/settings_crash_optin_body extractions this branch had added for the old wording. - MigrationImporter: keep main's MemoryDiagnostics try/finally wrapper and re-apply the UiMessage.Text extraction to the two ImportResult.Error sites inside it (Error now carries a UiMessage). - ImageCompressionPreset: keep displayNameRes + main's #1209 8K-cap comment. - strings.xml: union of both sides' additions. - New main UI strings (#876 unverified-sender chip + signature card) extracted to resources so NoHardcodedUiString stays green. Verified: :app:detekt, compile, resources, ktlint, :detekt-rules:test green; affected unit tests green except 5 pre-existing AdvancedCardTest transportNodeToggle flakes (pass in CI's noSentry variant).
…e presence Greptile P1/P2 on the merge: - UiTextDumper now records UiMessage.Text/Plural, FlashText.Resource and flashText construction sites (resolved through ResResolver, listOf/vararg format args substituted, nested carriers resolved recursively). These carriers are displayed via resolve() at render sites, so without this an English resource edit on the UiMessage path escaped the equivalence gate. Carriers are matched on the qualified name (receiver + callee) so the bare callee 'Text' never collides with the Compose sink; carrier calls skip sink processing (no double-emit). - Main.kt no longer emits '### heading'/'!! file missing' for files absent from a tree or containing no UI text: the dump compares text entries only, so an ordinary file add/delete can no longer fail the gate.
i18n: extract all user-facing UI strings to resources
Umbrella PR for the string-extraction program (25 commits, one per ticket). Every user-facing English literal in the app now lives in
values/strings.xml(or the owning module'svalues/), enforced permanently by CI. Translators can now cover 100% of the UI.What landed
NoHardcodedUiStringdetekt rule) — flags string literals passed to UI sinks (Text,label=,contentDescription=, placeholders, Toast/Snackbar/notification text, enum & sealed display fields), descending through+concat,if/else,?:,whenresults, and supertype constructor args. Baseline: zero entries.@Suppresscount: zero. Ratified do-not-translate literals live indetekt-config.yml → doNotTranslateLiteralswith per-entry policy comments.ui-text-equivalence) — dumps the UI-text skeleton of changed files before/after and fails on any unintended text change. The only bypass is thestring-changes-expectedlabel (applied to this PR: the migration intentionally relocates sink text, e.g. enum display fields → resources).ui/components/**components from pre-migration code; every migration ticket kept these green with unchanged golden files.FlashStage/UsbPairingPhaseenums,PropagationFailureReason.Reason+pf:storage tokens (legacy English decodes to raw), typed validator errors (InputErrorReason/HashErrorReason+messageResource()),UiMessage(Text/Plural/Raw) as the ViewModel→UI text carrier,UiMessageExceptionto carry resource ids across exception hops.:rns-host/:rns-api/:rns-stats/:rns-backend-ktstrings, notifications + nav labels, all ViewModels, utils/services/data models, and all UI screens & components. ~2,800 resources total.docs/translation-workflow.md: how enforcement works, string conventions (prefixes, positional placeholders, plurals), adding a locale, pseudo-locale how-to, and the lint checklist for the first real locale.Verification
testSentryKotlinBackendDebugUnitTest,testNoSentryKotlinBackendDebugUnitTest, module suites) — the only failures are 5 pre-existingAdvancedCardTest.transportNodeToggle*failures already onmain(unrelated to this PR).ktlintCheck+detekt(maxIssues 0) +cpdCheck+ Android lint green.N tiles/N identitiesplurals fix then==1grammar where the ticket explicitly requested plurals).app_nameresValues and notification channel ids unchanged.Notes for reviewers
$x dBm,v$version), and hardware product names intentionally stay in code — see the DNT allowlist comments anddocs/translation-workflow.md.