Skip to content

Conversation

w0xlt
Copy link
Contributor

@w0xlt w0xlt commented Apr 13, 2022

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

@laanwj
Copy link
Member

laanwj commented Apr 13, 2022

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
Copy link
Member

hebasto commented Apr 13, 2022

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.

@laanwj
Copy link
Member

laanwj commented Apr 13, 2022

It does :)

Huh. Yes, it is. Sorry.

@hebasto
Copy link
Member

hebasto commented Apr 15, 2022

@w0xlt

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

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

@w0xlt w0xlt force-pushed the translator_comments_transactiondesc branch from ea83098 to 767304e Compare April 18, 2022 16:53
@w0xlt
Copy link
Contributor Author

w0xlt commented Apr 18, 2022

Rebased. Thanks.

Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 767304e

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

@hebasto
Copy link
Member

hebasto commented Apr 21, 2022

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

--- a/src/qt/transactiondesc.cpp
+++ b/src/qt/transactiondesc.cpp
@@ -38,8 +38,16 @@ QString TransactionDesc::FormatTxStatus(const interfaces::WalletTxStatus& status
     if (depth < 0) {
         return tr("conflicted with a transaction with %1 confirmations").arg(-depth);
     } else if (depth == 0) {
-        const QString abandoned{status.is_abandoned ? QLatin1String(", ") + tr("abandoned") : QString()};
-        return tr("0/unconfirmed, %1").arg(inMempool ? tr("in memory pool") : tr("not in memory pool")) + abandoned;
+        QString s;
+        if (inMempool) {
+            s = tr("0/unconfirmed, in memory pool");
+        } else {
+            s = tr("0/unconfirmed, not in memory pool");
+        }
+        if (status.is_abandoned) {
+            s += QLatin1String(", ") + tr("abandoned");
+        }
+        return s;
     } else if (depth < 6) {
         return tr("%1/unconfirmed").arg(depth);
     } else {

@w0xlt w0xlt force-pushed the translator_comments_transactiondesc branch from 767304e to b0ecfd9 Compare April 26, 2022 02:20
@w0xlt
Copy link
Contributor Author

w0xlt commented Apr 26, 2022

@hebasto thanks for suggestion. Done.

@katesalazar
Copy link

Concept ACK

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

@w0xlt w0xlt force-pushed the translator_comments_transactiondesc branch from b0ecfd9 to dde7e9a Compare June 1, 2022 17:55
@hebasto
Copy link
Member

hebasto commented Jun 2, 2022

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

What is your opinion?

@w0xlt w0xlt force-pushed the translator_comments_transactiondesc branch from dde7e9a to 8cfb562 Compare June 2, 2022 16:56
@w0xlt
Copy link
Contributor Author

w0xlt commented Jun 2, 2022

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

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 8cfb562

@hebasto hebasto merged commit b11ab25 into bitcoin-core:master Jun 2, 2022
@jonatack
Copy link
Member

jonatack commented Jun 2, 2022

ACK, thanks for updating.

@w0xlt w0xlt deleted the translator_comments_transactiondesc branch June 3, 2022 13:47
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jun 3, 2022
…sc::FormatTxStatus`

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

Pull request description:

  This PR adds translator comments to `TransactionDesc::FormatTxStatus` as suggested in bitcoin-core/gui#552 (comment) and bitcoin-core/gui#552 (comment).

ACKs for top commit:
  hebasto:
    ACK 8cfb562

Tree-SHA512: 2c44b915e6309508f34fc22bb90e3d88ad32ed82fdb3a395f7c6716941edc1b311991140d28e838ad622a7484ed86aedd25e55674857fec8716d9575aed25fa0
@bitcoin-core bitcoin-core locked and limited conversation to collaborators Jun 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants