build: Include qt sources for parsing with extract_strings.py #22764

On master (4fc15d15667d9d9c4fb5515ce73c05b4596298ec) some strings are still untranslated.

This PR fixes this issue.

To verify:

1. ./autogen.sh && ./configure && make -C src translate before applying this change
2. apply this change
3. ./autogen.sh && ./configure && make -C src translate after applying this change

The result of git diff src/qt/bitcoinstrings.cpp:

--- a/src/qt/bitcoinstrings.cpp
+++ b/src/qt/bitcoinstrings.cpp
@@ -126,6 +126,7 @@ QT_TRANSLATE_NOOP("bitcoin-core", ""
 "You need to rebuild the database using -reindex to go back to unpruned "
 "mode.  This will redownload the entire blockchain"),
 QT_TRANSLATE_NOOP("bitcoin-core", "%s is set very high!"),
+QT_TRANSLATE_NOOP("bitcoin-core", "(press q to shutdown and continue later)"),
 QT_TRANSLATE_NOOP("bitcoin-core", "-maxmempool must be at least %d MB"),
 QT_TRANSLATE_NOOP("bitcoin-core", "A fatal internal error occurred, see debug.log for details"),
 QT_TRANSLATE_NOOP("bitcoin-core", "Cannot resolve -%s address: '%s'"),
@@ -204,6 +205,8 @@ QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Failed to prepare statement t
 QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Failed to read database verification error: %s"),
 QT_TRANSLATE_NOOP("bitcoin-core", "SQLiteDatabase: Unexpected application id. Expected %u, got %u"),
 QT_TRANSLATE_NOOP("bitcoin-core", "Section [%s] is not recognized."),
+QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be read"),
+QT_TRANSLATE_NOOP("bitcoin-core", "Settings file could not be written"),
 QT_TRANSLATE_NOOP("bitcoin-core", "Signing transaction failed"),
 QT_TRANSLATE_NOOP("bitcoin-core", "Specified -walletdir \"%s\" does not exist"),
 QT_TRANSLATE_NOOP("bitcoin-core", "Specified -walletdir \"%s\" is a relative path"),
@@ -242,4 +245,5 @@ QT_TRANSLATE_NOOP("bitcoin-core", "User Agent comment (%s) contains unsafe chara
 QT_TRANSLATE_NOOP("bitcoin-core", "Verifying blocks…"),
 QT_TRANSLATE_NOOP("bitcoin-core", "Verifying wallet(s)…"),
 QT_TRANSLATE_NOOP("bitcoin-core", "Wallet needed to be rewritten: restart %s to complete"),
+QT_TRANSLATE_NOOP("bitcoin-core", "press q to shutdown"),
 };

GUIX hashes

find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum

c58ff4550f95f70b8a6f63afc39a8e47d2ddf7fecd53b2e282315ff1a8b9de8a  guix-build-aff62dab5a7c/output/aarch64-linux-gnu/SHA256SUMS.part
962b35dbb264703ae1a7321e956cbc4f84f8b5684cdc103a83acd2ea04f039e2  guix-build-aff62dab5a7c/output/aarch64-linux-gnu/bitcoin-aff62dab5a7c-aarch64-linux-gnu-debug.tar.gz
e7b14a74a21400b146c387ed76faa51e45d4096e158b7eb5fbb44994bd39abff  guix-build-aff62dab5a7c/output/aarch64-linux-gnu/bitcoin-aff62dab5a7c-aarch64-linux-gnu.tar.gz
22d04419416c248180d3ac371987f49f89bea6e26f53260968dac137ccb94585  guix-build-aff62dab5a7c/output/arm-linux-gnueabihf/SHA256SUMS.part
132a2ae0228cb051e35c31a256e848a76d0e94ba410b9f75a0a20b3d3634f764  guix-build-aff62dab5a7c/output/arm-linux-gnueabihf/bitcoin-aff62dab5a7c-arm-linux-gnueabihf-debug.tar.gz
96a0b0eca6fa5e48a53f9681492a99141f5ca6283b456c21d2d255dcf21aace7  guix-build-aff62dab5a7c/output/arm-linux-gnueabihf/bitcoin-aff62dab5a7c-arm-linux-gnueabihf.tar.gz
65618f0c8a2f332ce322e35ed1c9c32ec966ff615a350451e5fd3da39fe8c825  guix-build-aff62dab5a7c/output/dist-archive/bitcoin-aff62dab5a7c.tar.gz
81e66d916ae48df497ab72ba2ab9b73ff6ce7c417d7b0eb65372d1647ec37f7a  guix-build-aff62dab5a7c/output/powerpc64-linux-gnu/SHA256SUMS.part
1c2a8c860fa55deea4b98f08ac5debd027bfe0d24c338815ac8211ca06c40932  guix-build-aff62dab5a7c/output/powerpc64-linux-gnu/bitcoin-aff62dab5a7c-powerpc64-linux-gnu-debug.tar.gz
9ba1be7511f50e8b980943ad1747ef6b84e39ed55be00954faf05ad086ad2c53  guix-build-aff62dab5a7c/output/powerpc64-linux-gnu/bitcoin-aff62dab5a7c-powerpc64-linux-gnu.tar.gz
b9edbcb8c61fe0ac63f88553d655dc3aff605182ffc497e96707bd5464dfc43c  guix-build-aff62dab5a7c/output/powerpc64le-linux-gnu/SHA256SUMS.part
bcb194e4bebfa6d99e7445cfd10c3dc026f340fcd95234f5eb06104d54c8b01e  guix-build-aff62dab5a7c/output/powerpc64le-linux-gnu/bitcoin-aff62dab5a7c-powerpc64le-linux-gnu-debug.tar.gz
0c11e762ff43c0f35a7a674dc022ac280fc382b692fdc97d3528fd802d89eac4  guix-build-aff62dab5a7c/output/powerpc64le-linux-gnu/bitcoin-aff62dab5a7c-powerpc64le-linux-gnu.tar.gz
df28eef1198bb5036ea34b353135d1edbac4e1ef8cad062d895a422a21f11d1c  guix-build-aff62dab5a7c/output/riscv64-linux-gnu/SHA256SUMS.part
20690073e883da112117fdbbf80da738c9eefef08a00cd66f8474aa8aceb5f12  guix-build-aff62dab5a7c/output/riscv64-linux-gnu/bitcoin-aff62dab5a7c-riscv64-linux-gnu-debug.tar.gz
1ce7b20e91758e5d3ae6061cda39621f3408d84b1705a87a6aa45ac6cf102865  guix-build-aff62dab5a7c/output/riscv64-linux-gnu/bitcoin-aff62dab5a7c-riscv64-linux-gnu.tar.gz
e62e10042fee2d96689eb910d168130b96421f7a8af2d84a6faae75f73597b34  guix-build-aff62dab5a7c/output/x86_64-apple-darwin18/SHA256SUMS.part
13cc71fa26e4a4b1f996e697374a1fbd049339fca1e5deadf625aea231ba39c2  guix-build-aff62dab5a7c/output/x86_64-apple-darwin18/bitcoin-aff62dab5a7c-osx-unsigned.dmg
17a0e7d713528354af1960cf8eef346744477fba591f7f677715a8a74616c325  guix-build-aff62dab5a7c/output/x86_64-apple-darwin18/bitcoin-aff62dab5a7c-osx-unsigned.tar.gz
67eacc028601e4b49cca8d44cfc616743513be5617419266af72d73d9824b550  guix-build-aff62dab5a7c/output/x86_64-apple-darwin18/bitcoin-aff62dab5a7c-osx64.tar.gz
ea53908436682c2059b1f9c9580187ff8e9635f6580d94132e36fea0cd966d41  guix-build-aff62dab5a7c/output/x86_64-linux-gnu/SHA256SUMS.part
81196f235135b0313c18f94dfffc7f85351a205016f26200353c1e308c79e5ff  guix-build-aff62dab5a7c/output/x86_64-linux-gnu/bitcoin-aff62dab5a7c-x86_64-linux-gnu-debug.tar.gz
fc75cb695ff941e1c874b7d5ee59f0ec701066778f850b0261e264e7780722f2  guix-build-aff62dab5a7c/output/x86_64-linux-gnu/bitcoin-aff62dab5a7c-x86_64-linux-gnu.tar.gz
2e8ccc5e396b08880c393c26b5db63001aca01cf87bcee4eb3c9d7254db40b78  guix-build-aff62dab5a7c/output/x86_64-w64-mingw32/SHA256SUMS.part
3ed45c53e121066c73c53c75cf89e481be7be5f50d8ae45282e72f8b8e56e4cf  guix-build-aff62dab5a7c/output/x86_64-w64-mingw32/bitcoin-aff62dab5a7c-win-unsigned.tar.gz
b6310076633b6eddeae958f0ec36080bb3e0275ed334b316b97c19caf43561f7  guix-build-aff62dab5a7c/output/x86_64-w64-mingw32/bitcoin-aff62dab5a7c-win64-debug.zip
e5a2c7ff04d63074faa5fe9e7aa6ea07f107f2a88dc5bde404a9cb0bbba86faa  guix-build-aff62dab5a7c/output/x86_64-w64-mingw32/bitcoin-aff62dab5a7c-win64-setup-unsigned.exe
ffdd25e3942ea07ce8e9eba5ebc633d883afb543290afd092233f5136e6a3097  guix-build-aff62dab5a7c/output/x86_64-w64-mingw32/bitcoin-aff62dab5a7c-win64.zip

