Add autocomplete to bitcoin-qt's console window. #7613

GamerSg commented at 4:46 AM on February 27, 2016: contributor

Would have preferred if the command arrays were in a vector, would have prevented hardcoding of array sizes in for loop. I will probably do that in another pull request.

autocomplete

luke-jr commented at 4:48 AM on February 27, 2016: member

Concept ACK.

jonasschnelli commented at 8:12 AM on February 27, 2016: contributor

Nice! Concept ACK. Will review/test soon.

paveljanik commented at 10:02 AM on February 27, 2016: contributor

Concept ACK. Nice!

The build without wallet failed - see https://travis-ci.org/bitcoin/bitcoin/jobs/112189472#L1844

qt/libbitcoinqt.a(qt_libbitcoinqt_a-rpcconsole.o): In function `RPCConsole::RPCConsole(PlatformStyle const*, QWidget*)':
rpcconsole.cpp:(.text+0x566a): undefined reference to `vWalletRPCCommands'

in src/qt/rpcconsole.cpp:None in 5de7f8a322 outdated

 276 | @@ -274,6 +277,19 @@ RPCConsole::RPCConsole(const PlatformStyle *platformStyle, QWidget *parent) :
 277 |      connect(ui->fontSmallerButton, SIGNAL(clicked()), this, SLOT(fontSmaller()));
 278 |      connect(ui->btnClearTrafficGraph, SIGNAL(clicked()), ui->trafficGraph, SLOT(clear()));
 279 |  
 280 | +    //Setup autocomplete and attach it
 281 | +    QStringList wordList;
 282 | +    for (int i =0; i < 53; ++i)

MarcoFalke commented at 5:00 PM on February 27, 2016:

Any reason why we are not using std::array?

jonasschnelli commented at 10:30 AM on February 28, 2016:

You need to build the 53 dynamic by something like ARRAYLEN(vRPCCommands).

GamerSg commented at 1:59 PM on February 28, 2016:

@MarcoFalke That would require C++11 which i believe at this point is not supported yet in the codebase. @jonasschnelli Apparently this is much trickier than it would seem to be. Straightforward way would be to use vector but would be kind of redundant for a const array. Will see what i can do about it.

MarcoFalke commented at 2:45 PM on February 28, 2016:

@GamerSg Right, somehow forgot we are still waiting on travis to get c++11 rolled out.

You could take a look at src/utilstrencodings.h which has a #define ARRAYLEN(array).

in src/qt/rpcconsole.cpp:None in 5de7f8a322 outdated

 283 | +    {
 284 | +        wordList<<vRPCCommands[i].name.c_str();
 285 | +    }
 286 | +    for (int i =0; i < 43; ++i)
 287 | +    {
 288 | +        wordList<<vWalletRPCCommands[i].name.c_str();

MarcoFalke commented at 5:05 PM on February 27, 2016:

You'd have to guard this. Try https://github.com/bitcoin/bitcoin/blob/317462123f8e41fd7dd967ab907e59ddffb19898/src/init.cpp#L951

kirkalx commented at 9:42 AM on February 28, 2016: contributor

Concept ACK, nice idea. Perhaps this could be implemented without making vRPCCommands etc. extern? Nit: some of the spacing is a little unusual.

in src/qt/rpcconsole.cpp:None in 6f6d5f059e outdated

 287 | +    }
 288 | +    #ifdef ENABLE_WALLET
 289 | +    bool fDisableWallet = GetBoolArg("-disablewallet", false);
 290 | +    if(!fDisableWallet)
 291 | +    {//Only add commands if wallet is not disabled
 292 | +        for (int i =0; i < 43; ++i)

jonasschnelli commented at 10:29 AM on February 28, 2016:

You need to build the 43 dynamic by something like ARRAYLEN(vWalletRPCCommands).

MarcoFalke commented at 11:26 AM on March 3, 2016:

@GamerSg Are you still working on this? I'd like to see this merged :)

jonasschnelli commented at 11:46 AM on March 3, 2016:

Yes. I also think this is a nice improvement. We just need to remove the magic numbers. Just tell me or Marco if on of us should continue this (without losing you commit/author name).

laanwj commented at 1:25 PM on March 3, 2016:

ARRAYLEN will not work for an external array, the information is not available at compile time. Please see my suggestion below to add a call to TableRPC.

GamerSg commented at 4:06 AM on March 4, 2016:

@MarcoFalke Sorry, been busy past few days. I'll iron out any remaining issues over the weekend.

As @laanwj said, sizeof(array) does not work either, because the data is in a seperate cpp file and unavailable at compile time to other compilation units.

in src/rpc/server.cpp:None in 6f6d5f059e outdated

 251 | @@ -252,7 +252,7 @@ UniValue stop(const UniValue& params, bool fHelp)
 252 |  /**
 253 |   * Call Table
 254 |   */
 255 | -static const CRPCCommand vRPCCommands[] =
 256 | +extern const CRPCCommand vRPCCommands[] =

jonasschnelli commented at 10:31 AM on February 28, 2016:

extern should not be required here?

GamerSg commented at 1:04 PM on February 28, 2016:

I believe its a style thing, it is optional but i like to mark an extern even at the definition to make it clearer to someone reading the code that it is global.

Edit: Actually it does not compile without the extern keyword, not really sure why since i almost never use extern and from my research it appears that there are some differences in C and C++.

laanwj commented at 1:24 PM on March 3, 2016:

I don't like exposing this array using extern, it is an implementation detail. It also doesn't interoperate with commands that are added later on by modules (in this case the wallet in walletRegisterRPCCommands). Now you need to pull from everywhere...

My suggestion would be to add a ListCommands to TableRPC, that returns a std::vector<std::string> of commands, and use that.

jonasschnelli added the label GUI on Feb 28, 2016

in src/qt/rpcconsole.cpp:None in 6f6d5f059e outdated

 286 | +        wordList<<vRPCCommands[i].name.c_str();
 287 | +    }
 288 | +    #ifdef ENABLE_WALLET
 289 | +    bool fDisableWallet = GetBoolArg("-disablewallet", false);
 290 | +    if(!fDisableWallet)
 291 | +    {//Only add commands if wallet is not disabled

MarcoFalke commented at 7:16 PM on February 28, 2016:

No need for this comment. Also you don't have to parse the arg. Maybe try just if (pwalletMain)?

laanwj commented at 1:21 PM on March 3, 2016: member

Concept ACK

GamerSg force-pushed on Mar 5, 2016

laanwj commented at 5:59 PM on March 7, 2016: member

Looks good to me now! utACK after squash

kirkalx commented at 4:20 AM on March 8, 2016: contributor

Sweet, utACK after style nits Refer doc/developer-notes.md for some coding style tips. Also a space after a comma, and before and after operators like = and << are always good things IMO :)

    for (size_t i =0; i < commandList.size(); ++i)
    {
        wordList<<commandList[i].c_str();
    }

    autoCompleter = new QCompleter(wordList,this);

