gui: Introduce bilingual GUI error messages #15340

hebasto · 2019-02-04T14:59:31Z

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).

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

maflcko · 2019-02-04T15:38:47Z

src/ui_interface.h

+bool InitErrorFormat(const char* fmt, const Args&... args)
+{
+    std::string original_message = strprintf(fmt, args...);
+    std::string translated_message = strprintf(_(fmt), args...);


I believe make translate wouldn't pick those up any more.

laanwj · 2019-02-04T15:41:22Z

This effectively removes all the translations as there is no _() to pick up anymore by gettext.

DrahtBot · 2019-02-04T15:46:37Z

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#16194 (refactor: share blockmetadata with BlockManager by jamesob)
#16127 (Add support for thread safety annotations when using std::mutex by ajtowns)
#16112 (util: Log early messages by MarcoFalke)
#16003 (init: an incorrect amount of file descriptors is requested, and a different amount is also asserted by tryphe)
#15946 (Allow maintaining the blockfilterindex when using prune by jonasschnelli)
#15891 (test: Require standard txs in regtest by default by MarcoFalke)
#15848 (Add a check for free disk space at first startup. by darosior)
#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.

luke-jr · 2019-02-04T15:51:15Z

This would be much easier to review if InitErrorFormat was named InitError...

hebasto · 2019-02-04T16:52:22Z

@MarcoFalke

I believe make translate wouldn't pick those up any more.

@laanwj

This effectively removes all the translations as there is no _() to pick up anymore by gettext.

Fixed.

The main goal of this PR is to avoid printing localized messages to the debug log file and stderr.
Bilingual GUI messages come for free.

hebasto · 2019-02-04T16:55:14Z

@luke-jr

This would be much easier to review if InitErrorFormat was named InitError...

Rationale: InitErrorFormat can accept a format string as the first argument.

jonasschnelli · 2019-02-04T19:19:10Z

I haven't looked at the code but printing error message always (additionally) in english seems to be a clever idea (especially with the faced complexity and immaturity of the UX).

hebasto · 2019-02-04T23:12:42Z

@MarcoFalke @laanwj it works with gettext now.

@luke-jr your comment has been addressed.

hebasto · 2019-02-04T23:20:48Z

Rebased.

maflcko · 2019-02-05T16:04:03Z

src/init.cpp

-    if(!ECC_InitSanityCheck()) {
-        InitError("Elliptic curve cryptography sanity check failure. Aborting.");
+    if (!ECC_InitSanityCheck()) {
+        InitError(_("Elliptic curve cryptography sanity check failure. Aborting.", true));


I don't understand why this should be translated. This should never fail and if it fails, I bet you are better off with the english error message?

Agree, I feel we had this discussion already. Please don't translate extremely rare or technical errors.

Fixed.

btw, this PR aims that a translated message:

is written to debug log in English

is printed to stderr in English

is shown in GUI in both languages

Therefore, english error message is always available even for translated messages.

laanwj · 2019-02-05T16:09:31Z

src/init.cpp


    if (!Random_SanityCheck()) {
-        InitError("OS cryptographic RNG sanity check failure. Aborting.");
+        InitError(_("OS cryptographic RNG sanity check failure. Aborting.", true));


Same for this one

laanwj · 2019-02-05T16:11:18Z

src/ui_interface.cpp

    }

 ADD_SIGNALS_IMPL_WRAPPER(ThreadSafeMessageBox);
+ADD_SIGNALS_IMPL_WRAPPER(ThreadSafeMessageBox2);


Please use a better name than ThreadSafeMessageBox2. What makes this function different?

Fixed.

btw, 2 in ThreadSafeMessageBox2 meant "bi-" in bilingual rather "second" :)

hebasto · 2019-02-05T19:13:44Z

@MarcoFalke @laanwj
Thank you for your reviews. All your comments have been addressed.
Would you mind re-reviewing?

maflcko · 2019-02-05T19:53:02Z

src/init.cpp


    if (!SetupNetworking())
-        return InitError("Initializing networking failed");
+        return InitError(_("Initializing networking failed", true));


same here. This should never fail

flack · 2019-02-08T20:48:32Z

If the point is to make it easier to google error messages, wouldn't it be more efficient to add some kind of error code to the output? E.g. the example from the screenshot could look like this in the English version:

ERR38278: Prune cannot be configured with a negative value

and like this in the German version:

ERR38278: Kürzungsmodus kann nicht mit einem negativen Wert konfiguriert werden

This would also help searchability if the English error text changes between versions. Only drawback I suppose is that somebody would have to come up with all the error codes :-)

gmaxwell · 2019-02-11T20:35:56Z

I like the idea (so Concept ACK), no thoughts on the implementation.

@flack Trying to assign numerical error codes in the face of parallel development seems tricky... but not impossible.

The dual languages also helps in the common case that the user also reads English... since the translation may not be good or may not be clear (esp. even a native speaker of another language may have only seen various technical terms but a translator managed to use a rarely used word in the translated language). Error numbers don't have this benefit, so the dual language output would be useful independently of error numbers.

flack · 2019-02-11T20:55:41Z

@gmaxwell yes, it would require some coordination. But I suppose a linter can take care of most of the problems, esp. since the numbers don't have to be consecutive or follow any kind of logic really.

It's true what you say about wonky translations. "Kürzungsmodus" from the screenshot is actually a case in point. It literally means "shortening mode". No way I would have guessed that this refers to pruning (or at least not on the first try). But otoh, if users have the UI set to e.g. German, then all they see are German terms, i.e. there's probably a checkbox labeled "Kürzungsmodus" somewhere and they might remember that they clicked that. Throwing the word "pruning" into the mix might just add to the confusion at this point, exactly because the meaning is different ("I selected shortening mode and now it's complaining about 'pruning' ?!?").

Anyhow, I get that my suggestion cannot be easily implemented in the short term, but how about making it a bit more clear that the two messages are actually referring to the same thing (instead of being one translated error and one untranslated error)? E.g. something like:

Kürzungsmodus kann nicht mit einem negativen Wert konfiguriert werden

Original-Meldung: Prune cannot be configured with a negative value

"Original-Meldung" means "original message". Could also say "untranslated message" or something.

laanwj · 2019-02-12T10:30:38Z

since the translation may not be good or may not be clear

I think we need to be careful here also to respect the translators. They are not developers and will not understand internal technical details in error messages.

If an error message has more technical terms in it than the standard bitcoin and GUI ones such as window, wallet, block, transaction …. then it shouldn't be translated.

Translators will get pissed at me for this or substitute some nonsense word. In both cases no one is better off.

flack · 2019-02-12T10:44:14Z

If an error message has more technical terms in it than the standard bitcoin and GUI ones such as window, wallet, block, transaction …. then it shouldn't be translated.

Still, you have to enable the user to make the connection between the error message and what they might have selected in the GUI settings. So e.g. if you don't translate "prune" in the error message, then you also shouldn't translate it in the rest of the GUI (or at least show the English term in brackets)

laanwj · 2019-02-12T11:08:53Z

prune is a grey area I guess. It doesn't go with the obvious bitcoin or computer words but it also isn't something like "glibc sanity check failed" which makes no sense to users nor translators whatsoever.

So to be clear Im not against showing error messages in both languages, but this shouldn't a reason to translate things that would otherwise not be because the English message can be used to work around a terrible translation.

hebasto · 2019-02-12T22:28:51Z

Rebased.

DrahtBot · 2019-03-02T22:37:53Z

Needs rebase

hebasto · 2019-06-16T22:20:51Z

Rebased after #15288 merged.

@flack your comment has been addressed.

OP and screenshot updated.

flack · 2019-06-16T22:25:03Z

@hebasto awesome!

hebasto · 2019-06-16T22:33:52Z

After the e2c8ba9 commit merged there is no straight way to pass function template InitError(const bilingual_str& fmt, const Args&... args) through the Chain interface.

I ask for advice @ryanofsky, @MarcoFalke, @jonasschnelli, @promag.

maflcko

Concept ACK

maflcko · 2019-06-17T01:11:37Z

src/util/system.h

+    std::string translated_str;
+};
+
+inline bilingual_str _(const char* psz, bool translate)


What is the point of the bool argument, when it is always set to true at the call site? Should be called bool ignored or something like this?

enum class Tr{BI;}; inline bilingueal_str_(const char* psz, Tr ignored);

maflcko · 2019-06-17T10:22:16Z

src/ui_interface.h


+/** Show bilingual error message **/
+template <typename... Args>
+bool InitError(const bilingual_str& fmt, const Args&... args)


I'd prefer if you removed the plain InitError, since it doesn't seem to be required.

Also, would be nice to add InitError to the linter ./test/lint/lint-format-strings.sh

I'd prefer if you removed the plain InitError, since it doesn't seem to be required.

If I understand you correctly then what about #15340 (comment)?
Also see: fea324f

I mean that InitError could take as first argument a string or bilingual string, so that the formatting always happens inside of InitError. Then add InitError to the linter.

hebasto · 2019-06-17T13:04:50Z

After the e2c8ba9 commit merged there is no straight way to pass function template InitError(const bilingual_str& fmt, const Args&... args) through the Chain interface.

The solution is #16224.

Should this PR be closed?

hebasto · 2019-06-17T15:58:50Z

Closing in favor of #16224 which provides a different approach and works with the Chain interface as expected.

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 #15340 (it works with the `Chain` interface; see: #15340 (comment)). Refs: - #16218 (partial fix) - #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

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

