fix: improve governance/proposal dialog strings#6885
fix: improve governance/proposal dialog strings#6885PastaPastaPasta merged 1 commit intodashpay:developfrom
Conversation
- make strings easier to translate - stop translating non-translatable strings - use `%n` where possible
✅ No Merge Conflicts DetectedThis PR currently has no conflicts with other open PRs. |
WalkthroughThis change updates UI and message strings to improve translation handling and simplifies result message assembly. In the UI .ui file, several static strings and placeholders are annotated with notr="true". In proposal creation, plural-aware formatting is used for ETA minutes and combo item text generation is adjusted. In governance voting, the result summary is refactored to linearly build a message that conditionally includes success counts, failure counts, and error details. No new UI elements, business logic, or public interfaces are introduced. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/qt/forms/proposalwizard.ui (1)
430-430: Consider whether "TxID:" should be translatable.The label
"TxID:"is marked withnotr="true". While technical terms are often kept in English, some translation systems prefer to localize technical UI labels for consistency with the rest of the interface. This is a design decision.If you decide "TxID:" should be translatable, remove the
notr="true"attribute:-<string notr="true">TxID:</string> +<string>TxID:</string>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/qt/forms/proposalwizard.ui(7 hunks)src/qt/governancelist.cpp(1 hunks)src/qt/proposalwizard.cpp(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.{cpp,h,cc,cxx,hpp}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{cpp,h,cc,cxx,hpp}: Dash Core C++ codebase must be written in C++20 and require at least Clang 16 or GCC 11.1
Dash uses unordered_lru_cache for efficient caching with LRU eviction
Files:
src/qt/proposalwizard.cppsrc/qt/governancelist.cpp
🔇 Additional comments (7)
src/qt/proposalwizard.cpp (3)
63-63: LGTM! Numbers should not be translated.The change from
tr("%1").arg(i)toQString().setNum(i)correctly removes translation wrapping from numeric values, which should never be translated.
293-294: LGTM! Proper Qt pluralization.The change to
tr("Estimated time remaining: %n minute(s)", "", mins)correctly uses Qt's plural-aware formatting with the%nplaceholder, enabling proper translations for different plural forms across languages.
324-325: LGTM! Correctly separates translatable from non-translatable text.The change separates the translatable success message from the technical ID by concatenating
tr("Your proposal was submitted successfully.")withQString("\nID: %1").arg(govId). This prevents the ID from being included in translation strings where it doesn't belong.src/qt/forms/proposalwizard.ui (3)
113-113: LGTM! Example placeholders should not be translated.Correctly marking placeholder text like
"short-unique-name"and"https://example.com/my-proposal"withnotr="true"as these are examples that should remain consistent across all languages.Also applies to: 139-139
258-258: LGTM! Numbers should not be translated.Correctly marking the numeric value
"0"withnotr="true"as numbers should not be translated.
345-345: LGTM! Placeholder indicators should not be translated.Correctly marking the placeholder indicator
"-"withnotr="true"as these are UI placeholders that should remain consistent across languages.Also applies to: 440-440, 664-664
src/qt/governancelist.cpp (1)
598-609: LGTM! Proper pluralization and clearer message construction.The changes correctly implement:
- Qt's plural-aware formatting with
%nfor both success and failure counts- Linear message assembly using
+=operator- More explicit condition checking (
nSuccessful > 0instead of!message.isEmpty())- Proper separation of translatable text ("Errors:") from non-translatable content (error details)
This enables proper translations for different plural forms across languages.
af85608 fix: improve governance/proposal dialog strings (UdjinM6) Pull request description: ## Issue being fixed or feature implemented - make strings easier to translate - stop translating non-translatable strings - use `%n` where possible ## What was done? ## How Has This Been Tested? ## Breaking Changes n/a ## Checklist: - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ Top commit has no ACKs. Tree-SHA512: bba2247fc64f152656972d7e30c12eea6d892a622751c8e798a0ccb522c297356b1ae3964d9db25aa1c920eb71d862e026b0a490ddce784563aea9caa5445425
6fd7059 chore: mark v23 as release (pasta) ae08f53 docs: integrate 6946 release notes into final (pasta) 74a222d Merge #6946: feat: show seed on wallet creation (pasta) 877343a Merge #6943: fix: don't treat arrays/objects as string literals for composite methods (pasta) 00368fb Merge #6940: fix: reuse best clsig to avoid potential race condition (pasta) 8eceb98 Merge #6938: fix: logic error in `CheckDecryptionKey` (pasta) 3f30664 Merge #6929: fix: repair `makeseeds.py`, `getblockchaininfo[softforks]` help text, drop extra `generate`s from test, resolve macOS GID issue (pasta) 7ba4f1c Merge #6928: docs: update man pages for 23.0 (pasta) a6c1d6a Merge #6920: chore: update minimum chain work, tx stats, checkpoints, seeds and SECURITY.md for v23 (pasta) 84df1f0 Merge #6909: chore: Translations 2025-10 (pasta) a6449b1 Merge #6885: fix: improve governance/proposal dialog strings (pasta) Pull request description: ## Issue being fixed or feature implemented See commits ## Checklist: _Go over all the following points, and put an `x` in all the boxes that apply._ - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: UdjinM6: utACK 6fd7059 Tree-SHA512: a0d93a61a4f4978fbe120bea832ce683a8ae7c16c892a381d91ddc4b25344c0f3ad3306a1a30a166a7dfa6e38e4532708587cc23cc372126828b8e22d141dc85
fe1cff3 chore: bump release to 23.0.2 (pasta) a8f15c1 Merge #7032: fix: drop gsl usage from RebuildListFromBlock function wrapper (pasta) 736bb26 chore: bump manpages for 23.0.1 (pasta) d5c7d25 chore: bump nMinimumChainWork and defaultAssumeValid (pasta) 4f8aa71 chore: bump version to 23.0.1 (pasta) 0865b7c docs: add release notes for 23.0.1 (pasta) 2048b42 Merge #6986: test: new commandline argument -tinyblk to use blk size just 64kb instead 16Mb (pasta) 1a9b20c Merge #7013: fix: update BuildTestVectors call to adjust batch size based on output flag (pasta) 36e4679 Merge #7009: fix: include QDebug directly (pasta) 69d0c9c Merge #6999: feat: verify and repair evodb diffs automatically at node startup (pasta) ca16437 Merge #6996: perf: reduce cs_main lock scope in evodb verify/repair operations (pasta) 207526e Merge #6977: fix: bls benchmarks crash when ran independently (pasta) 226aaf4 Merge #6969: feat: add evodb verify and repair RPC commands (pasta) 92abe9b Merge #6964: perf: remove duplicated check of same key in the instant send database (pasta) 5a1ec4c Merge #6961: fix: correct BLS scheme setting in `MigrateLegacyDiffs()` when `nVersion` is present (pasta) bf653d3 Merge #6949: depends: Qt 5.15.18 (pasta) faf58cd merge bitcoin#30774: Qt 5.15.16 (Kittywhiskers Van Gogh) 6a995f5 Merge #6944: fix: HD chain encryption check ordering issue (pasta) 6fd7059 chore: mark v23 as release (pasta) ae08f53 docs: integrate 6946 release notes into final (pasta) 74a222d Merge #6946: feat: show seed on wallet creation (pasta) 877343a Merge #6943: fix: don't treat arrays/objects as string literals for composite methods (pasta) 00368fb Merge #6940: fix: reuse best clsig to avoid potential race condition (pasta) 8eceb98 Merge #6938: fix: logic error in `CheckDecryptionKey` (pasta) 3f30664 Merge #6929: fix: repair `makeseeds.py`, `getblockchaininfo[softforks]` help text, drop extra `generate`s from test, resolve macOS GID issue (pasta) 7ba4f1c Merge #6928: docs: update man pages for 23.0 (pasta) a6c1d6a Merge #6920: chore: update minimum chain work, tx stats, checkpoints, seeds and SECURITY.md for v23 (pasta) 84df1f0 Merge #6909: chore: Translations 2025-10 (pasta) a6449b1 Merge #6885: fix: improve governance/proposal dialog strings (pasta) ebf3a64 docs: typos (pasta) 4ad5533 docs: typos (pasta) f407453 doc: Replace Bitcoin Core PR references with Dash Core backport PRs (pasta) 78d0725 docs: add note on proto bump and platformban p2p (pasta) e0519c3 docs: fix whitespace errors (pasta) bc8db22 docs: minor improvements to release notes (pasta) c338511 docs: reorganize rpc updates to organize extended address changes (thephez) 700c46e style: make heading style consistent (thephez) bd636bd docs: add contributors (pasta) 6d29bc3 docs: revert changes deferred to v24 (pasta) 615f5ff docs: make the downgrade warning more confident (pasta) 567771a Apply suggestions from code review (PastaPastaPasta) 2b3211a docs: add link to 22.1.3 release notes (pasta) 548a38a docs: remove individual release notes files (pasta) e770c25 docs: add v23.0.0 release notes and archive v22.1.3 (pasta) Pull request description: ## Issue being fixed or feature implemented Includes changes from 23.0.0 release too because we never merged it back. ## What was done? ## How Has This Been Tested? ## Breaking Changes ## Checklist: - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have added or updated relevant unit/integration/functional/e2e tests - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_ ACKs for top commit: PastaPastaPasta: utACK 491db4a kwvg: utACK 491db4a Tree-SHA512: 61895cd1f9d01ac7be1d407afe1ddd24b98e8242cb03229ecd586a4d2d1c43dbc62c98da52a8c715b3a5943fb40e99b23251e691f778779af9d6da94392122a3
3 participants
Issue being fixed or feature implemented
%nwhere possibleWhat was done?
How Has This Been Tested?
Breaking Changes
n/a
Checklist: