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
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -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

url = https://github.com/troglobit/editline.git
25 changes: 13 additions & 12 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -251,24 +251,25 @@ add_subdirectory( vendor/websocketpp EXCLUDE_FROM_ALL )
setup_library( fc SOURCES ${sources} LIBRARY_TYPE STATIC )
install( DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/include/" DESTINATION include )

# begin readline stuff
find_package(Curses)
find_package(Readline)
# begin editline stuff
#find_package(Curses)
set(EDITLINE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/vendor/editline" )
find_package(Editline)

file(GLOB HEADERS "include/bts/cli/*.hpp")

if (READLINE_FOUND)
target_compile_definitions (fc PRIVATE HAVE_READLINE)
set(readline_libraries ${Readline_LIBRARY})
if (CURSES_FOUND)
list(APPEND readline_libraries ${CURSES_LIBRARY})
endif()
set(readline_includes ${Readline_INCLUDE_DIR})
if (EDITLINE_FOUND)
target_compile_definitions (fc PRIVATE HAVE_EDITLINE)
set(editline_libraries ${Editline_LIBRARY})
#if (CURSES_FOUND)
# list(APPEND editline_libraries ${CURSES_LIBRARY})
#endif()
set(editline_includes ${Editline_INCLUDE_DIR})
endif()
if(WIN32)
target_compile_definitions( fc PRIVATE _CRT_NONSTDC_NO_DEPRECATE )
endif(WIN32)
# end readline stuff
# end editline stuff

if( NOT CPP_STANDARD )
set( CPP_STANDARD, "-std=c++11" )
Expand Down Expand Up @@ -362,7 +363,7 @@ target_include_directories(fc
${OPENSSL_INCLUDE_DIR}
"vendor/diff-match-patch-cpp-stl"
${CMAKE_CURRENT_SOURCE_DIR}/vendor/websocketpp
"${readline_includes}"
"${editline_includes}"

PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}
${CMAKE_CURRENT_SOURCE_DIR}/vendor/boost_1.51/include
Expand Down
50 changes: 50 additions & 0 deletions CMakeModules/FindEditline.cmake
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# - Try to find editline include dirs and libraries
#
# Usage of this module as follows:
#
# find_package(Editline)
#
# Variables used by this module, they can change the default behaviour and need
# to be set before calling find_package:
#
# Editline_ROOT_DIR Set this variable to the root installation of
# editline if the module has problems finding the
# proper installation path.
#
# Variables defined by this module:
#
# EDITLINE_FOUND System has editline, include and lib dirs found
# Editline_INCLUDE_DIR The editline include directories.
# Editline_LIBRARY The editline library.

find_path(Editline_ROOT_DIR
NAMES include/editline.h
HINTS ${EDITLINE_DIR}
)

find_path(Editline_INCLUDE_DIR
NAMES editline.h
HINTS ${Editline_ROOT_DIR}/include
)

find_library(Editline_LIBRARY
NAMES libeditline.a
HINTS ${Editline_ROOT_DIR}/src/.libs
)

if(Editline_INCLUDE_DIR AND Editline_LIBRARY)
set(EDITLINE_FOUND TRUE)
else(Editline_INCLUDE_DIR AND Editline_LIBRARY)
FIND_LIBRARY(Editline_LIBRARY NAMES editline)
include(FindPackageHandleStandardArgs)
FIND_PACKAGE_HANDLE_STANDARD_ARGS(Editline DEFAULT_MSG Editline_INCLUDE_DIR Editline_LIBRARY )
MARK_AS_ADVANCED(Editline_INCLUDE_DIR Editline_LIBRARY)
endif(Editline_INCLUDE_DIR AND Editline_LIBRARY)

mark_as_advanced(
Editline_ROOT_DIR
Editline_INCLUDE_DIR
Editline_LIBRARY
)

MESSAGE( STATUS "Found Editline: ${Editline_LIBRARY}" )
49 changes: 0 additions & 49 deletions CMakeModules/FindReadline.cmake

This file was deleted.

118 changes: 63 additions & 55 deletions src/rpc/cli.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,8 @@
#include <unistd.h>
#endif

#ifdef HAVE_READLINE
# include <readline/readline.h>
# include <readline/history.h>
// I don't know exactly what version of readline we need. I know the 4.2 version that ships on some macs is
// missing some functions we require. We're developing against 6.3, but probably anything in the 6.x
// series is fine
# if RL_VERSION_MAJOR < 6
# ifdef _MSC_VER
# pragma message("You have an old version of readline installed that might not support some of the features we need")
# pragma message("Readline support will not be compiled in")
# else
# warning "You have an old version of readline installed that might not support some of the features we need"
# warning "Readline support will not be compiled in"
# endif
# undef HAVE_READLINE
# endif
#ifdef HAVE_EDITLINE
# include "editline.h"
# ifdef WIN32
# include <io.h>
# endif
Expand Down Expand Up @@ -134,53 +120,73 @@ char * dupstr (const char* s) {
return (r);
}

char* my_generator(const char* text, int state)
/****
* @brief loop through list of commands, attempting to find a match
* @param token what the user typed
* @param match
* @returns the full name of the command or NULL if no matches found
*/
static char *my_rl_complete(char *token, int *match)
{
static int list_index, len;
const char *name;

if (!state) {
list_index = 0;
len = strlen (text);
}

auto& cmd = cli_commands();

while( list_index < cmd.size() )
{
name = cmd[list_index].c_str();
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

}

/* If no names matched, then return NULL. */
return ((char *)NULL);
int matchlen = 0;
int count = 0;
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

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


if (!strncmp ((*it).c_str(), token, partlen)) {
method_name = (*it).c_str();
matchlen = partlen;
count ++;
}
}

if (count == 1) {
*match = 1;
return strdup (method_name + matchlen);
}

return NULL;
}


static char** cli_completion( const char * text , int start, int end)
/***
* @brief return an array of matching commands
* @param token the incoming text
* @param av the resultant array of possible matches
* @returns the number of matches
*/
static int cli_completion(char *token, char ***av)
{
char **matches;
matches = (char **)NULL;

#ifdef HAVE_READLINE
if (start == 0)
matches = rl_completion_matches ((char*)text, &my_generator);
else
rl_bind_key('\t',rl_abort);
#endif

return (matches);
int num, total = 0;
char **copy;

auto& cmd = cli_commands();
num = cmd.size();

copy = (char **) malloc (num * sizeof(char *));
for (auto it = cmd.begin(); it != cmd.end(); ++it) {
if (!strncmp ((*it).c_str(), token, strlen (token))) {
copy[total] = strdup ( (*it).c_str() );
total ++;
}
}
*av = copy;

return total;
}


/***
* @brief Read input from the user
* @param prompt the prompt to display
* @param line what the user typed
*/
void cli::getline( const fc::string& prompt, fc::string& line)
{
// getting file descriptor for C++ streams is near impossible
// so we just assume it's the same as the C stream...
#ifdef HAVE_READLINE
#ifdef HAVE_EDITLINE
#ifndef WIN32
if( isatty( fileno( stdin ) ) )
#else
Expand All @@ -192,7 +198,9 @@ void cli::getline( const fc::string& prompt, fc::string& line)
if( _isatty( _fileno( stdin ) ) )
#endif
{
rl_attempted_completion_function = cli_completion;
//rl_attempted_completion_function = cli_completion;
rl_set_complete_func(my_rl_complete);
rl_set_list_possib_func(cli_completion);

static fc::thread getline_thread("getline");
getline_thread.async( [&](){
Expand All @@ -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.

if( *line_read )
add_history(line_read);
line = line_read;
Expand Down
1 change: 1 addition & 0 deletions vendor/editline
Submodule editline added at 5be965