Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace Readline, bitshares-core issue 673 #14

Merged
merged 4 commits into from
Feb 28, 2018

Conversation

jmjatlanta
Copy link

This is for the issue bitshares/bitshares-core#673

This is the Pull Request for the bitshares-fc portion. The submodule https://github.com/troglobit/editline was added to this library.

The related pull request that handles the bitshares-core portion is at bitshares/bitshares-core#685

list_index++;

if (strncmp (name, text, len) == 0)
return (dupstr(name));

Choose a reason for hiding this comment

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

With this, the dupstr() function will no longer be used an should be removed as well.

Copy link
Author

Choose a reason for hiding this comment

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

Removed

src/rpc/cli.cpp Outdated
const char* method_name;

auto& cmd = cli_commands();
for (auto it = cmd.begin(); it != cmd.end(); it++) {
Copy link

@pmconrad pmconrad Feb 20, 2018

Choose a reason for hiding this comment

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

Using a foreach loop over cmd would make the code look better. Same in next function.

Copy link
Author

Choose a reason for hiding this comment

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

Switched to foreach in both places

src/rpc/cli.cpp Outdated

auto& cmd = cli_commands();
for (auto it = cmd.begin(); it != cmd.end(); it++) {
int partlen = strlen (token); /* Part of token */

Choose a reason for hiding this comment

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

Move this out of the loop. token is never modified, no need to re-count in every cycle.

Copy link
Author

Choose a reason for hiding this comment

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

Moved partlen initialization to outside the loop

src/rpc/cli.cpp Outdated
@@ -201,7 +209,7 @@ void cli::getline( const fc::string& prompt, fc::string& line)
line_read = readline(prompt.c_str());
if( line_read == nullptr )
FC_THROW_EXCEPTION( fc::eof_exception, "" );
rl_bind_key( '\t', rl_complete );
//el_bind_key( '\t', rl_complete );

Choose a reason for hiding this comment

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

Why is this commented out? Is \t the default?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, \t is the default. Removed the line.

Copy link
Member

Choose a reason for hiding this comment

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

Please push your new commit :)

Copy link
Author

Choose a reason for hiding this comment

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

Commit lost somewhere. Redone.

.gitmodules Outdated
@@ -7,3 +7,6 @@
[submodule "vendor/websocketpp"]
path = vendor/websocketpp
url = https://github.com/zaphoyd/websocketpp.git
[submodule "vendor/editline"]
path = vendor/editline
Copy link
Member

Choose a reason for hiding this comment

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

Please replace the tabs with spaces.

Copy link
Author

Choose a reason for hiding this comment

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

Replaced tab with 4 spaces

src/rpc/cli.cpp Outdated
@@ -201,7 +209,7 @@ void cli::getline( const fc::string& prompt, fc::string& line)
line_read = readline(prompt.c_str());
if( line_read == nullptr )
FC_THROW_EXCEPTION( fc::eof_exception, "" );
rl_bind_key( '\t', rl_complete );
//el_bind_key( '\t', rl_complete );
Copy link
Member

Choose a reason for hiding this comment

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

Please push your new commit :)

@pmconrad pmconrad mentioned this pull request Feb 25, 2018
@jmjatlanta
Copy link
Author

jmjatlanta commented Feb 26, 2018

I lost a commit somehow. Changes have been redone.

Note: The new editline library has a liberal license, but still requires documentation changes. See the license at https://github.com/troglobit/editline/blob/master/LICENSE

@pmconrad
Copy link

Thanks!

@oxarbitrage oxarbitrage self-requested a review February 28, 2018 15:38
@abitmore abitmore merged commit 48dbc09 into bitshares:master Feb 28, 2018
@jmjatlanta jmjatlanta deleted the Issue_673 branch March 3, 2018 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants