Pass bilingual_str argument to AbortNode() by hebasto · Pull Request #18927 · bitcoin/bitcoin

hebasto · 2020-05-10T10:12:21Z

This PR is a followup of #16224, and it adds bilingual_str type argument support to the AbortNode() functions.

hebasto · 2020-05-10T10:15:14Z

This PR could be tested with a patch similar to the following:

diff --git a/src/qt/locale/bitcoin_nl.ts b/src/qt/locale/bitcoin_nl.ts
index 73c18cd4d..13bda2db2 100644
--- a/src/qt/locale/bitcoin_nl.ts
+++ b/src/qt/locale/bitcoin_nl.ts
@@ -3524,8 +3524,8 @@ Notitie: Omdat de vergoeding per byte wordt gerekend, zal een vergoeding van "10
         <translation>P2P-adressen aan het laden...</translation>
     </message>
     <message>
-        <source>Error: Disk space is too low!</source>
-        <translation>Error: Opslagruimte te weinig!</translation>
+        <source>Disk space is too low!</source>
+        <translation>Opslagruimte te weinig!</translation>
     </message>
     <message>
         <source>Loading banlist...</source>
diff --git a/src/validation.cpp b/src/validation.cpp
index 9034f66e9..c9539c325 100644
--- a/src/validation.cpp
+++ b/src/validation.cpp
@@ -2317,7 +2317,7 @@ bool CChainState::FlushStateToDisk(
         // Write blocks and block index to disk.
         if (fDoFullFlush || fPeriodicWrite) {
             // Depend on nMinDiskSpace to ensure we can write block index
-            if (!CheckDiskSpace(GetBlocksDir())) {
+            if (CheckDiskSpace(GetBlocksDir())) {
                 return AbortNode(state, "Disk space is too low!", _("Disk space is too low!"));
             }
             {

$ ./src/qt/bitcoin-qt -testnet -lang=nl
Error: Disk space is too low!
Error: Disk space is too low!
Error: Disk space is too low!

$ tail -4 ~/.bitcoin/testnet3/debug.log 
2020-05-10T09:55:41Z [shutoff] *** Disk space is too low!
2020-05-10T09:55:41Z [shutoff] Error: Disk space is too low!
2020-05-10T09:55:41Z [shutoff] ForceFlushStateToDisk: failed to flush state (Disk space is too low!)
2020-05-10T09:55:41Z [shutoff] Shutdown: done

src/util/translation.h

hebasto · 2020-05-10T12:32:27Z

Updated 1511dd0 -> 6fad0cf (pr18927.02 -> pr18927.03, diff):

addressed @MarcoFalke's comment:

I think translators should not be able to influence executions paths by blanking messages

DrahtBot · 2020-05-10T13:37:24Z

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

Conflicts

No conflicts as of last run.

hebasto · 2020-05-10T14:08:21Z

Updated 6fad0cf -> 4067da9 (pr18927.03 -> pr18927.04, diff):

more uses of bilingual_str::empty()

hebasto · 2020-05-14T04:53:06Z

Rebased 4067da9 -> c5e34e5 (pr18927.04 -> pr18927.05) due to the conflict with #18922.

maflcko · 2020-06-04T15:26:07Z

Needs rebase

Since bilingual_str type is fully supported, the MSG_NOPREFIX flag is no longer needed.

hebasto · 2020-06-04T15:34:41Z

@MarcoFalke

Needs rebase

Rebased c5e34e5 -> d924f2a (pr18927.05 -> pr18927.07).

maflcko · 2020-06-07T12:24:26Z

src/validation.cpp

+        uiInterface.ThreadSafeMessageBox(userMessage, "", CClientUIInterface::MSG_ERROR);
    } else {
-        uiInterface.ThreadSafeMessageBox(_("Error: A fatal internal error occurred, see debug.log for details"), "", CClientUIInterface::MSG_ERROR | CClientUIInterface::MSG_NOPREFIX);
+        uiInterface.ThreadSafeMessageBox(_("A fatal internal error occurred, see debug.log for details"), "", CClientUIInterface::MSG_ERROR);


Now that the signature is identical to InitError, the call could be simplified to just a call to AbortError, which would be a new alias for InitError?

constexpr auto AbortError = InitError;

Now that the signature is identical to InitError...

Which function signature do you mean?

maflcko · 2020-06-08T10:46:19Z

src/validation.cpp

+        uiInterface.ThreadSafeMessageBox(userMessage, "", CClientUIInterface::MSG_ERROR);
    } else {
-        uiInterface.ThreadSafeMessageBox(_("Error: A fatal internal error occurred, see debug.log for details"), "", CClientUIInterface::MSG_ERROR | CClientUIInterface::MSG_NOPREFIX);
+        uiInterface.ThreadSafeMessageBox(_("A fatal internal error occurred, see debug.log for details"), "", CClientUIInterface::MSG_ERROR);


All that InitError does is pass a translated string into ThreadSafeMessageBox, so this line could be replaced by a simple call to InitError, no?

Suggested change

uiInterface.ThreadSafeMessageBox(_("A fatal internal error occurred, see debug.log for details"), "", CClientUIInterface::MSG_ERROR);

AbortError(_("A fatal internal error occurred, see debug.log for details"));

hebasto · 2020-06-08T15:40:02Z

Updated d924f2a -> 5527be0 (pr18927.07 -> pr18927.08, diff):

implemented @MarcoFalke's suggestion (Pass bilingual_str argument to AbortNode() #18927 (comment), Pass bilingual_str argument to AbortNode() #18927 (comment))

maflcko · 2020-06-09T21:51:26Z

ACK 5527be0 👟

Show signature and timestamp

Signature:

Timestamp of file with hash c4a6904cff17d24ba7d628d970246ed0ce8d86a0caa4d68859d702cd905c7c6a -

fa02b47 refactor: Use AbortError in FatalError (MarcoFalke) Pull request description: `FatalError` has been copied from `AbortNode`, so the two should use the same style to avoid confusion. Follow-up to #18927 ACKs for top commit: hebasto: ACK fa02b47, I have reviewed the code and it looks OK, I agree it can be merged. Tree-SHA512: 2cf6d18a6ffb5c2e5cf54f0a072a7cef6dc7e924152b2fee44e6ff2c6c53bad962afd364eda30d8a73883d656429ea68391090e6a27057e69eaefd7c4dad0a33

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

Summary: ``` This PR is a followup of #16224, and it adds bilingual_str type argument support to the AbortNode() functions. ``` Backport of core [[bitcoin/bitcoin#18927 | PR18927]]. Depends on D8648. Test Plan: ninja all check-all Reviewers: #bitcoin_abc, jasonbcox Reviewed By: #bitcoin_abc, jasonbcox Differential Revision: https://reviews.bitcoinabc.org/D8649

hebasto force-pushed the 200510-abort branch from 71e9391 to 1511dd0 Compare May 10, 2020 10:26

DrahtBot added GUI Utils/log/libs Validation labels May 10, 2020

maflcko reviewed May 10, 2020

View reviewed changes

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

hebasto force-pushed the 200510-abort branch from 1511dd0 to 6fad0cf Compare May 10, 2020 12:29

DrahtBot mentioned this pull request May 10, 2020

gui: Do not translate InitWarning messages in debug.log #18922

Merged

hebasto force-pushed the 200510-abort branch from 6fad0cf to 4067da9 Compare May 10, 2020 14:06

DrahtBot added the Needs rebase label May 13, 2020

hebasto force-pushed the 200510-abort branch from 4067da9 to c5e34e5 Compare May 14, 2020 04:51

DrahtBot removed the Needs rebase label May 14, 2020

fanquake mentioned this pull request May 27, 2020

doc: Separate repository for the gui #19071

Merged

hebasto force-pushed the 200510-abort branch from c5e34e5 to f024ba7 Compare June 4, 2020 15:29

hebasto added 3 commits June 4, 2020 18:32

refactor: Use bilingual_str::empty()

d1cca12

Pass bilingual_str argument to AbortNode()

083daf7

Drop MSG_NOPREFIX flag

d924f2a

Since bilingual_str type is fully supported, the MSG_NOPREFIX flag is no longer needed.

hebasto force-pushed the 200510-abort branch from f024ba7 to d924f2a Compare June 4, 2020 15:33

hebasto closed this Jun 6, 2020

hebasto reopened this Jun 6, 2020

hebasto closed this Jun 6, 2020

hebasto reopened this Jun 6, 2020

maflcko reviewed Jun 7, 2020

View reviewed changes

maflcko reviewed Jun 8, 2020

View reviewed changes

refactor: Add AbortError alias

5527be0

hebasto closed this Jun 9, 2020

hebasto reopened this Jun 9, 2020

hebasto closed this Jun 9, 2020

hebasto reopened this Jun 9, 2020

hebasto mentioned this pull request Jun 10, 2020

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

Closed

maflcko merged commit 4b30c41 into bitcoin:master Jun 16, 2020

hebasto deleted the 200510-abort branch June 16, 2020 13:55

maflcko mentioned this pull request Jun 16, 2020

refactor: Use AbortError in FatalError #19295

Merged

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

	uiInterface.ThreadSafeMessageBox(_("A fatal internal error occurred, see debug.log for details"), "", CClientUIInterface::MSG_ERROR);
	AbortError(_("A fatal internal error occurred, see debug.log for details"));

Conversation

hebasto commented May 10, 2020

Uh oh!

hebasto commented May 10, 2020

Uh oh!

Uh oh!

hebasto commented May 10, 2020

Uh oh!

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

Uh oh!

Conflicts

Uh oh!

hebasto commented May 10, 2020

Uh oh!

hebasto commented May 14, 2020

Uh oh!

maflcko commented Jun 4, 2020

Uh oh!

hebasto commented Jun 4, 2020

Uh oh!

maflcko Jun 7, 2020

Choose a reason for hiding this comment

Uh oh!

hebasto Jun 8, 2020

Choose a reason for hiding this comment

Uh oh!

hebasto Jun 8, 2020

Choose a reason for hiding this comment

Uh oh!

maflcko Jun 8, 2020

Choose a reason for hiding this comment

Uh oh!

hebasto Jun 8, 2020

Choose a reason for hiding this comment

Uh oh!

hebasto commented Jun 8, 2020

Uh oh!

maflcko commented Jun 9, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DrahtBot commented May 10, 2020 •

edited

Loading