Added 'Backup Wallet' menu option to GUI #834

sje397 commented at 12:28 PM on February 14, 2012: contributor

icon from the LGPL Nuvola set (like the tick) - http://www.icon-king.com/projects/nuvola/
include 'boost/version.hpp' in db.cpp so that the overwrite version of copy can be used
catch exceptions in BackupWallet (e.g. filesystem_error thrown when trying to overwrite without the overwrite flag set)
include db.h in walletmodel.cpp for BackupWallet function

laanwj commented at 12:38 PM on February 14, 2012: member

I'm not sure whether it's correct or not, but FYI locking isn't done around the call to BackupWallet in bitcoinrpc.cpp either.

sipa commented at 3:28 PM on February 14, 2012: member

All RPC calls already lock cs_main and cs_wallet already...

laanwj commented at 3:55 PM on February 14, 2012: member

Ok yeah I see now in ThreadRPCServer2... ok locking it is then

sipa commented at 3:59 PM on February 14, 2012: member

Actually, that is not necessary I think. BackupWallet() locks cs_db, flushes wallet.dat, and copies it at a stable time. Concurrent modifications to the wallet structure (which do not touch the database) are harmless.

sipa commented at 4:32 PM on February 14, 2012: member

One minor problem: in the select file dialog, .dat is not added automatically?

laanwj commented at 4:46 PM on February 14, 2012: member

Seems to be a common problem with Qt file save dialogs (see also #833). According to this stack overflow answer, the behavior with respect to automatically adding a suffix (when using the static functions such as getSaveFilename) is dependent on the platform:

http://stackoverflow.com/questions/2609119/qt-qfiledialog-native-dialogs-only-with-static-functions

You could use QFileDialog::setDefaultSuffix, but that means you'd have to create a QFileDialog instance yourself instead of the simple static call:

http://doc.trolltech.com/4.7/qfiledialog.html#defaultSuffix-prop

laanwj commented at 5:24 PM on February 14, 2012: member

I think the cleanest would be to define our own static getSaveFileName (for example, in guiutil.cpp) that handles the default suffix properly, and use it everywhere we use QFileDialog::getSaveFileName now (saving .csv in export, .png in qrcode and .dat in wallet backup).

TheBlueMatt commented at 8:50 PM on February 14, 2012: member

Can you update doc/assets-attribution.txt and contrib/debian/copyright with the new copyright info for new icons?

TheBlueMatt commented at 8:52 PM on February 14, 2012: member

Other than that, code looks good (though I havent tested or built anything)

sje397 commented at 12:04 PM on February 15, 2012: contributor

I updated the copyright info and resquashed.

Added 'Backup Wallet' menu option

- icon from the LGPL Nuvola set (like the tick) - http://www.icon-king.com/projects/nuvola/
- include 'boost/version.hpp' in db.cpp so that the overwrite version of copy can be used
- catch exceptions in BackupWallet (e.g. filesystem_error thrown when trying to overwrite without the overwrite flag set)
- include db.h in walletmodel.cpp for BackupWallet function
- updated doc/assets-attribution.txt and contrib/debian/copyright with copyright info for new icon

4efbda3f25

laanwj commented at 1:25 PM on February 15, 2012: member

@sipa: I'm working on the default suffix issue, and will issue a pull request when finished. Please don't let it block this one from accepting it into 0.6.0.

sje397 commented at 1:31 PM on February 15, 2012: contributor

@laanwj: fyi, the extension works fine on my kubuntu (built with qt 4.7.4)

laanwj commented at 1:37 PM on February 15, 2012: member

Yes the problem is that getSaveFileName gets a native file dialog, which does the right thing in Windows (and KDE, seemingly) but not in GNOME. So I'm working around this by making a wrapper that adds the default suffix when the native file dialog fails to do so... This is used in multiple places in the program so shouldn't hold up this pull.

sipa commented at 2:44 PM on February 15, 2012: member

ACK

TheBlueMatt commented at 6:56 PM on February 16, 2012: member

Record keeping: this fixes #370

gavinandresen commented at 6:24 PM on February 17, 2012: contributor

ACK

laanwj referenced this in commit ec2ed58232 on Feb 17, 2012

laanwj merged this on Feb 17, 2012

laanwj closed this on Feb 17, 2012

coblee referenced this in commit f5bb81df33 on Jul 17, 2012

DrahtBot locked this on Sep 8, 2021