[Qt] Don't translate warning messages #7134

pull jonasschnelli wants to merge 2 commits into bitcoin:master from jonasschnelli:2015/11/qt_getwarnings changing 2 files +7 −4

jonasschnelli commented at 1:51 PM on November 30, 2015: contributor

Translating RPC response-values seems to be a bad idea. This PR disabled translating of RPC responses when the GUI is enabled/running. The info message in the GUI overview page will no longer be translated.

Fixes #5895
in src/main.cpp:None in be565df03f outdated
```
4006 | +        strStatusBar = strRPC = "Warning: We do not appear to fully agree with our peers! You may need to upgrade, or other nodes may need to upgrade.";
4007 |      }
4008 |  
4009 | +    // try to i18n the string
4010 | +    if (translate)
4011 | +        strStatusBar = _(strStatusBar.c_str());
```
MarcoFalke commented at 1:58 PM on November 30, 2015:

Does this even work after translation update? I assume this at least does not translate anymore when any of the stings get changed?

MarcoFalke commented at 1:58 PM on November 30, 2015:

Why bother keeping?

jonasschnelli commented at 2:04 PM on November 30, 2015:

We might want to keep the info string on the overview page i18n'ed while avoiding translation on RPC level (check the screen):

<img width="938" alt="bildschirmfoto 2015-11-30 um 14 45 39" src="https://cloud.githubusercontent.com/assets/178464/11473588/8e036990-9773-11e5-8180-be395c77789c.png">

MarcoFalke commented at 2:18 PM on November 30, 2015:

Imo, you can't do this. The translations will go away.

MarcoFalke commented at 2:19 PM on November 30, 2015:

You could try make translate in /src and make the screenshot again. I am assuming it will show English.

jonasschnelli commented at 2:22 PM on November 30, 2015:

The translations will go away.

Hmm... what exactly do you mean with this?

If I'm right, strStatusBar = _(strStatusBar.c_str()); should translate the string if possible, and if not, keep the existing string. Future changes of the original string or of translations should not be a problem.

Or am I blind somewhere?

jonasschnelli commented at 2:28 PM on November 30, 2015:

Ah. I get you point. The missing _(<string>) will prevent the python script from auto-extracting strings-that-require translation. Hmm... right. This is suboptimal.
MarcoFalke commented at 2:29 PM on November 30, 2015:

@jonasschnelli Try

cd src make translate cd .. make src/qt/bitcoin-qt -regtest -lang=pl_PL& screenshot
laanwj commented at 2:56 PM on November 30, 2015:

Yes, you must only pass static strings into _(), otherwise the extraction won't work. If you must do this, just copy the message, or use a similar message.

laanwj commented at 2:57 PM on November 30, 2015:

I'd say just remove the translations of warnings/error messages completely.

MarcoFalke commented at 3:04 PM on November 30, 2015:

just copy the message, or use a similar message.

+1
jonasschnelli added the label GUI on Nov 30, 2015
jonasschnelli added the label RPC on Nov 30, 2015
MarcoFalke commented at 2:34 PM on November 30, 2015: member

Concept ACK
[Qt] Don't translate GetWarnings() for RPC responses 86e905b16b
jonasschnelli force-pushed on Nov 30, 2015
jonasschnelli renamed this:
~~[Qt] Don't translate GetWarnings() for RPC responses~~
[Qt] Don't translate GetWarnings()
on Nov 30, 2015
jonasschnelli renamed this:
~~[Qt] Don't translate GetWarnings()~~
[Qt] Don't translate warning messages
on Nov 30, 2015
jonasschnelli commented at 3:56 PM on November 30, 2015: contributor

Updated. Remove translations for GetWarnings() entirely. Will also remove the translation of the GUI warning string in the overview page.

in src/main.cpp:None in 86e905b16b outdated

3980 | @@ -3981,7 +3981,7 @@ std::string GetWarnings(const std::string& strFor)
3981 |      string strRPC;
3982 |  
3983 |      if (!CLIENT_VERSION_IS_RELEASE)
3984 | -        strStatusBar = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications");
3985 | +        strStatusBar = "This is a pre-release test build - use at your own risk - do not use for mining or merchant applications";

MarcoFalke commented at 4:03 PM on November 30, 2015:

It seems this is not copy-paste able in the qt client. I imagine those would be hard to translate for some users.

laanwj commented at 4:07 PM on November 30, 2015:

Good idea, let's make it copy/pastable then.

laanwj commented at 4:06 PM on November 30, 2015: member

utACK Yeah, this is also better with regard to Google-ability of the errors
[Qt] make warnings label selectable c52a2922f0
jonasschnelli commented at 7:59 PM on November 30, 2015: contributor

Added a commit that makes the warnings label selectable (allow to copy text either with context menu or Ctrl-C).
paveljanik commented at 6:54 AM on December 1, 2015: contributor

copy&paste: OK. Helps to solve the googlability problem.

But I do not agree with seeing an English sentence on the otherwise German application. That calls for more issue coming from our users "This sentence is untranslated". NACK here.
jonasschnelli commented at 7:29 AM on December 1, 2015: contributor

@paveljanik: I think it's okay to show a beta warning in english. Also in very rare situations where a large fork has found, i think it's even better to show this in english. People will be concerned and might look up the error string somewhere (or past in on forumrs/IRC).
paveljanik commented at 7:33 AM on December 1, 2015: contributor

Especially when some rare case happens, we should explain to user what happened :-) In his preferred language. In my previous life, I was a head of several localization teams and my experience says that having mixed languages in the UI is a way to hell.
laanwj commented at 8:32 AM on December 1, 2015: member

But I do not agree with seeing an English sentence on the otherwise German application.

Well in that case this should be solved differently: the core should send a constant error code to the UI for known errors, and the UI converts this into a (translated) text.

To be honest I think the whole GetWarnings() mechanism, with errors with priority levels that override each other in confusing ways, and are reported on the to-be-deprecated getinfo. is kind of crappy, see also #7130, so I wouldn't object to a deeper redesign.

Edit: but that shouldn't block this pull, which solves a legitimate issue. It's strange to have locale-specific errors returned when the GUI is running and not when bitcoind does. These errors and warnings are, overall, pretty rare so I don't see it as a big problem to have them untranslated for a while.
laanwj commented at 8:39 AM on December 1, 2015: member

Well you know what, let's just duplicate the messages for now. There are already translations so having them get lost is a bit of a waste. Going to submit an alternative solution for this in a bit.
laanwj referenced this in commit 99be01c44b on Dec 1, 2015
laanwj commented at 8:49 AM on December 1, 2015: member

See #7141
laanwj closed this on Dec 1, 2015
laanwj referenced this in commit 1a514e6760 on Dec 1, 2015
laanwj referenced this in commit aabc897801 on Dec 1, 2015
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#5895 Localization bug w/ unicode in debug console #7141 rpc: Don't translate warning messages