Skip to content

Conversation

@promag
Copy link
Contributor

@promag promag commented Sep 11, 2020

Add support to use @best or @height instead of block hash in the
RPC console. The approach used is to simply replace those expressions
with the relevant RPC method.

@promag promag requested a review from hebasto September 11, 2020 08:58
@promag promag force-pushed the 2020-09-block-alias-console branch from 42464cc to 7e04b09 Compare September 11, 2020 09:00
@promag promag added the Feature label Sep 11, 2020
@promag promag closed this Sep 11, 2020
@promag promag reopened this Sep 11, 2020
Add support to use @best or @height instead of block hash in the
RPC console. The approach used is to simply replace those expressions
with the relevant RPC method.
@promag promag force-pushed the 2020-09-block-alias-console branch from 7e04b09 to d2379e5 Compare September 11, 2020 13:19
@promag
Copy link
Contributor Author

promag commented Sep 11, 2020

Now using QRegExp, not sure why it fails to build with QRegularExpression since it's (supposedly) available since Qt 5.0.

@hebasto
Copy link
Member

hebasto commented Sep 11, 2020

Now using QRegExp, not sure why it fails to build with QRegularExpression since it's (supposedly) available since Qt 5.0.

$(package)_config_opts += -no-feature-regularexpression

@promag
Copy link
Contributor Author

promag commented Sep 11, 2020

This is an alternative approach for the GUI change in bitcoin/bitcoin#16439

try
{
command.replace(QRegExp("@best"), "getbestblockhash()");
command.replace(QRegExp("@([\\d]+)"), "getblockhash(\\1)");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to have "\b@best\b" (and likewise for digits) so you don't replace "[email protected]" in a label or similar?

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach ACK d2379e5.

Only help-console message provides a user with info about new aliases. The help messages of the affected RPC's still lack these updates, but I think it is OK for the GUI console.

This PR seems preferable than the 2f9f2d66cbada9134238f25aa12691876bdeff6b commit from bitcoin/bitcoin#16439.

" example: getblock(getblockhash(0),1)[tx][0]\n\n")));
" example: getblock(getblockhash(0),1)[tx][0]\n\n"

"Aliases for block hash can be used."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"Aliases for block hash can be used."
"Aliases for block hash can be used.\n"

Comment on lines +391 to +392
command.replace(QRegExp("@best"), "getbestblockhash()");
command.replace(QRegExp("@([\\d]+)"), "getblockhash(\\1)");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The opening word boundary doesn't work with a "word" that starts with the @ character. But the closing word boundary indeed improves behavior and error handling:

Suggested change
command.replace(QRegExp("@best"), "getbestblockhash()");
command.replace(QRegExp("@([\\d]+)"), "getblockhash(\\1)");
command.replace(QRegExp("@best\\b"), "getbestblockhash()");
command.replace(QRegExp("@([\\d]+)\\b"), "getblockhash(\\1)");

@maflcko maflcko changed the title gui: Support block hash alias in console Support block hash alias in console Sep 20, 2020
#include <QKeyEvent>
#include <QMenu>
#include <QMessageBox>
#include <QRegExp>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this warrants adding QRegExp to the project

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have it. Maybe you are thinking of QRegularExpression?

Comment on lines +391 to +392
command.replace(QRegExp("@best"), "getbestblockhash()");
command.replace(QRegExp("@([\\d]+)"), "getblockhash(\\1)");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think RPCParseCommandLine would be the proper place to do this.

@hebasto
Copy link
Member

hebasto commented Mar 24, 2021

@promag Any update on this? :)

@hebasto hebasto added UX All about "how to get things done" Waiting for author labels May 29, 2021
@promag
Copy link
Contributor Author

promag commented Jun 6, 2021

@hebasto I'm not convinced this is a good change. Closing for now and leaving it up for grabs.

@promag promag closed this Jun 6, 2021
@promag promag deleted the 2020-09-block-alias-console branch June 6, 2021 13:37
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Feature UX All about "how to get things done" Waiting for author

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants