Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

[Qt] Console: allow empty arguments #9329

Merged
merged 3 commits into from Dec 19, 2016

Conversation

Projects
None yet
3 participants
Member

jonasschnelli commented Dec 12, 2016

Should fix #9210 and re-allow empty arguments like command '' <more arg...> or command "" <more args...>.

@jonasschnelli jonasschnelli added the GUI label Dec 12, 2016

Member

luke-jr commented Dec 12, 2016

Maybe add a unit test to check this case?

Member

luke-jr commented Dec 12, 2016

This doesn't appear to actually fix the problem at all: luke-jr/bitcoin@3710cf3

jonasschnelli and others added some commits Dec 12, 2016

Member

jonasschnelli commented Dec 13, 2016

Added @luke-jr's unit test, added some more fixes.

Member

luke-jr commented Dec 13, 2016

Here's a few more, including one that fails:

    RPCConsole::RPCExecuteCommandLine(result, "rpcNestedTest(abc,,abc)");
    QVERIFY(result == "[\"abc\",\"\",\"abc\"]");
    RPCConsole::RPCExecuteCommandLine(result, "rpcNestedTest(abc,,)");  // this one fails
    QVERIFY(result == "[\"abc\",\"\",\"\"]");
    RPCConsole::RPCExecuteCommandLine(result, "rpcNestedTest(abc )");
    QVERIFY(result == "[\"abc\"]");
    RPCConsole::RPCExecuteCommandLine(result, "rpcNestedTest( abc)");
    QVERIFY(result == "[\"abc\"]");
Member

jonasschnelli commented Dec 14, 2016

Added more rules and tests.
Now, empty arguments are no longer supported when using the comma-syntax.
Examples

  • [NOK] command <arg0>,,<arg2>
  • [OK] command <arg0>,"",<arg2>
  • [NOK] command(<arg0>,,<arg2>)
  • [NOK] command(<arg0>,)
  • [OK] command <arg0> "" <arg2>
  • [OK] command( <arg0> , <arg2> )
Owner

laanwj commented Dec 15, 2016

Now, empty arguments are no longer supported when using the comma-syntax.

Makes sense, I think.
Thanks for adding tests :)
code-review-ACK 413ce40

@laanwj laanwj self-requested a review Dec 15, 2016

@laanwj laanwj merged commit 390bd14 into bitcoin:master Dec 19, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

laanwj added a commit that referenced this pull request Dec 19, 2016

Merge #9329: [Qt] Console: allow empty arguments
390bd14 [Qt] Console: don't allow empty arguments when using the comma-syntax (Jonas Schnelli)
6a32c0f Qt/Test: Check handling of empty arguments in RPC debug console (Luke Dashjr)
89c8d2c [Qt] Console: allow empty arguments (Jonas Schnelli)

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Dec 21, 2016

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Dec 21, 2016

luke-jr added a commit to bitcoinknots/bitcoin that referenced this pull request Dec 21, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment