use new message() function in BitcoinGUI #2045

pull Diapolo wants to merge 1 commits into bitcoin:master from Diapolo:use_message changing 2 files +31 −31

Diapolo commented at 9:09 PM on November 27, 2012: none
- use it for displaying URI parsing warnings
- use it for displaying error and information in backup wallet function (the information display is new and the error was a warning before)
- cleanup BitcoinGUI::incomingTransaction() -- use message() + the information icon from message -- comment out an unused parameter in the function definition and declaration -- move all pre-checks at the beginning of the function @laanwj There is this comment in qmessagebox.h (// the following functions are obsolete) that lists the static QMessageBox functions (warning, information and so on) as obsolete, so I think we should talk about replacing them over the next months.
BitcoinPullTester commented at 9:31 PM on November 27, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/49d58d2d1b862112fa6a4f7fbfb45d726b023c25 for binaries and test log.
BitcoinPullTester commented at 10:27 PM on November 27, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/8f824def5784ab9177803af9ce561e4ffd3a4eac for binaries and test log.

use new message() function in BitcoinGUI

- use it for displaying URI parsing warnings
- use it for displaying error and information in backup wallet function
  (the information display is new and the error was a warning before)

- cleanup BitcoinGUI::incomingTransaction()
-- use message() + the information icon from message
-- comment out an unused parameter in the function definition and
   declaration
-- move all pre-checks at the beginning of the function

50ecd7b689

in src/qt/bitcoingui.cpp:None in 8f824def57 outdated
```
 885 | +            message(tr("Backup Failed"), tr("There was an error trying to save the wallet data to the new location."),
 886 | +                      true, CClientUIInterface::ICON_ERROR);
 887 |          }
 888 | +        else
 889 | +            message(tr("Backup Successful"), tr("The wallet data was successfully saved to the new location."),
 890 | +                      false, CClientUIInterface::ICON_INFORMATION);
```
laanwj commented at 10:02 AM on December 3, 2012:

I wonder why we still have a mysterious boolean parameter for the 'modal' state, even though that's one of the CClientUIInterface flags? :-)

Diapolo commented at 10:06 AM on December 3, 2012:

I think the reason is in bitcoin.cpp ThreadSafeMessageBox(), where we first use the modal flag. I'm sure it could be removed, when I have time I'll take a look. Do you think this should block merging this pull then?

laanwj commented at 10:17 AM on December 3, 2012:

Possibly. I think the current semantics is pretty vague. Some of the flags are usable, but not modal, because it's separated out to a different parameter. A pull request to change this would affect exactly the same areas as this one. So let's try to do it right in one go.

Diapolo commented at 12:30 PM on December 3, 2012:

See #2065
BitcoinPullTester commented at 1:43 PM on December 3, 2012: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/50ecd7b68970062bfb540798370dbfab6d376086 for binaries and test log.
Diapolo commented at 6:44 AM on December 4, 2012: none

@laanwj This should be sane now, it was rebased after the removal of the modal flag.

in src/qt/bitcoingui.cpp:None in 50ecd7b689

 893 | @@ -898,8 +894,12 @@ void BitcoinGUI::backupWallet()
 894 |      QString filename = QFileDialog::getSaveFileName(this, tr("Backup Wallet"), saveDir, tr("Wallet Data (*.dat)"));
 895 |      if(!filename.isEmpty()) {
 896 |          if(!walletModel->backupWallet(filename)) {
 897 | -            QMessageBox::warning(this, tr("Backup Failed"), tr("There was an error trying to save the wallet data to the new location."));
 898 | +            message(tr("Backup Failed"), tr("There was an error trying to save the wallet data to the new location."),

laanwj commented at 6:46 AM on December 4, 2012:

IMO this one should be modal

Diapolo commented at 7:00 AM on December 4, 2012:

It IS :), as MSG_ERROR = (ICON_ERROR | BTN_OK | MODAL).

laanwj commented at 7:07 AM on December 4, 2012:

Ok, cool :)

Diapolo commented at 6:31 AM on December 7, 2012: none

@laanwj Considered ready to get merged?
Diapolo commented at 8:26 PM on December 12, 2012: none

@laanwj ping
laanwj referenced this in commit 07c3f846c2 on Dec 13, 2012
laanwj merged this on Dec 13, 2012
laanwj closed this on Dec 13, 2012
laudney referenced this in commit 079348f110 on Mar 19, 2014
DrahtBot locked this on Sep 8, 2021

Contributors

Diapolo

BitcoinPullTester

laanwj

Linked (view graph)

#2065 Bitcoin-Qt: remove obsolete modal flag from GUI APIs