gui: Bilingual GUI error messages #16224

hebasto · 2019-06-17T13:02:55Z

This is an alternative to #15340 (it works with the Chain interface; see: #15340 (comment)).
Refs:

gui: Translated messages should not be written to debug log (InitError, InitWarning, ...) #16218 (partial fix)
Remove duplicated "Error: " prefix in logs #15894 (comment)

This PR:

makes GUI error messages bilingual: user's native language + untranslated (i.e. English)
insures that only untranslated messages are written to the debug log file and to stderr (that is not the case on master).

If a translated string is unavailable only an English string appears to a user.

Here are some examples (updated):

qt5ct: using qt5ct plugin message is my local environment specific; please ignore it.

Note for reviewers: InitWarning() is out of this PR scope.

hebasto · 2019-06-17T13:07:39Z

@MarcoFalke your #15340 (comment) has been addressed.

ryanofsky · 2019-06-17T15:02:58Z

EDIT: I didn't realize when I wrote this there was lots of previous discussion already about showing untranslated messages in #15340.

Concept ACK, especially for the code cleanup making error handling more consistent. Some thoughts:

I'm not sure it would be better to show same message twice in error dialogs, than to only show the translated message and something like "More detailed debug information can be found in debug.log," or maybe just the translated message and a "View debug log..." button, to reduce the amount of clutter. I'd hope that in most cases translated error messages would be clear enough to help users solve problems, and only more rarely would users need untranslated strings for to report a bug or look things up.
Using _("") syntax for translated strings is pretty common and well understood, but _("", true) for bilingual strings seems harder to distinguish and figure out. Might worth considering other syntaxes _b(""), bilingual(""), translate("") or even ""_translate with user literal syntax. Other option would be not having any special syntax and just adding normal functions that return literal and formatted bilingual_str objects.

PR looks good in its current form though, and it would be good to get opinions from other reviewers.

maflcko · 2019-06-17T15:42:32Z

Is there even a point in having two different translation functions long term? Shouldn't all non-gui code use the one that returns a bilingual_str?

ryanofsky · 2019-06-17T15:58:55Z

Is there even a point in having two different translation functions long term?

Good point. I don't think there is. So another alternative could be to update the _ function to return both translated and untranslated strings, and not have any new syntax at all.

DrahtBot · 2019-06-17T18:37:48Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

The Zero Allocations project #18849 (The Zero Allocations project by jb55)
Remove g_rpc_node global #18740 (Remove g_rpc_node global by ryanofsky)
[WIP] Index for UTXO Set Statistics #18000 (Index for UTXO Set Statistics by fjahr)
qt: Do not block GUI thread in RPCConsole #17659 (qt: Do not block GUI thread in RPCConsole by hebasto)
util: Set safe permissions for data directory and wallets/ subdir #17127 (util: Correct permissions for datadir and wallets subdir by hebasto)
assumeutxo #15606 ([experimental] UTXO snapshots by jamesob)

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.

hebasto · 2019-06-19T15:53:57Z

Rebased.

src/util/translation.h

hebasto · 2019-06-25T16:46:02Z

Rebased.

@practicalswift Thank you for review.
Your comments have been addressed. Nothing prevents return value optimization now ;)

hebasto · 2019-06-25T16:54:09Z

@ryanofsky

Using _("") syntax for translated strings is pretty common and well understood, but _("", true) for bilingual strings seems harder to distinguish and figure out. Might worth considering other syntaxes _b(""), bilingual(""), translate("") or even ""_translate with user literal syntax.

IIUC, this will require appropriate changes for the share/qt/extract_strings_qt.py script, right?

src/util/translation.h

hebasto · 2019-06-28T18:31:59Z

@MarcoFalke

Is there even a point in having two different translation functions long term? Shouldn't all non-gui code use the one that returns a bilingual_str?

Implemented.

It could probably be done as a scripted-diff.

Done.

Also rebased on top of #16278.

maflcko · 2019-06-28T19:03:01Z

ACK 72d3599e5b06fb1799c8b3689e76d7b97b067531

Not sure about the huge scripted diff, you can probably replace the body with

sed -i --regexp-extended 's/\b_\("([^"]+|\\")*"\)/&.translated/g' $(git grep --files-with-matches '\b_(' src)