jonasschnelli commented at 9:26 AM on March 8, 2016: contributor

Currently, the wallet RPC commands are missing. You need to build up your autocomplete list in RPCConsole::setClientModel() (and only if the modal exists) instead of in RPCConsole:: RPCConsole(). This is required because of the more modular creation of the tableRPC (see #7307).

laanwj commented at 8:02 AM on March 11, 2016: member

@jonasschnelli good catch!

kirkalx commented at 8:05 AM on March 11, 2016: contributor

If you are still struggling with git for squashing, @GamerSg, Wladimir helped me with the same in #7458

GamerSg commented at 1:55 PM on March 11, 2016: contributor

Hi all, i just find time during the weekends to work on this, so ill fix up the remaining issues tomorrow.

Add autocomplete to bitcoin-qt's console window.

Removed externs
Added listCommands() to CRPCTable

Move autocomplete init to RPCConsole::setClientModel()

ce7413fcb7

GamerSg force-pushed on Mar 12, 2016

GamerSg commented at 5:20 AM on March 12, 2016: contributor

Moved the initialisation to setClientModel() as @jonasschnelli suggested. Squashed the commits as well.

MarcoFalke commented at 3:03 PM on March 12, 2016: member

Looks like this works just fine, now.

ACK ce7413f:

screenshot from 2016-03-12 15-55-14

paveljanik commented at 4:59 PM on March 12, 2016: contributor

Works OK. Please fix the indentation of "}" in rpcconsole.cpp.

jonasschnelli commented at 8:18 AM on March 14, 2016: contributor

Tested ACK ce7413fcb7d28bd72e5ade7dc9756504de766fc2

jonasschnelli merged this on Mar 14, 2016

jonasschnelli closed this on Mar 14, 2016

jonasschnelli referenced this in commit 3798e5de33 on Mar 14, 2016

paveljanik commented at 5:42 PM on March 30, 2016: contributor

@gamersg When you activate (Enter) one of the selected items from autocompleter, it is executed, but the entry line is not emptied... Fixed in #7772.

svost referenced this in commit 0f9f9e57ba on Feb 6, 2017

svost referenced this in commit b92e09336d on Feb 6, 2017

cddjr referenced this in commit 0b04b153e0 on Apr 5, 2017

cddjr referenced this in commit 5fb3aee384 on Apr 6, 2017

jonasschnelli referenced this in commit 816fab9cca on Oct 17, 2018

zkbot referenced this in commit 77c2a5f810 on Dec 4, 2020

PastaPastaPasta referenced this in commit a2854053ae on Jun 27, 2021

PastaPastaPasta referenced this in commit 6538104447 on Jun 28, 2021

PastaPastaPasta referenced this in commit b8ecf6b466 on Jun 29, 2021

PastaPastaPasta referenced this in commit e584c6f24c on Jul 1, 2021

PastaPastaPasta referenced this in commit e202bda4e6 on Jul 1, 2021

PastaPastaPasta referenced this in commit e14cda2d6a on Jul 1, 2021

MarcoFalke locked this on Sep 8, 2021