TransactionDesc::FormatTxStatus and TransactionStatus
#552
This PR implements the changes suggested in #538 (comment) .
. remove redundant scope, rename nDepth -> depth, remove unused parameters and add translator comments in TransactionDesc::FormatTxStatus()
. Use member initializers and remove unused field qint64 TransactionStatus::open_for in TransactionStatus.
0@@ -1,4 +1,4 @@
1-// Copyright (c) 2011-2021 The Bitcoin Core developers
2+// Copyright (c) 2011-2022 The Bitcoin Core developers
34@@ -40,22 +35,19 @@ class TransactionStatus
35 };
36
37 /// Transaction counts towards available balance
38- bool countsForBalance;
39+ bool countsForBalance{false};
40 /// Sorting key based on status
41- std::string sortKey;
42+ std::string sortKey{};
25-public:
26- TransactionStatus() : countsForBalance(false), sortKey(""),
27- matures_in(0), status(Unconfirmed), depth(0), open_for(0)
28- { }
29+struct TransactionStatus {
30
clang-format-diff.py after each commit is a good habit :)
Approach ACK 0b234f96eaf85bcbe106e12750eefc57e009170d.
In 0b234f96eaf85bcbe106e12750eefc57e009170d “qt: remove redundant scope”:
nDepth/depth/const interfaces::WalletTx& wtx and int numBlocks are not used, and could be dropped (maybe in a separated commit)In b0ffe4e47c0500bcd54f82e6b88a0941cae9485e “qt: refactor TransactionStatus”:
open_for member could be split into a separated commitbool needsUpdate is still uninitializedAlso could you please, in addition to the link to #538, add actual descriptions of this PR changes to OP, because it goes into a merge commit message.
49 @{*/
50- Status status;
51- qint64 depth;
52+ Status status{Unconfirmed};
53+ qint64 depth{0};
54 qint64 open_for; /**< Timestamp if status==OpenUntilDate, otherwise number
b279331569e42cab721949d6669a452d6ba1558a
nit, initialize open_for or move this commit after 064bbdf243?
35@@ -36,13 +36,17 @@ 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
40 return tr("conflicted with a transaction with %1 confirmations").arg(-depth);
41 } else if (depth == 0) {
42 const QString abandoned{status.is_abandoned ? QLatin1String(", ") + tr("abandoned") : QString()};
43+ //: This status represents an unconfirmed transaction that is in the memory pool or possibly abandoned
44 return tr("0/unconfirmed, %1").arg(inMempool ? tr("in memory pool") : tr("not in memory pool")) + abandoned;
A translator comments applied to a line with multiple independent translatable strings will only apply to the first one. You can check what translator comment is applying to what string by making the translations, see more here: translation_process.md
You could break this into multiple lines in order to give translator comments to the strings in memory pool and not in memory pool. Seems useful to provide context for this strings.
-BEGIN VERIFY SCRIPT-
s() { sed -i -e 's/nDepth/depth/g' $(git grep -l 'nDepth' $1); }
s src/qt/transactiondesc.cpp
-END VERIFY SCRIPT-Almost ACK 2c5dbf2fc4556ff83009eca2178f860efcb73f8e except for 62ab92b5e992d9c5bbe251632acbc7a5df517510 “qt, refactor: add translator comments in TransactionDesc::FormatTxStatus()”.
If we are adding translator comments, we should also strive to prevent splitting of strings into less meaningful parts.
Suggesting to separate that commit into a dedicated PR, as all of other commits are good to be merged.
@w0xlt Please name this PR in a bit meaningful way because the PR title goes into the project commit history.
Mind having another (final?) look? @MarcoFalke This PR addresses your #538 (comment). Does it look ok?
Code Review ACK https://github.com/bitcoin-core/gui/commit/343f83d0886ae39c9dacb29762ce712711b2bad2
Thanks for working on this refactoring. I have reviewed the code and agree that it can be merged.
