Save/restore RPCConsole geometry only for window

[hebasto]

After using the GUI with -disablewallet the "Node window" inherits the geometry of the main window, that could be unexpected for users.

This PR provides independent geometry settings for RPCConsole in both modes:

• window sizes and QSplitter sizes when -disablewallet=0
• only QSplitter sizes when -disablewallet=1

[maflcko]

qt/rpcconsole.cpp:(.text+0x77ed): undefined reference to `WalletModel::isWalletEnabled()'

[hebasto]

Fixed.

[luke-jr]

Concept ACK, though the code seems uglier than I would like (ignore this nit if we can't think of a better way)

Perhaps add a version number or comment to the splitter settings while you're at it, since adding/changing columns likely shouldn't use old sizes... Or at least a comment reminding us to do so later when/if we change the columns again.

[promag]

Code review ACK b5523f1.

I think this #ifdef madness could go away by moving WalletModel::isWalletEnabled() to another place where it always compiled in.

[luke-jr]

I think this #ifdef madness could go away by moving WalletModel::isWalletEnabled() to another place where it always compiled in.

But then the code here will always be compiled in... the whole point of not building wallet support is to omit the code for it :p

[promag]

@luke-jr I understand that but my proposal doesn't mean wallet support is compiled in, only downside is a runtime check:

bool IsWalletEnabled()
{
#ifdef ENABLE_WALLET
    return !gArgs.GetBoolArg("-disablewallet", DEFAULT_DISABLE_WALLET);
#else
    return false;
#endif
}

[luke-jr]

But with this, everything conditional on IsWalletEnabled() (in this case, the settings code) is being compiled in to a non-wallet build.

(Unless perhaps IsWalletEnabled is carefully made always-inlined so the compiler knows to remove the branch?)

[hebasto]

Rebased b5523f1 -> 01d9586 (pr194.02 -> pr194.03) due to the conflict with #180.

[jarolrod]

Can you show a screenshot of the behavior this PR is supposed to exhibit and what it is supposed to fix? Comparing with master, I don't see what this is supposed to do.

Testing on macOS 11.1 with Qt 5.15.2

Master

Wallet Enabled
Run bitcoin-qt with wallet enabled. The main window is resized to be wider.

Main Window	Node Window

Wallet Disabled
run bitcoin-qt with disablewallet=1. The Node Window has inherited the geometry of the Main Window when bitcoin-qt was run with wallet enabled.

Node Window

Now, I'll resize the Node Window to be smaller.

Node Window

Restart With Wallet Re-Enabled
The Main Window inherits the geometry of the Node Window when I resized the Node Window while bitcoin-qt was run with the -disablewallet=1.

Main Window

PR

Wallet Enabled
Run bitcoin-qt with wallet enabled. The main window is resized to be wider.

Main Window	Node Window

Wallet Disabled
run bitcoin-qt with disablewallet=1. The Node Window has inherited the geometry of the Main Window when bitcoin-qt was run with wallet enabled. This is the same behavior as master.

Node Window

Now, I'll resize the Node Window to be smaller.

Node Window

Restart With Wallet Re-Enabled
The Main Window inherits the geometry of the Node Window when I resized the Node Window while bitcoin-qt was run with the -disablewallet=1. This is the same behavior as master.

Main Window

[hebasto]

@jarolrod

Comparing with master, I don't see what this is supposed to do.

Initially, I tested this pr in the Cinnamon DE only.
Here are the steps to reproduce the issue on macOS BigSur 11.2.1 (20D74) on the master branch:

1. run bitcoin-qt -disablewallet=0, go to the menu -> Window -> Information, note the size/location of the "Node window", quit
2. run bitcoin-qt -disablewallet=1, resize/move the window and note its size/location, quit
3. run bitcoin-qt -disablewallet=0, go to the menu -> Window -> Information, note the size/location of the "Node window" is inherited from the the step 2

[Talkless]

I think this #ifdef madness could go away by moving WalletModel::isWalletEnabled() to another place where it always compiled in.

isWalletEnabled could be constexpr function (that implies inline) that returns false if ENABLE_WALLET=0, and out-of-line function checking program arguments at runtime otherwise.

[Talkless]

tACK 01d9586, tested on Debian Sid with Qt 5.15.2. I've managed to reproduce issue using #194 (comment) instructions, and I see that this PR does detach main window and information window sizes. Built with --enable-wallet and --disable-wallet.

[jarolrod]

ACK 01d9586, tested on macOS 11.2 Qt 5.15.2

Reproduced the steps thanks to @hebasto's instructions. Confirming the behavior described on master and this PR fixes that

[promag]

Code review ACK 01d9586.

This is a straightforward solution to handle both cases (with and without wallet support).

A different path would be to move load/save window state out of RPCConsole and also use QSettings::beginGroup to distinguish each case.

[hebasto]

@promag

Thank you for your review!

A different path would be to move load/save window state out of RPCConsole and also use QSettings::beginGroup to distinguish each case.

While working on this PR I did consider both variants you mentioned. The current implementation was chosen due to a smaller diff.