Bitcoin-Qt: fixes for using display unit from options #2930

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:displayUnit changing 9 files +67 −50

Diapolo commented at 1:19 PM on August 24, 2013: none
- extend PaymentServer with setOptionsModel() and rework initNetManager() to make use of that
- fix all other places in the code to use display unit from options and no hard-coded unit
BitcoinPullTester commented at 2:26 PM on August 24, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/512379a27a0994d1056f06de2235e29f99fb399c for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.
in src/qt/transactiontablemodel.cpp:None in 512379a27a outdated
```
 222 |  TransactionTableModel::TransactionTableModel(CWallet* wallet, WalletModel *parent):
 223 |          QAbstractTableModel(parent),
 224 |          wallet(wallet),
 225 |          walletModel(parent),
 226 | -        priv(new TransactionTablePriv(wallet, this)),
 227 | +        priv(new TransactionTablePriv(wallet, this, parent->getOptionsModel()->getDisplayUnit())),
```
laanwj commented at 7:30 PM on August 28, 2013:

I think this will not update the unit when it's changed in option. And initializing here in the constructor fails when parent->getOptionsModel() is somehow zero. I think I'd prefer a setOptionsModel on TransactionTableModel that passes through to the private object, which is then used in describe().

Edit: or maybe make the unit a parameter of describe() and keep the OptionsModel only in the AddressTableModel. I don't really like having a pointer to the OptionsModel in another model -- ideally the unit should be part of the presentation / view not the model, but this would be pretty difficult here.

Diapolo commented at 8:35 AM on August 29, 2013:

I'm reworking this via your edit suggestion, which is easy to implement. But I'm asking myself, where or when the TransactionTableModel::TransactionTableModel() constructor is called? AFAIK there are no NULL pointer checks for walletModel, which seems strange!

laanwj commented at 2:29 PM on August 29, 2013:

TransactionTableModel is part of the WalletModel, so you can assume it to always exist. But if you want to add NULL pointer checks I won't stop you :)

Diapolo commented at 2:32 PM on August 29, 2013:

No thanks, I'm happy to see this merged ;).
laanwj commented at 7:30 PM on August 28, 2013: member

ACK on the paymentserver part

Bitcoin-Qt: fixes for using display unit from options

- extend PaymentServer with setOptionsModel() and rework initNetManager()
  to make use of that
- fix all other places in the code to use display unit from options and no
  hard-coded unit

bdd0c59ab0

laanwj referenced this in commit 1a12b3e090 on Aug 29, 2013
laanwj merged this on Aug 29, 2013
laanwj closed this on Aug 29, 2013
Diapolo deleted the branch on Aug 29, 2013
Bushstar referenced this in commit 428f304506 on Apr 8, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Diapolo

BitcoinPullTester

laanwj