hebasto · 2020-05-05T02:08:44Z

Updated 5ff042d4955ab0f8a0dd8ed2522cf0da17f5b00d -> 18bd83b:

this PR does not add any new error message translations, see @MarcoFalke's comment:

I'd say we should leave all new translations for a later pull. You can blindly mark everything Untranslated for now

added "util: Cleanup translation.h" commit to address @MarcoFalke's comment from wallet: Avoid translating RPC errors #18699

Sjors · 2020-05-08T15:19:45Z

re-tACK 18bd83b

maflcko · 2020-05-08T16:00:01Z

ACK 18bd83b 🐦

Thanks for finally making the debug log of international users readable. I
found it painful to translate the debug log in bug reports back to English.

I checked that no new translations are added, but did not run the GUI.

Show signature and timestamp

Signature:

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK 18bd83b1fee2eb47ed4ad05c91f2d6cc311fc9ad 🐦

Thanks for finally making the debug log of international users readable. I
found it painful to translate the debug log in bug reports back to English.

I checked that no new translations are added, but did not run the GUI.
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUiI8wwAnm6ab4DxH44gY+MYdYKbbQe+Qayc2wuMw1wvpNfYdJDZ2xK2N5ZWCIoc
grA4eVeluZggMf30d8pKP+HFofQVugYFyTB/lG/9PnSl//WAgezClqkxnW1YE0pm
xfVfKC1f7RBGswfcm+Rrq7m0LfzWEjoGgCCbcjQm86kAP+Mt8phmjQRNc43FycFJ
dLO7DFZMiqjSIhLi4bbSqiynUXuTY2tMbjv4e23gHWKFcnvndBE3QjfirsRlcp4f
Ly5lvEFCABOdnMQ0K8/9bSn2XqHwWyZxXJ+Yj1twkCchcjmcnHTfAmTJXs0Vahqd
vapzAw2X0dk6x8JWZcu+am7rFSzZu+tAHWnDC1DgjgsnDZMMTI4rjPbtBP9X01ka
C6SjRDt91NZdm2x+jXrHKEhmQdQ3FDVU2hGb6g3cjxe/RJAyxZ3W5ZTTYqvQXOjh
992J7pQOtQjQHovjFWyQ4WSxlTXPd7PFhZ1NyOJE2UCJD8rdUtav2nxzbh0q3SX2
Seu4nsii
=xXpH
-----END PGP SIGNATURE-----

Timestamp of file with hash 1fd623baf008aff28a965813df642d958b9fc77a22a30025ff01d55db57e248e -

Updated name-specific RPC error messages for upstream bitcoin/bitcoin#16224.

2308385 [test] Add test for cfcheckpt (Jim Posen) f9e00bb [net processing] Message handling for getcfcheckpt. (Jim Posen) 9ccaaba [init] Add -peerblockfilters option (Jim Posen) Pull request description: Serve cfcheckpt messages if basic block filter index is enabled and `-peercfilters` is set. `NODE_COMPACT_FILTERS` is not signaled to peers, but functionality can be used for testing and serving pre-configured clients. ACKs for top commit: jonatack: Code review re-ACK 2308385 the only change since my review @ 967e2b1 is an update required for #16224 that was merged yesterday. fjahr: re-ACK 2308385 jkczyz: re-ACK 2308385 ariard: re-Code Review ACK 2308385 clarkmoody: Tested ACK 2308385 MarcoFalke: re-ACK 2308385 🌳 theStack: ACK 2308385 Tree-SHA512: 8c751bbd7d1c31a413096462ae025c3d2f3163c7016cbec472a5f5ec267f8dd19a2dfc4d749876d7409c1db546e6fdd16461c6863effcfa0d3e993edcfa92a08