maflcko reviewed Feb 4, 2019

View reviewed changes

hebasto force-pushed the 20190204-bilingual-initerror branch from ecd6197 to 0faa14c Compare February 4, 2019 15:41

hebasto force-pushed the 20190204-bilingual-initerror branch from 0faa14c to 63b7018 Compare February 4, 2019 16:46

hebasto closed this Feb 4, 2019

hebasto reopened this Feb 4, 2019

hebasto force-pushed the 20190204-bilingual-initerror branch 3 times, most recently from 54c8d00 to fb394a6 Compare February 4, 2019 23:01

fanquake added the GUI label Feb 4, 2019

maflcko reviewed Feb 5, 2019

View reviewed changes

laanwj reviewed Feb 5, 2019

View reviewed changes

hebasto force-pushed the 20190204-bilingual-initerror branch from fb394a6 to 01781b4 Compare February 5, 2019 19:07

maflcko reviewed Feb 5, 2019

View reviewed changes

hebasto force-pushed the 20190204-bilingual-initerror branch from 01781b4 to 9a53d13 Compare February 5, 2019 20:00

DrahtBot added the Needs rebase label Feb 8, 2019

hebasto force-pushed the 20190204-bilingual-initerror branch from 9a53d13 to 539950b Compare February 12, 2019 21:46

DrahtBot removed the Needs rebase label Feb 12, 2019

DrahtBot added the Needs rebase label Mar 2, 2019

hebasto closed this Mar 9, 2019

hebasto mentioned this pull request Apr 30, 2019

Remove duplicated "Error: " prefix in logs #15894

Merged

hebasto mentioned this pull request Jun 15, 2019

Fix InitError() and InitWarning() content #15329

Closed

maflcko mentioned this pull request Jun 15, 2019

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

Closed

hebasto reopened this Jun 16, 2019

hebasto added 5 commits June 17, 2019 00:11

Do not translate captions in log and stderr

810e228

Do not translate extremely rare / technical errors

fea324f

Add bilingual messages facilities

a9be63d

Add bilingual InitError() function

302806e

Use bilingual InitError() in simple cases

8aca2cf

hebasto force-pushed the 20190204-bilingual-initerror branch from 539950b to 8aca2cf Compare June 16, 2019 22:06

fanquake removed the Needs rebase label Jun 17, 2019

maflcko reviewed Jun 17, 2019

View reviewed changes

hebasto mentioned this pull request Jun 17, 2019

gui: Bilingual GUI error messages #16224

Merged

hebasto closed this Jun 17, 2019

hebasto deleted the 20190204-bilingual-initerror branch July 18, 2019 15:15

bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022

gui: Introduce bilingual GUI error messages #15340

gui: Introduce bilingual GUI error messages #15340

Uh oh!

Conversation

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

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Feb 4, 2019

Uh oh!

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

Uh oh!

Conflicts

Uh oh!

luke-jr commented Feb 4, 2019

Uh oh!

hebasto commented Feb 4, 2019

Uh oh!

hebasto commented Feb 4, 2019

Uh oh!

jonasschnelli commented Feb 4, 2019

Uh oh!

hebasto commented Feb 4, 2019

Uh oh!

hebasto commented Feb 4, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hebasto commented Feb 5, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flack commented Feb 8, 2019

Uh oh!

gmaxwell commented Feb 11, 2019

Uh oh!

flack commented Feb 11, 2019

Uh oh!

laanwj commented Feb 12, 2019

Uh oh!

flack commented Feb 12, 2019

Uh oh!

laanwj commented Feb 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Feb 12, 2019

Uh oh!

DrahtBot commented Mar 2, 2019

Uh oh!

hebasto commented Jun 16, 2019

Uh oh!

flack commented Jun 16, 2019

Uh oh!

hebasto commented Jun 16, 2019

Uh oh!

maflcko left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

hebasto commented Feb 4, 2019 •

edited

Loading

DrahtBot commented Feb 4, 2019 •

edited

Loading

laanwj commented Feb 12, 2019 •

edited

Loading