ryanofsky
commented at 5:07 pm on November 15, 2024:
contributor
Noticed while reviewing #31287 (review) that the [default wallet] part of progress messages remains untranslated while the rest of the string is translated.
Fix this in all places where CWallet::ShowProgress (which has a cancel button) and Chain::showProgress (which doesn’t have a cancel button) are called by making “default wallet” into a translated string.
DrahtBot
commented at 5:08 pm on November 15, 2024:
contributor
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Conflicts
Reviewers, this pull request conflicts with the following ones:
#28710 (Remove the legacy wallet and BDB dependency by achow101)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
DrahtBot added the label
Wallet
on Nov 15, 2024
hebasto
commented at 4:43 pm on November 16, 2024:
member
Concept ACK.
maflcko
commented at 11:17 am on November 17, 2024:
member
What are the steps to test this with sqlite? If it isn’t needed after the bdb removal, it can probably be skipped?
ryanofsky force-pushed
on Nov 18, 2024
ryanofsky
commented at 3:49 pm on November 18, 2024:
contributor
What are the steps to test this with sqlite? If it isn’t needed after the bdb removal, it can probably be skipped?
Not sure if this will always be the case, but it is currently possible to create an sqlite wallet with no name with bitcoin-cli -regtest createwallet "".
Regardless of the details of the bug though, I think the real problem is that the GetDisplayName() method name suggests it should be used to get wallet names for display while the code comment and implementation suggest it’s only meant be used for logging. So I updated the bugfix and added a second refactoring commit to this PR to clean up the interface and prevent this type of problem in the future. That might motivate the PR more beyond the current bugfix.
Updated 87152db6668edbdb863adfb12c45533b37522ca6 -> ebb77ab77df4081f27ce55e8b28b0d35990caac8 (pr/dtran.1 -> pr/dtran.2, compare) adding some refactoring after the bugfix.
wallet: Translate [default wallet] string in progress messages
Noticed while reviewing #31287
(https://github.com/bitcoin/bitcoin/pull/31287#discussion_r1843809721) that the
[default wallet] part of progress messages remains untranslated while the rest
of the string is translated. Fix this in all places where Wallet::ShowProgress
(which has a cancel button) and chain::showProgress (which doesn't have a
cancel button) are called by making "default wallet" into a translated string.
To minimize scope of this bugfix, this introduces a new wallet DisplayName()
method which behaves differently than the existing GetDisplayName() method. The
existing method will be cleaned up in the following commit.
370abf849b
wallet, refactor: Replace GetDisplayName() with LogName()
The GetDisplayName() method name was confusing because it suggested the return
value could be used for display, while documentation and implementation
indicated it only meant to be used for logging. Also the name didn't suggest
that it was formatting the wallet names, which made it harder understand how
messages were formatted in the places it was called. Fix these issues by
splitting up the GetDisplayName() method and replacing it with LogName() /
DisplayName() methods.
This commit is a refactoring that does not change any behavior.
a46ec1dece
DrahtBot added the label
Needs rebase
on Jan 15, 2025
ryanofsky force-pushed
on Jan 15, 2025
ryanofsky
commented at 8:27 pm on January 15, 2025:
contributor
Rebased ebb77ab77df4081f27ce55e8b28b0d35990caac8 -> a46ec1dece8d8a1b16ff1fc3d2932bca130bf67f (pr/dtran.2 -> pr/dtran.3, compare) due to conflict with #31061
DrahtBot removed the label
Needs rebase
on Jan 16, 2025
achow101
commented at 0:33 am on May 3, 2025:
member
ACKa46ec1dece8d8a1b16ff1fc3d2932bca130bf67f
DrahtBot requested review from hebasto
on May 3, 2025
DrahtBot added the label
Needs rebase
on May 7, 2025
DrahtBot
commented at 2:28 pm on May 7, 2025:
contributor
🐙 This pull request conflicts with the target branch and needs rebase.
This is a metadata mirror of the GitHub repository
bitcoin/bitcoin.
This site is not affiliated with GitHub.
Content is generated from a GitHub metadata backup.
generated: 2025-05-08 09:13 UTC
This site is hosted by @0xB10C More mirrored repositories can be found on mirror.b10c.me