Add translator comments to `TransactionDesc::FormatTxStatus`

w0xlt commented at 3:32 pm on April 13, 2022: contributor

This PR adds translator comments to TransactionDesc::FormatTxStatus as suggested in #552 (review) and #552 (comment).

laanwj commented at 6:24 pm on April 13, 2022: member

Concept ACK, but I think this belongs in the GUI repo. It also does more than add translator comments, which should be in the PR description.

hebasto commented at 6:27 pm on April 13, 2022: member

Concept ACK, but I think this belongs in the GUI repo.

It does :)

It also does more than add translator comments, which should be in the PR description.

This PR is based on top of the #552. Should be rebased after merging the latter. Only the last commit belongs to this PR.

hebasto added the label Translations on Apr 13, 2022

laanwj commented at 9:03 pm on April 13, 2022: member

It does :)

Huh. Yes, it is. Sorry.

hebasto commented at 9:39 am on April 15, 2022: member

@w0xlt

#552 has just been merged. Want to rebase this one?

And please finish every translator comment with a full stop character .

w0xlt force-pushed on Apr 18, 2022

w0xlt commented at 4:54 pm on April 18, 2022: contributor

Rebased. Thanks.

shaavan approved

shaavan commented at 12:57 pm on April 19, 2022: contributor

ACK 767304ee352e1c02bf9b5219555391b07473a5ff

The added translator’s comments are clean and concise and clearly represent the meaning of the sentences they intend to explain.

hebasto commented at 6:48 pm on April 21, 2022: member

