Rescuing uncontroversial parts of #5891
Alternative to #8746
Concept ACK
Concept ACK. Maybe add a comment that signrawtransaction can be removed from this list if it's ever split into a wallet call and a utility call.
If we're adding privkey stuff here, then signmessagewithprivkey needs to be added.
Won't those commands not show up in history? I think it should at least show that the command happened.
Concept ACK
@sipa If signrawtransaction is split, there will likely still be users trying to use the old usage for at least one release, so I'm not sure it makes sense to remove it from the filter, at least not at the same time?
@achow101 I agree it would be better to add dummy history items, but comments on previous PRs seem to suggest that is a source of disagreement.
Added signmessagewithprivkey to the filter.
@luke-jr instead of masking out the arguments as I did in my PR, what if you just added the command name to the history. Since all the commands pass through that filter method, you could have it return the string that goes into the history. For non-filtered commands, it returns the command itself. For filtered ones, it just returns the command name.
Instead of masking out the arguments as I did in my PR, what if you just added the command name to the history.
Would work for me. My main comment on doing it @achow101 's way with masking individual arguments is just that that is too brittle and hard to maintain. If having just the command name to the history is useful in any way, which I doubt a bit with the incredible autocompletion that the debug console has these days, you could do that.
I consider it useful to have any placeholder in history, since otherwise one might <up>-<enter> and get the second-to-last execution instead. At least with a dummy placeholder, they get an error/help.
walletpassphrase (should this be done in a different pull request?)getnewaddress(walletpassphrase(test)) results in executing walletpassphrase(test) but actually not "hiding" the passphrase from the history.<img width="992" alt="bildschirmfoto 2016-10-09 um 10 15 55" src="https://cloud.githubusercontent.com/assets/178464/19219059/6def7364-8e09-11e6-8f29-134993143926.png">
Rebased and taught it to handle nested commands. When a filtered command is encountered, all its parameters are replaced with "(…)" in the command history. Unit tests now check this.
72 | + << "signmessagewithprivkey" 73 | + << "signrawtransaction" 74 | + << "walletpassphrase" 75 | + << "walletpassphrasechange" 76 | + << "encryptwallet"; 77 | +
nit: newline
?
Tested a bit.
Needs coverage for importmulti now.
Shell-like, but doesn't store changed history commands until executing it.
Filters importprivkey, signrawtransaction, walletpassphrase, walletpassphrasechange, and encryptwallet
Original code was missing braces, and short-circuited before checking everything after importprivkey
Looks good. Squash/combine some commits?
Rather not squash... They look reasonably logical progression, and squashing is a bad practice anyway.
Tested ACK 8562792095a78fdef2dacc3c5e32c41206df4c87
Tested ACK 8562792095a78fdef2dacc3c5e32c41206df4c87
Wshadow statistics:
1 qt/rpcconsole.cpp:173:56: warning: declaration shadows a local variable [-Wshadow]
qt/rpcconsole.cpp:173:56: warning: declaration shadows a local variable [-Wshadow]
auto add_to_current_stack = [&](const std::string& curarg) {
^
qt/rpcconsole.cpp:167:17: note: previous declaration is here
std::string curarg;
^
1 warning generated.
@paveljanik That looks like a bug in the compiler?
Why do you think so? It is clear that curarg inside the block shadows curarg outside.
@paveljanik: Maybe write a fix PR?
@paveljanik It's a different scope, and lambda functions do not inherit the scope of the calling function beyond what you tell it to.
I thought [&] was only supposed to capture stuff actually referenced?
@luke-jr What name do you want me to use inside lambda instead of curarg? strArg?
Sure