18bd83b util: Cleanup translation.h (Hennadii Stepanov) e95e658 doc: Do not translate technical or extremely rare errors (Hennadii Stepanov) 7e923d4 Make InitError bilingual (Hennadii Stepanov) 917ca93 Make ThreadSafe{MessageBox|Question} bilingual (Hennadii Stepanov) 23b9fa2 gui: Add detailed text to BitcoinGUI::message (Hennadii Stepanov) Pull request description: This is an alternative to bitcoin#15340 (it works with the `Chain` interface; see: bitcoin#15340 (comment)). Refs: - bitcoin#16218 (partial fix) - bitcoin#15894 (comment) This PR: - makes GUI error messages bilingual: user's native language + untranslated (i.e. English) - insures that only untranslated messages are written to the debug log file and to `stderr` (that is not the case on master). If a translated string is unavailable only an English string appears to a user. Here are some **examples** (updated): ![Screenshot from 2020-04-24 17-08-37](https://user-images.githubusercontent.com/32963518/80222043-e2458780-864e-11ea-83fc-197b7121dba5.png) ![Screenshot from 2020-04-24 17-12-17](https://user-images.githubusercontent.com/32963518/80222051-e5407800-864e-11ea-92f7-dfef1144becd.png) * `qt5ct: using qt5ct plugin` message is my local environment specific; please ignore it. --- Note for reviewers: `InitWarning()` is out of this PR scope. ACKs for top commit: Sjors: re-tACK 18bd83b MarcoFalke: ACK 18bd83b 🐦 Tree-SHA512: 3cc8ec44f84403e54b57d11714c86b0855ed90eb794b5472e432005073354b9e3f7b4e8e7bf347a4c21be47299dbc7170f2d0c4b80e308205ff09596e55a4f96

2308385 [test] Add test for cfcheckpt (Jim Posen) f9e00bb [net processing] Message handling for getcfcheckpt. (Jim Posen) 9ccaaba [init] Add -peerblockfilters option (Jim Posen) Pull request description: Serve cfcheckpt messages if basic block filter index is enabled and `-peercfilters` is set. `NODE_COMPACT_FILTERS` is not signaled to peers, but functionality can be used for testing and serving pre-configured clients. ACKs for top commit: jonatack: Code review re-ACK 2308385 the only change since my review @ 967e2b1 is an update required for bitcoin#16224 that was merged yesterday. fjahr: re-ACK 2308385 jkczyz: re-ACK 2308385 ariard: re-Code Review ACK 2308385 clarkmoody: Tested ACK 2308385 MarcoFalke: re-ACK 2308385 🌳 theStack: ACK bitcoin@2308385 Tree-SHA512: 8c751bbd7d1c31a413096462ae025c3d2f3163c7016cbec472a5f5ec267f8dd19a2dfc4d749876d7409c1db546e6fdd16461c6863effcfa0d3e993edcfa92a08

