This PR adds the bilingual_str struct and a strprintf overload:
https://github.com/bitcoin/bitcoin/blob/0626b8cbdf0aa971500eb5613c7ab4096c496966/src/tinyformat.h#L1066-L1067
Both new features allow bitcoin code to easily send dual translated and non-translated messages to the GUI and the logging framework.
This PR is only a refactoring (has been split off the #16224 (see: https://github.com/bitcoin/bitcoin/pull/16224/#issuecomment-509718579)) and does not change behavior.
https://github.com/bitcoin/bitcoin/pull/16224/#issuecomment-509701396
ACK 4e1eda717e, the diff seems verbose but I think it makes sense to be more explicit where translated strings are passed in. Currently we have issues where translated strings end up in the debug log.
Also, this is a requirement for future fixes such as #16224
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3ACK 4e1eda717e, the diff seems verbose but I think it makes sense
4to be more explicit where translated strings are passed in.
5Currently we have issues where translated strings end up in the debug log.
6
7Also, this is a requirement for future fixes such as [#16224](/bitcoin-bitcoin/16224/)
8-----BEGIN PGP SIGNATURE-----
9
10iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
11pUgAxgv7B8gVJW+NBhgvYFjV7GzTcEDz0+R5pW0pW+8JXt5bYm4l9YkmBZVq/kk+
12Pf/Wr2Vv91OoFIAiAu8iObedp35PzybgeZ4Dc9HlidhOHZvqxOrjNf+lINNyh+ZZ
13ibv0VVcNQTeffMf6+zQoHazuyJPtNarJHmfPQQPBuYPbniW8DpDcRtamMIBxg0sd
142FMLWtgt214boyf0aNUTmQUjxqwtafuqSpOri1VVMc458lIAKma8tdQUo9U/ryfP
15+F8ev8SuK+ZJF3DRqah5mCO6+5Cd5vv0e/wsVemL6DBW11IuTOdW8WCFQoTIQINL
16ayT1m4fbe9wdKh1r/j4Hla8qGjrtPSfgoEj+wfghACLP2MVZxZwqraLz2tH4NFQ4
178zVtjR7Ms1PVSSNFGEoEWU7ouYdX/Jgz/KDp/MxpeFMwzTNZkBWrP5An1AwuJyyt
185VR16+80UJfF5qnOXpO3tt7t7CQzB194ZBJVLneTVUye0Q/87xW8Y5MjCDn5QBVD
19TtPRZurp
20=g2TS
21-----END PGP SIGNATURE-----
Timestamp of file with hash a12ab7776476c20805e88823e532aa26d34082ee0510d1f17344e7869974c62a -
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
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.
Thank for a making new PR to separate refactoring and behavior changes!
Conditional utACK 3b7348ff8a4633679a387dcbfe266348dcba6c2a if this PR is fixed to have a descriptive title and OP. For title I’d suggest, “Add bilingual_str class”. In the description, I’d say something like, “This PR adds a bilingual_str class and a strprintf overload that allows bitcoin code to easily send dual translated and non-translated messages to the GUI and the logging framework. This PR is only a refactoring, and does not change behavior.”
As I commented on the other PR, I think this PR and its followup would be simpler if the _ function signature was changed to return bilingual_str in the same commit that changed ThreadSafeMessageBox, InitError, InitWarning, and similar functions to accept bilingual_str arguments. This would avoid the churn of having to add .translated suffixes and then remove them later. It would mean replacing the scripted diff commit with a manual commit, but I think the manual commit should be smaller, and honestly I’d consider it an advantage not to have to decipher the sed / bash code anyway. But this is just my suggested approach, I’m also fine with the current approach.
@ryanofsky Thank you for your review.
For title I’d suggest, “Add bilingual_str class”. In the description, I’d say something like, “This PR adds a bilingual_str class and a strprintf overload that allows bitcoin code to easily send dual translated and non-translated messages to the GUI and the logging framework. This PR is only a refactoring, and does not change behavior.”
Done.
… I’m also fine with the current approach.
Let it be ;)
utACK 375d90974a9ba3569bb6710c8f26cdda53d01389. Only changes since last review are rebase and updating the PR title & description (thanks!). There currently a lint error making travis fail, but this looks good otherwise.
One thing is I would probably remove “gui:” prefix from PR title because that even though this is refactoring that doesn’t change behavior and is meant to help the gui, the prefix could mislead reviewers to thinking the PR doesn’t touch non-gui code.
This is a prerequisite for introducing bilingual error messages.
Note: #includes are arranged by clang-format-diff.py script.
-BEGIN VERIFY SCRIPT-
sed -i 's/inline std::string _(const char\* psz)/inline bilingual_str _(const char\* psz)/' src/util/translation.h
sed -i 's/return G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(psz) : psz;/return bilingual_str{psz, G_TRANSLATION_FUN ? (G_TRANSLATION_FUN)(psz) : psz};/' src/util/translation.h
sed -i 's/\b_("\([^"]\|\\"\)*")/&.translated/g' $(git grep --files-with-matches '\b_("' src)
echo Hard cases - multiline strings.
sed -i 's/"Visit %s for further information about the software.")/&.translated/g' src/init.cpp
sed -i "s/\"Only rebuild the block database if you are sure that your computer's date and time are correct\")/&.translated/g" src/init.cpp
sed -i 's/" restore from a backup.")/&.translated/g' src/wallet/db.cpp
sed -i 's/" or address book entries might be missing or incorrect.")/&.translated/g' src/wallet/wallet.cpp
echo Special case.
sed -i 's/_(COPYRIGHT_HOLDERS)/&.translated/' src/util/system.cpp test/lint/lint-format-strings.py
-END VERIFY SCRIPT-There currently a lint error making travis fail, but this looks good otherwise.
Fixed.
One thing is I would probably remove “gui:” prefix from PR title because that even though this is refactoring that doesn’t change behavior and is meant to help the gui, the prefix could mislead reviewers to thinking the PR doesn’t touch non-gui code.
Fair point. Done.
ACK 753f7cccce
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3ACK 753f7cccce
4-----BEGIN PGP SIGNATURE-----
5
6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
7pUiBwgv+OJt8U0HjITstlQNagRqZacUEQZyME/hdiZRw7caFb5FGuDAulw8igqrl
8PyfXIFF++rOnK/SztA/KklP6vTaTQ5VhVJpMYLsklwE0Bq4WLfhrKOybpnI+WZbo
9Iv/jbKwTpjw2XR/LtVIZ6F1Dos3CTBFfJwDXgru6JEi86ByOMf9X7yCJdEi1v546
10Bpg9brypH3hwcmv40mTlqazxYGvbX8PKRxH8TpEStgmyujpsKsnLHsAAQAFqAO+R
11mlSC8IhT35IXAxUlr8LTvyIK1sVmShfOm9j2xUZh9DLBPR8aVt19eHgcYUNh/z1Z
12ybNOe+MyYnfAnfHFmWAFaBd9lFiZWfbANsqI+htv1z5WDoEbq2j2urB7xnHbTiX+
13fUmsCzUgMlIu88B02CyUuCqmeRYkPoPRIX0PEgqOVD83+TyseMjUzBPKHGgQhL71
14GUV3yqgxAyme+cqFm8gHg3K/bBeY2vm5HRfK1RplJSssQrWJtzLpeT4uMSj5VSnV
15qcBkK4py
16=g2sy
17-----END PGP SIGNATURE-----
Timestamp of file with hash ca358d398db24f0840fdb656fc63a356463d01b9f675517cbd06d4b4e20071c3 -