Re: [PATCH] readline extension rl_bind_key addition and other enhancements

From: Date: Wed, 14 Mar 2012 20:28:49 +0000
Subject: Re: [PATCH] readline extension rl_bind_key addition and other enhancements
Groups: php.internals 
Request: Send a blank email to [email protected] to get a copy of this message
Hi,

A few comments on the patch:

A) for licensing reasons we should try to keep as few readline only things as possible in there (gpl
vs. php license)
B) thread safty isn't an issue for readline but you still should do the init and deinit in
rinit/rshutdown not minit/mshutdown, probably also do only bind_key_functions =NULL in rinit and
init the hashtable on demand later.
C) don't compare the r result of  zend_hash_find and others against -1 or such but use
SUCCESS/FAILURE
D) i don't really like new features in 5.3 anymore at this stage
E) please log a bug and attach the patch so we can track this

Johannes

P.s. sorry for top posting ...

Osama Abu Elsorour <[email protected]> wrote:

>I was recently involved in a project that relied heavily on readline to provide console text
>input capabilities. However I soon noticed that the current readline is lacking some important
>functionality.
>
>This patch applies only to PHP compiled with libreadline (and don't have an effect if
>compiled with libeditline). It adds/modified the following:
>- Adds support for rl_bind_key function by exposing:
>function readline_bind_key_function($key, $callback_func)
>where:
>$key: Key code to bind to.
>$callback_func: A callback function in the form:
>function callback($key, $count) where $key and $count are the same parameters passed from
>rl_bind_key()
>Setting $callback_func to null will cause the binding to be removed for $key.
>- Modifies the behavior of readline_info() to allow for setting the readline properties
>"point" and "end" (it used to only read them but not set them).
>
>Patch below:
>
>--- php-5.3.10/ext/readline/readline.c	2012-01-01 15:15:04.000000000 +0200
>+++ php-5.3.10-patched/ext/readline/readline.c	2012-03-13 09:35:43.000000000 +0200
>@@ -57,9 +57,12 @@ PHP_FUNCTION(readline_callback_read_char
> PHP_FUNCTION(readline_callback_handler_remove);
> PHP_FUNCTION(readline_redisplay);
> PHP_FUNCTION(readline_on_new_line);
>+PHP_FUNCTION(readline_bind_key_function);
> 
> static zval *_prepped_callback = NULL;
> 
>+static HashTable *_bind_key_functions_ht = NULL;
>+
> #endif
> 
> static zval *_readline_completion = NULL;
>@@ -121,6 +124,12 @@ ZEND_END_ARG_INFO()
> 
> ZEND_BEGIN_ARG_INFO(arginfo_readline_on_new_line, 0)
> ZEND_END_ARG_INFO()
>+
>+ZEND_BEGIN_ARG_INFO_EX(arginfo_readline_bind_key_function, 0, 0, 2)
>+	ZEND_ARG_INFO(0, key)
>+	ZEND_ARG_INFO(0, funcname)
>+ZEND_END_ARG_INFO()
>+
> #endif
> /* }}} */
> 
>@@ -142,6 +151,7 @@ static const zend_function_entry php_rea
> 	PHP_FE(readline_callback_handler_remove,	arginfo_readline_callback_handler_remove)
> 	PHP_FE(readline_redisplay, arginfo_readline_redisplay)
> 	PHP_FE(readline_on_new_line, arginfo_readline_on_new_line)
>+	PHP_FE(readline_bind_key_function, arginfo_readline_bind_key_function)
> #endif
> 	PHP_FE_END
> };
>@@ -165,6 +175,9 @@ ZEND_GET_MODULE(readline)
> 
> PHP_MINIT_FUNCTION(readline)
> {
>+	ALLOC_HASHTABLE(_bind_key_functions_ht);
>+	zend_hash_init(_bind_key_functions_ht, 255, NULL, ZVAL_PTR_DTOR, 0);
>+	
>     	using_history();
>     	return SUCCESS;
> }
>@@ -181,6 +194,9 @@ PHP_RSHUTDOWN_FUNCTION(readline)
> 		zval_ptr_dtor(&_prepped_callback);
> 		_prepped_callback = 0;
> 	}
>+	
>+	zend_hash_destroy(_bind_key_functions_ht);
>+	FREE_HASHTABLE(_bind_key_functions_ht);
> #endif
> 
> 	return SUCCESS;
>@@ -254,8 +270,16 @@ PHP_FUNCTION(readline_info)
> 			}
> 			RETVAL_STRING(SAFE_STRING(oldstr),1);
> 		} else if (!strcasecmp(what, "point")) {
>+			if (value) {
>+				convert_to_long_ex(value);
>+				rl_point = Z_LVAL_PP(value);
>+			}
> 			RETVAL_LONG(rl_point);
> 		} else if (!strcasecmp(what, "end")) {
>+			if (value) {
>+				convert_to_long_ex(value);
>+				rl_end = Z_LVAL_PP(value);
>+			}
> 			RETVAL_LONG(rl_end);
> #ifdef HAVE_LIBREADLINE
> 		} else if (!strcasecmp(what, "mark")) {
>@@ -612,6 +636,74 @@ PHP_FUNCTION(readline_on_new_line)
> }
> /* }}} */
> 
>+/* {{{ proto bool readline_bind_key_function(string funcname) 
>+ Readline rl_bind_key */
>+
>+static int _readline_bind_key_cb(int count, int key)
>+{ 
>+	zval *params[2];
>+	TSRMLS_FETCH();
>+	
>+	params[0]=_readline_long_zval(key);
>+	params[1]=_readline_long_zval(count);
>+	
>+	zval **_callback_func = NULL;
>+	long _key = key;
>+	int r = zend_hash_find(_bind_key_functions_ht, (char *) &_key, sizeof(_key), (void
>**)&_callback_func);
>+	if (r == -1)
>+		return -1;
>+	
>+	int _return_int = 0;
>+	zval *_callback_return = NULL;
>+	MAKE_STD_ZVAL(_callback_return);
>+	if (call_user_function(CG(function_table), NULL, *_callback_func, _callback_return, 2, params
>TSRMLS_CC) == SUCCESS) {
>+		_return_int = _callback_return->value.lval;
>+		zval_dtor(_callback_return);
>+		FREE_ZVAL(_callback_return);
>+	}
>+
>+	return _return_int; 
>+}
>+
>+PHP_FUNCTION(readline_bind_key_function)
>+{
>+	long key;
>+	zval *arg = NULL;
>+	char *name = NULL;
>+	
>+	if (FAILURE == zend_parse_parameters(ZEND_NUM_ARGS() TSRMLS_CC, "l!z", &key,
>&arg)) {
>+		RETURN_FALSE;
>+	}
>+	
>+	if (Z_TYPE_P(arg) == IS_NULL) {
>+		zend_hash_del(_bind_key_functions_ht, (char *)&key, sizeof(key));
>+		rl_bind_key(key, rl_insert);
>+	}
>+	else {
>+		if (!zend_is_callable(arg, 0, &name TSRMLS_CC)) {
>+			php_error_docref(NULL TSRMLS_CC, E_WARNING, "%s is not callable", name);
>+			efree(name);
>+			RETURN_FALSE;
>+		}
>+		efree(name);
>+		
>+		zval *_temp_callback = NULL;
>+		MAKE_STD_ZVAL(_temp_callback);
>+		*_temp_callback = *arg;
>+		zval_copy_ctor(_temp_callback);
>+		
>+		zend_hash_add(_bind_key_functions_ht, (char *)&key, sizeof(key), &_temp_callback,
>sizeof(zval *), NULL); 
>+		
>+		rl_bind_key(key, &_readline_bind_key_cb);
>+	}
>+	
>+	RETURN_TRUE;
>+}
>+
>+/* }}} */
>+
> #endif
>
>
>--
>PHP Internals - PHP Runtime Development Mailing List
>To unsubscribe, visit: http://www.php.net/unsub.php
>


Thread (3 messages)

« previous php.internals (#58935) next »