5527be0 refactor: Add AbortError alias (Hennadii Stepanov) d924f2a Drop MSG_NOPREFIX flag (Hennadii Stepanov) 083daf7 Pass bilingual_str argument to AbortNode() (Hennadii Stepanov) d1cca12 refactor: Use bilingual_str::empty() (Hennadii Stepanov) Pull request description: This PR is a [followup](#16218 (comment)) of #16224, and it adds `bilingual_str` type argument support to the `AbortNode()` functions. ACKs for top commit: MarcoFalke: ACK 5527be0 👟 Tree-SHA512: bf8b15b14912b1f672e6e588fffa1e6eb6f00b4b23d15d0ced7f18fbdf76919244427feb7217007fe29617049308e13def893a03a87358db819cca9692f59905

5527be0 refactor: Add AbortError alias (Hennadii Stepanov) d924f2a Drop MSG_NOPREFIX flag (Hennadii Stepanov) 083daf7 Pass bilingual_str argument to AbortNode() (Hennadii Stepanov) d1cca12 refactor: Use bilingual_str::empty() (Hennadii Stepanov) Pull request description: This PR is a [followup](bitcoin#16218 (comment)) of bitcoin#16224, and it adds `bilingual_str` type argument support to the `AbortNode()` functions. ACKs for top commit: MarcoFalke: ACK 5527be0 👟 Tree-SHA512: bf8b15b14912b1f672e6e588fffa1e6eb6f00b4b23d15d0ced7f18fbdf76919244427feb7217007fe29617049308e13def893a03a87358db819cca9692f59905

753f7cc scripted-diff: Make translation bilingual (Hennadii Stepanov) 7c45e14 Add bilingual message type (Hennadii Stepanov) 0b86e51 Refactor out translation.h (Hennadii Stepanov) Pull request description: 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: bitcoin/bitcoin#16224)) and does not change behavior. ACKs for top commit: MarcoFalke: ACK 753f7cc ryanofsky: utACK 753f7cc. Only change since last review is fixing lint error (double includes) Tree-SHA512: 52b0654421d558e4775c0484d78be26319fe3db5118af9b0a9bdfbdaad53a3704f527a5d5aba1013a64560b9b6a0c3c4cf0a6782e49aa731e18d99de95220385

Summary: Backport of Core [[bitcoin/bitcoin#16224 | PR16224]] - part 1-5 Commit bitcoin/bitcoin@23b9fa2 Test Plan: The added code is not yet used in this commit. Just make sure it still compiles. `ninja` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7985

Summary: > This PR: > > - makes GUI error messages bilingual: user's native language + untranslated (i.e. English) > - insures that only untranslated messages are written to the debug log file and to stderr (that is not the case on master). Backport of Core [[bitcoin/bitcoin#16224 | PR16224]] - part 2 of 5 bitcoin/bitcoin@917ca93 Depends on D7985 Test Plan: `ninja && ninja check` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7986

Summary: > This PR: > > - makes GUI error messages bilingual: user's native language + untranslated (i.e. English) > - insures that only untranslated messages are written to the debug log file and to stderr (that is not the case on master). Backport of Core [[bitcoin/bitcoin#16224 | PR16224]] - part 3 of 5 Commit: bitcoin/bitcoin@7e923d4 Depends on D7986 Test Plan: `ninja && ninja check` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7997

…e errors Summary: Backport of Core [[bitcoin/bitcoin#16224 | PR16224]] - part 4 of 5 Commit bitcoin/bitcoin@e95e658 Depend on D7997 Test Plan: Proofreading (minor documentation change) Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7998

Summary: This concludes backport of Core [[bitcoin/bitcoin#16224 | PR16224]] - part 5 of 5 Depends on D7998 Commit bitcoin/bitcoin@18bd83b Test Plan: `ninja && ninja check` Reviewers: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Reviewed By: O1 Bitcoin ABC, #bitcoin_abc, deadalnix Subscribers: deadalnix Differential Revision: https://reviews.bitcoinabc.org/D7999

18bd83b util: Cleanup translation.h (Hennadii Stepanov) e95e658 doc: Do not translate technical or extremely rare errors (Hennadii Stepanov) 7e923d4 Make InitError bilingual (Hennadii Stepanov) 917ca93 Make ThreadSafe{MessageBox|Question} bilingual (Hennadii Stepanov) 23b9fa2 gui: Add detailed text to BitcoinGUI::message (Hennadii Stepanov) Pull request description: This is an alternative to bitcoin#15340 (it works with the `Chain` interface; see: bitcoin#15340 (comment)). Refs: - bitcoin#16218 (partial fix) - bitcoin#15894 (comment) This PR: - makes GUI error messages bilingual: user's native language + untranslated (i.e. English) - insures that only untranslated messages are written to the debug log file and to `stderr` (that is not the case on master). If a translated string is unavailable only an English string appears to a user. Here are some **examples** (updated): ![Screenshot from 2020-04-24 17-08-37](https://user-images.githubusercontent.com/32963518/80222043-e2458780-864e-11ea-83fc-197b7121dba5.png) ![Screenshot from 2020-04-24 17-12-17](https://user-images.githubusercontent.com/32963518/80222051-e5407800-864e-11ea-92f7-dfef1144becd.png) * `qt5ct: using qt5ct plugin` message is my local environment specific; please ignore it. --- Note for reviewers: `InitWarning()` is out of this PR scope. ACKs for top commit: Sjors: re-tACK 18bd83b MarcoFalke: ACK 18bd83b 🐦 Tree-SHA512: 3cc8ec44f84403e54b57d11714c86b0855ed90eb794b5472e432005073354b9e3f7b4e8e7bf347a4c21be47299dbc7170f2d0c4b80e308205ff09596e55a4f96 # Conflicts: # src/dashd.cpp # src/httpserver.cpp # src/index/base.cpp # src/init.cpp # src/interfaces/chain.cpp # src/interfaces/chain.h # src/interfaces/node.cpp # src/net.h # src/qt/bitcoingui.cpp # src/ui_interface.h # src/wallet/init.cpp # src/wallet/load.cpp

fanquake added the GUI label Jun 17, 2019

hebasto mentioned this pull request Jun 17, 2019

gui: Introduce bilingual GUI error messages #15340

Closed

hebasto force-pushed the 20190617-bilingual-errors branch from 7fd5179 to c55cc2d Compare June 17, 2019 13:26

hebasto mentioned this pull request Jun 17, 2019

gui: Translated messages should not be written to debug log (InitError, InitWarning, ...) #16218

Closed

DrahtBot added the Needs rebase label Jun 18, 2019

hebasto force-pushed the 20190617-bilingual-errors branch from c55cc2d to 81baf61 Compare June 19, 2019 15:53

DrahtBot added Needs rebase and removed Needs rebase labels Jun 19, 2019

practicalswift reviewed Jun 25, 2019

View reviewed changes

src/util/translation.h Outdated Show resolved Hide resolved

practicalswift reviewed Jun 25, 2019

View reviewed changes

src/util/translation.h Outdated Show resolved Hide resolved

hebasto force-pushed the 20190617-bilingual-errors branch from 81baf61 to 1855f54 Compare June 25, 2019 16:42

DrahtBot removed the Needs rebase label Jun 25, 2019

maflcko mentioned this pull request Jun 25, 2019

Remove duplicated "Error: " prefix in logs #15894

Merged

maflcko reviewed Jun 26, 2019

View reviewed changes

src/util/translation.h Outdated Show resolved Hide resolved

DrahtBot added the Needs rebase label Jun 27, 2019

hebasto force-pushed the 20190617-bilingual-errors branch 2 times, most recently from afc38ac to 4933242 Compare June 28, 2019 18:13

DrahtBot removed the Needs rebase label Jun 28, 2019

hebasto force-pushed the 20190617-bilingual-errors branch 2 times, most recently from aead754 to e985a07 Compare June 29, 2019 08:55

hebasto added 2 commits May 5, 2020 04:46

doc: Do not translate technical or extremely rare errors

e95e658

util: Cleanup translation.h

18bd83b

hebasto force-pushed the 20190617-bilingual-errors branch from 5ff042d to 18bd83b Compare May 5, 2020 02:02

maflcko merged commit 5b24f60 into bitcoin:master May 8, 2020

hebasto deleted the 20190617-bilingual-errors branch May 8, 2020 16:25

jonatack mentioned this pull request May 8, 2020

Serve cfcheckpt requests #18877

Merged

hebasto mentioned this pull request May 10, 2020

Pass bilingual_str argument to AbortNode() #18927

Merged

domob1812 added a commit to domob1812/namecoin-core that referenced this pull request May 11, 2020

Merge branch 'auxpow'

e247092

Updated name-specific RPC error messages for upstream bitcoin/bitcoin#16224.

str4d mentioned this pull request Oct 18, 2020

Support internationalization for CLI documentation & the metrics screen. zcash/zcash#2249

Open

hebasto mentioned this pull request Aug 5, 2021

Prompt to reset settings when settings.json cannot be read bitcoin-core/gui#379

Merged

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

gui: Bilingual GUI error messages #16224

gui: Bilingual GUI error messages #16224

Uh oh!

Conversation

hebasto commented Jun 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Jun 17, 2019

Uh oh!

ryanofsky commented Jun 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Jun 17, 2019

Uh oh!

ryanofsky commented Jun 17, 2019

Uh oh!

DrahtBot commented Jun 17, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

hebasto commented Jun 19, 2019

Uh oh!

Uh oh!

Uh oh!

hebasto commented Jun 25, 2019

Uh oh!

hebasto commented Jun 25, 2019

Uh oh!

Uh oh!

hebasto commented Jun 28, 2019

Uh oh!

maflcko commented Jun 28, 2019

Uh oh!

hebasto commented May 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Sjors commented May 8, 2020

Uh oh!

maflcko commented May 8, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

hebasto commented Jun 17, 2019 •

edited

Loading

ryanofsky commented Jun 17, 2019 •

edited

Loading

DrahtBot commented Jun 17, 2019 •

edited

Loading

hebasto commented May 5, 2020 •

edited

Loading