So this adds missing strings like "(press q to shutdown and continue later)", to bitcoinstrings.cpp, but another translated string like "press q to shutdown", from the same line in our source still seems to be missing. Is there a reason for that? (I haven't tested this at all).

@fanquake

So this adds missing strings like "(press q to shutdown and continue later)", to bitcoinstrings.cpp, but another translated string like "press q to shutdown", from the same line in our source still seems to be missing.

Both are added :)

Wait, the idea is that inside the Qt sources, tr( is used which is picked up automatically. The python script shouldn't be necessary for those.

These shouldn't be here at all!

src/qt/bitcoin.cpp:        bilingual_str error = _("Settings file could not be read");
src/qt/bitcoin.cpp:        bilingual_str error = _("Settings file could not be written");
src/qt/splashscreen.cpp:            (resume_possible ? _("(press q to shutdown and continue later)").translated
src/qt/splashscreen.cpp:                                : _("press q to shutdown").translated) +

See bitcoin-core/gui#454

See bitcoin-core/gui#454

Closing in favor of https://github.com/bitcoin-core/gui/pull/454.

Reopening this to find out if it makes sense to take it up again.

Wait, the idea is that inside the Qt sources, tr( is used which is picked up automatically. The python script shouldn't be necessary for those. @laanwj I agree it is good to use QObject::tr() instead of _() for display-only translations. But I think it makes sense to use _() for bilingual translations that aren't purely for display, but which are also logged untranslated to debug.log. The GUI should probably mostly contain QObject::tr() strings that are always translated, but for critical errors bilingual _() strings that can be logged and displayed are useful. The _() function returns and formats bilingual strings easily, while QObject::tr() requires more awkwardness and repetition to create bilingual strings.

We could introduce a new mechanism that wraps QObject::tr() and formats bilingual strings, but _() already works, so I wonder why we wouldn't just extend the extract_strings_qt.py command line and allow using it in Qt code.

Rebased.

Code review ACK b59b31ae0b04054c5cf225dad87046d3771707fc. Being able to use _() macro in qt would allow simplifying some code, for example replacing repetitive:

std::string quoted_path = strprintf("%s", fs::quoted(fs::PathToString(settings_path)));
error.original += strprintf("Settings file %s might be corrupt or invalid.", quoted_path);
error.translated += tr("Settings file %1 might be corrupt or invalid.").arg(QString::fromStdString(quoted_path)).toStdString();

from https://github.com/bitcoin-core/gui/pull/602#pullrequestreview-991088266 with simpler:

error += strprintf(_("Settings file %s might be corrupt or invalid."), fs::quoted(fs::PathToString(settings_path)));

Most strings shown in the GUI should be translated-only, not bilingual, so QObject::tr() is a better choice than _() to provide the translation. But _() is useful for special cases like critical errors where bilingual_str provides a translated string for display and an untranslated string for logs and details.

@laanwj I agree it is good to use QObject::tr() instead of _() for display-only translations. But I think it makes sense to use _() for bilingual translations that aren't purely for display, but which are also logged untranslated to debug.log.

Conceptually I agree. There are some exceptions where it can be useful.

My aversion to this is mostly on a code-hygienic level. _() introduces a dispatch through the ui_interface, core code, going back to Qt. We already know the ui_interface is connected to Qt, for sake of running the Qt GUI, but… i dunno

My aversion to this is mostly on a code-hygienic level. _() introduces a dispatch through the ui_interface, core code, going back to Qt. We already know the ui_interface is connected to Qt, for sake of running the Qt GUI, but… i dunno

If it helps, it looks like this is not using ui_interface stuff presently. _ function is defined to call G_TRANSLATION_FUN pointer:

https://github.com/bitcoin/bitcoin/blob/01e9e2d1ca7fc08d65663b398c71ecec6a01f686/src/util/translation.h#L63-L66

which is calling QCoreApplication::translate:

https://github.com/bitcoin/bitcoin/blob/01e9e2d1ca7fc08d65663b398c71ecec6a01f686/src/qt/main.cpp#L17-L19

So it is a little less efficient than calling Qt directly, but it's not pulling in many more dependencies besides the bilingual_str functions in util/translation.h which it is trying to use.

@jarolrod you might want to review this?

<!--e57a25ab6845829454e8d69fc972939a-->

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

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage

For detailed information about the code coverage, see the test coverage report.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

concept ack

Concept ACK

@jarolrod want to followup here?

<!--8ac04cdde196e94527acabf64b896448-->

There hasn't been much activity lately. What is the status here?

Finding reviewers may take time. However, if the patch is no longer relevant, please close this pull request. If the author lost interest or time to work on this, please close it and mark it 'Up for grabs' with the label, so that it can be picked up in the future.

ACK b59b31ae0b04054c5cf225dad87046d3771707fc