To avoid unnecessary string split, which makes translation much harder, I’d suggest before applying translation comments to refactor a bit:

 0--- a/src/qt/transactiondesc.cpp
 1+++ b/src/qt/transactiondesc.cpp
 2@@ -38,8 +38,16 @@ QString TransactionDesc::FormatTxStatus(const interfaces::WalletTxStatus& status
 3     if (depth < 0) {
 4         return tr("conflicted with a transaction with %1 confirmations").arg(-depth);
 5     } else if (depth == 0) {
 6-        const QString abandoned{status.is_abandoned ? QLatin1String(", ") + tr("abandoned") : QString()};
 7-        return tr("0/unconfirmed, %1").arg(inMempool ? tr("in memory pool") : tr("not in memory pool")) + abandoned;
 8+        QString s;
 9+        if (inMempool) {
10+            s = tr("0/unconfirmed, in memory pool");
11+        } else {
12+            s = tr("0/unconfirmed, not in memory pool");
13+        }
14+        if (status.is_abandoned) {
15+            s += QLatin1String(", ") + tr("abandoned");
16+        }
17+        return s;
18     } else if (depth < 6) {
19         return tr("%1/unconfirmed").arg(depth);
20     } else {

w0xlt force-pushed on Apr 26, 2022

w0xlt commented at 2:23 am on April 26, 2022: contributor

@hebasto thanks for suggestion. Done.

in src/qt/transactiondesc.cpp:59 in b0ecfd9c52 outdated

56+        return s;
57     } else if (depth < 6) {
58+        //: This status represents a transaction confirmed in at least one block, but less than 6 blocks.
59         return tr("%1/unconfirmed").arg(depth);
60     } else {
61+        //: This status represents a transaction confirmed in 6 or more blocks.

katesalazar commented at 6:41 pm on April 26, 2022:

6 is a magic number, but you are not to blame.

katesalazar commented at 6:42 pm on April 26, 2022: contributor

Concept ACK

in src/qt/transactiondesc.cpp:39 in b0ecfd9c52 outdated

35@@ -36,13 +36,27 @@ QString TransactionDesc::FormatTxStatus(const interfaces::WalletTxStatus& status
36 {
37     int depth = status.depth_in_main_chain;
38     if (depth < 0) {
39+        //: This status represents an unconfirmed transaction that conflicts with another.

jarolrod commented at 6:30 pm on May 30, 2022:

Suggesting to add more context on the location of this string and what it represents:

 0diff --git a/src/qt/transactiondesc.cpp b/src/qt/transactiondesc.cpp
 1index c921b5fb1..0a52d2e3e 100644
 2--- a/src/qt/transactiondesc.cpp
 3+++ b/src/qt/transactiondesc.cpp
 4@@ -36,27 +36,41 @@ QString TransactionDesc::FormatTxStatus(const interfaces::WalletTxStatus& status
 5 {
 6     int depth = status.depth_in_main_chain;
 7     if (depth < 0) {
 8-        //: This status represents an unconfirmed transaction that conflicts with another.
 9+        /*: Text explaining the current status of a transaction, shown in the
10+            status field of the details window for this transaction. This status
11+            represents an unconfirmed transaction that conflicts with another
12+            confirmed transaction. */
13         return tr("conflicted with a transaction with %1 confirmations").arg(-depth);
14     } else if (depth == 0) {
15         QString s;
16         if (inMempool) {
17-            //: This status represents an unconfirmed transaction that is in the memory pool.
18+            /*: Text explaining the current status of a transaction, shown in the
19+                status field of the details window for this transaction. This status
20+                represents an unconfirmed transaction that is in the memory pool. */
21             s = tr("0/unconfirmed, in memory pool");
22         } else {
23-            //: This status represents an unconfirmed transaction that is not in the memory pool.
24+            /*: Text explaining the current status of a transaction, shown in the
25+                status field of the details window for this transaction. This status
26+                represents an unconfirmed transaction that is not in the memory pool. */
27             s = tr("0/unconfirmed, not in memory pool");
28         }
29         if (status.is_abandoned) {
30-            //: This status represents an abandoned transaction.
31+            /*: Text explaining the current status of a transaction, shown in the
32+                status field of the details window for this transaction. This
33+                status represents an abandoned transaction. */
34             s += QLatin1String(", ") + tr("abandoned");
35         }
36         return s;
37     } else if (depth < 6) {
38-        //: This status represents a transaction confirmed in at least one block, but less than 6 blocks.
39+        /*: Text explaining the current status of a transaction, shown in the
40+            status field of the details window for this transaction. This
41+            status represents a transaction confirmed in at least one block,
42+            but less than 6 blocks. */
43         return tr("%1/unconfirmed").arg(depth);
44     } else {
45-        //: This status represents a transaction confirmed in 6 or more blocks.
46+        /*: Text explaining the current status of a transaction, shown in the
47+            status field of the details window for this transaction. This status
48+            represents a transaction confirmed in 6 or more blocks. */
49         return tr("%1 confirmations").arg(depth);
50     }
51 }

jarolrod commented at 6:31 pm on May 30, 2022:

I have written about how translator comments should be written here: translator-comments.md

Extra information on what it represents to the user and where it is helps the translator think of a correct translation but also allows them to locate the string themselves in the GUI, which would also aid with translations.

I also have a branch with these suggested changes here: https://github.com/jarolrod/gui/tree/tc-fixup

w0xlt commented at 6:01 pm on June 1, 2022:

Thanks. Great article. I updated the code with your suggestions.

jarolrod commented at 6:32 pm on May 30, 2022: member

Concept ACK

w0xlt force-pushed on Jun 1, 2022

in src/qt/transactiondesc.cpp:41 in dde7e9ae23 outdated

35@@ -36,13 +36,41 @@ QString TransactionDesc::FormatTxStatus(const interfaces::WalletTxStatus& status
36 {
37     int depth = status.depth_in_main_chain;
38     if (depth < 0) {
39+        /*: Text explaining the current status of a transaction, shown in the
40+            status field of the details window for this transaction. This status
41+            represents an unconfirmed transaction that conflicts with another

jonatack commented at 9:31 pm on June 1, 2022:

“another” here is a little bit contradictory as it suggests that the other transaction is also unconfirmed; propose s/another/a/

w0xlt commented at 4:57 pm on June 2, 2022:

Done. Thanks.

in src/qt/transactiondesc.cpp:45 in dde7e9ae23 outdated

42+            confirmed transaction. */
43         return tr("conflicted with a transaction with %1 confirmations").arg(-depth);
44     } else if (depth == 0) {
45-        const QString abandoned{status.is_abandoned ? QLatin1String(", ") + tr("abandoned") : QString()};
46-        return tr("0/unconfirmed, %1").arg(inMempool ? tr("in memory pool") : tr("not in memory pool")) + abandoned;
47+        QString s;

jonatack commented at 9:41 pm on June 1, 2022:

 0@@ -42,25 +42,23 @@ QString TransactionDesc::FormatTxStatus(const interfaces::WalletTxStatus& status
 1             confirmed transaction. */
 2         return tr("conflicted with a transaction with %1 confirmations").arg(-depth);
 3     } else if (depth == 0) {
 4-        QString s;
 5         if (inMempool) {
 6             /*: Text explaining the current status of a transaction, shown in the
 7                 status field of the details window for this transaction. This status
 8                 represents an unconfirmed transaction that is in the memory pool. */
 9-            s = tr("0/unconfirmed, in memory pool");
10+            return tr("0/unconfirmed, in memory pool");
11         } else {
12             /*: Text explaining the current status of a transaction, shown in the
13                 status field of the details window for this transaction. This status
14                 represents an unconfirmed transaction that is not in the memory pool. */
15-            s = tr("0/unconfirmed, not in memory pool");
16+            return tr("0/unconfirmed, not in memory pool");
17         }
18         if (status.is_abandoned) {
19             /*: Text explaining the current status of a transaction, shown in the
20                 status field of the details window for this transaction. This
21                 status represents an abandoned transaction. */
22-            s += QLatin1String(", ") + tr("abandoned");
23+            return QLatin1String(", ") + tr("abandoned");
24         }
25-        return s;
26     } else if (depth < 6) {

hebasto commented at 9:53 pm on June 1, 2022:

What about s += QLatin1String(", ") + tr("abandoned"); ?

hebasto commented at 9:30 am on June 2, 2022: member

@w0xlt This looks ready to be merged with or without addressing #583 (review)

What is your opinion?

qt, refactor: add translator comments in `TransactionDesc::FormatTxStatus()` 8cfb5627d5

w0xlt force-pushed on Jun 2, 2022

w0xlt commented at 4:57 pm on June 2, 2022: contributor

@hebasto I addressed #583 (review) . Thanks.

hebasto approved

hebasto commented at 5:30 pm on June 2, 2022: member

ACK 8cfb5627d51ecaa1d1e92ec21e2ac56a380c77e6

hebasto merged this on Jun 2, 2022

hebasto closed this on Jun 2, 2022

jonatack commented at 5:39 pm on June 2, 2022: contributor

ACK, thanks for updating.

w0xlt deleted the branch on Jun 3, 2022

sidhujag referenced this in commit 243c842ae9 on Jun 3, 2022

bitcoin-core locked this on Jun 3, 2023

Add translator comments to TransactionDesc::FormatTxStatus #583

Add translator comments to `TransactionDesc::FormatTxStatus` #583