Re: [PATCH] PR python/16699: GDB Python command completion with overriden complete vs. completer class

[Sergio Durigan Junior <sergiodj at redhat dot com>]

On Tuesday, August 19 2014, I wrote:

> On Tuesday, July 08 2014, I wrote:
>
>> Thank you for the catches.  I will update my local version.
>
> Ping.

Ping^2.

> -- 
> Sergio
> GPG key ID: 0x65FC5E36
> Please send encrypted e-mail if possible
> http://sergiodj.net/
>
> gdb/
> 2014-08-19  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	PR python/16699
> 	* cli/cli-decode.c (set_cmd_completer_handle_brkchars): New
> 	function.
> 	(add_cmd): Set "completer_handle_brkchars" to NULL.
> 	* cli/cli-decode.h (struct cmd_list_element)
> 	<completer_handle_brkchars>: New field.
> 	* command.h (completer_ftype_void): New typedef.
> 	(set_cmd_completer_handle_brkchars): New prototype.
> 	* completer.c (set_gdb_completion_word_break_characters): New
> 	function.
> 	(complete_line_internal): Call "completer_handle_brkchars"
> 	callback from command.
> 	* completer.h: Include "command.h".
> 	(set_gdb_completion_word_break_characters): New prototype.
> 	* python/py-cmd.c (cmdpy_completer_helper): New function.
> 	(cmdpy_completer_handle_brkchars): New function.
> 	(cmdpy_completer): Adjust to use cmdpy_completer_helper.
> 	(cmdpy_init): Set completer_handle_brkchars to
> 	cmdpy_completer_handle_brkchars.
>
> gdb/testsuite/
> 2014-08-19  Sergio Durigan Junior  <sergiodj@redhat.com>
>
> 	PR python/16699
> 	* gdb.python/py-completion.exp: New file.
> 	* gdb.python/py-completion.py: Likewise.
>
> diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
> index c409d9c..7bc51a1 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -161,6 +161,15 @@ set_cmd_completer (struct cmd_list_element *cmd, completer_ftype *completer)
>    cmd->completer = completer; /* Ok.  */
>  }
>  
> +/* See definition in commands.h.  */
> +
> +void
> +set_cmd_completer_handle_brkchars (struct cmd_list_element *cmd,
> +			       completer_ftype_void *completer_handle_brkchars)
> +{
> +  cmd->completer_handle_brkchars = completer_handle_brkchars;
> +}
> +
>  /* Add element named NAME.
>     Space for NAME and DOC must be allocated by the caller.
>     CLASS is the top level category into which commands are broken down
> @@ -236,6 +245,7 @@ add_cmd (const char *name, enum command_class class, cmd_cfunc_ftype *fun,
>    c->prefix = NULL;
>    c->abbrev_flag = 0;
>    set_cmd_completer (c, make_symbol_completion_list_fn);
> +  c->completer_handle_brkchars = NULL;
>    c->destroyer = NULL;
>    c->type = not_set_cmd;
>    c->var = NULL;
> diff --git a/gdb/cli/cli-decode.h b/gdb/cli/cli-decode.h
> index 865d4a0..5920559 100644
> --- a/gdb/cli/cli-decode.h
> +++ b/gdb/cli/cli-decode.h
> @@ -176,6 +176,15 @@ struct cmd_list_element
>         "baz/foo", return "baz/foobar".  */
>      completer_ftype *completer;
>  
> +    /* Handle the word break characters for this completer.  Usually
> +       this function need not be defined, but for some types of
> +       completers (e.g., Python completers declared as methods inside
> +       a class) the word break chars may need to be redefined
> +       depending on the completer type (e.g., for filename
> +       completers).  */
> +
> +    completer_ftype_void *completer_handle_brkchars;
> +
>      /* Destruction routine for this command.  If non-NULL, this is
>         called when this command instance is destroyed.  This may be
>         used to finalize the CONTEXT field, if needed.  */
> diff --git a/gdb/command.h b/gdb/command.h
> index 4ac3bfb..3b96212 100644
> --- a/gdb/command.h
> +++ b/gdb/command.h
> @@ -159,8 +159,16 @@ extern void set_cmd_sfunc (struct cmd_list_element *cmd,
>  typedef VEC (char_ptr) *completer_ftype (struct cmd_list_element *,
>  					 const char *, const char *);
>  
> +typedef void completer_ftype_void (struct cmd_list_element *,
> +				   const char *, const char *);
> +
>  extern void set_cmd_completer (struct cmd_list_element *, completer_ftype *);
>  
> +/* Set the completer_handle_brkchars callback.  */
> +
> +extern void set_cmd_completer_handle_brkchars (struct cmd_list_element *,
> +					       completer_ftype_void *);
> +
>  /* HACK: cagney/2002-02-23: Code, mostly in tracepoints.c, grubs
>     around in cmd objects to test the value of the commands sfunc().  */
>  extern int cmd_cfunc_eq (struct cmd_list_element *cmd,
> diff --git a/gdb/completer.c b/gdb/completer.c
> index 44920dd..67d7f45 100644
> --- a/gdb/completer.c
> +++ b/gdb/completer.c
> @@ -449,6 +449,21 @@ expression_completer (struct cmd_list_element *ignore,
>    return location_completer (ignore, p, word);
>  }
>  
> +/* See definition in completer.h.  */
> +
> +void
> +set_gdb_completion_word_break_characters (completer_ftype *fn)
> +{
> +  /* So far we are only interested in differentiating filename
> +     completers from everything else.  */
> +  if (fn == filename_completer)
> +    rl_completer_word_break_characters
> +      = gdb_completer_file_name_break_characters;
> +  else
> +    rl_completer_word_break_characters
> +      = gdb_completer_command_word_break_characters;
> +}
> +
>  /* Here are some useful test cases for completion.  FIXME: These
>     should be put in the test suite.  They should be tested with both
>     M-? and TAB.
> @@ -677,6 +692,9 @@ complete_line_internal (const char *text,
>  			   p--)
>  			;
>  		    }
> +		  if (reason == handle_brkchars
> +		      && c->completer_handle_brkchars != NULL)
> +		    (*c->completer_handle_brkchars) (c, p, word);
>  		  if (reason != handle_brkchars && c->completer != NULL)
>  		    list = (*c->completer) (c, p, word);
>  		}
> @@ -750,6 +768,9 @@ complete_line_internal (const char *text,
>  		       p--)
>  		    ;
>  		}
> +	      if (reason == handle_brkchars
> +		  && c->completer_handle_brkchars != NULL)
> +		(*c->completer_handle_brkchars) (c, p, word);
>  	      if (reason != handle_brkchars && c->completer != NULL)
>  		list = (*c->completer) (c, p, word);
>  	    }
> diff --git a/gdb/completer.h b/gdb/completer.h
> index 7aa0f3b..bc7ed96 100644
> --- a/gdb/completer.h
> +++ b/gdb/completer.h
> @@ -18,6 +18,7 @@
>  #define COMPLETER_H 1
>  
>  #include "gdb_vecs.h"
> +#include "command.h"
>  
>  extern VEC (char_ptr) *complete_line (const char *text,
>  				      const char *line_buffer,
> @@ -48,6 +49,13 @@ extern char *get_gdb_completer_quote_characters (void);
>  
>  extern char *gdb_completion_word_break_characters (void);
>  
> +/* Set the word break characters array to the corresponding set of
> +   chars, based on FN.  This function is useful for cases when the
> +   completer doesn't know the type of the completion until some
> +   calculation is done (e.g., for Python functions).  */
> +
> +extern void set_gdb_completion_word_break_characters (completer_ftype *fn);
> +
>  /* Exported to linespec.c */
>  
>  extern const char *skip_quoted_chars (const char *, const char *,
> diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
> index 21f2a20..8219699 100644
> --- a/gdb/python/py-cmd.c
> +++ b/gdb/python/py-cmd.c
> @@ -208,45 +208,155 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
>    do_cleanups (cleanup);
>  }
>  
> +/* Helper function for the Python command completers (both "pure"
> +   completer and brkchar handler).  This function takes COMMAND, TEXT
> +   and WORD and tries to call the Python method for completion with
> +   these arguments.  It also takes HANDLE_BRKCHARS_P, an argument to
> +   identify whether it is being called from the brkchar handler or
> +   from the "pure" completer.  In the first case, it effectively calls
> +   the Python method for completion, and records the PyObject in a
> +   static variable (used as a "cache").  In the second case, it just
> +   returns that variable, without actually calling the Python method
> +   again.  This saves us one Python method call.
> +
> +   It is important to mention that this function is built on the
> +   assumption that the calls to cmdpy_completer_handle_brkchars and
> +   cmdpy_completer will be subsequent with nothing intervening.  This
> +   is true for our completer mechanism.
> +
> +   This function returns the PyObject representing the Python method
> +   call.  */
> +
> +static PyObject *
> +cmdpy_completer_helper (struct cmd_list_element *command,
> +			const char *text, const char *word,
> +			int handle_brkchars_p)
> +{
> +  cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
> +  PyObject *textobj, *wordobj;
> +  /* This static variable will server as a "cache" for us, in order to
> +     store the PyObject that results from calling the Python
> +     function.  */
> +  static PyObject *resultobj = NULL;
> +
> +  if (handle_brkchars_p)
> +    {
> +      /* If we were called to handle brkchars, it means this is the
> +	 first function call of two that will happen in a row.
> +	 Therefore, we need to call the completer ourselves, and cache
> +	 the return value in the static variable RESULTOBJ.  Then, in
> +	 the second call, we can just use the value of RESULTOBJ to do
> +	 our job.  */
> +      if (resultobj != NULL)
> +	Py_DECREF (resultobj);
> +
> +      resultobj = NULL;
> +      if (!obj)
> +	error (_("Invalid invocation of Python command object."));
> +      if (!PyObject_HasAttr ((PyObject *) obj, complete_cst))
> +	{
> +	  /* If there is no complete method, don't error.  */
> +	  return NULL;
> +	}
> +
> +      textobj = PyUnicode_Decode (text, strlen (text), host_charset (), NULL);
> +      if (!textobj)
> +	error (_("Could not convert argument to Python string."));
> +      wordobj = PyUnicode_Decode (word, sizeof (word), host_charset (), NULL);
> +      if (!wordobj)
> +	{
> +	  Py_DECREF (textobj);
> +	  error (_("Could not convert argument to Python string."));
> +	}
> +
> +      resultobj = PyObject_CallMethodObjArgs ((PyObject *) obj, complete_cst,
> +					      textobj, wordobj, NULL);
> +      Py_DECREF (textobj);
> +      Py_DECREF (wordobj);
> +      if (!resultobj)
> +	{
> +	  /* Just swallow errors here.  */
> +	  PyErr_Clear ();
> +	}
> +
> +      Py_XINCREF (resultobj);
> +    }
> +
> +  return resultobj;
> +}
> +
> +/* Python function called to determine the break characters of a
> +   certain completer.  We are only interested in knowing if the
> +   completer registered by the user will return one of the integer
> +   codes (see COMPLETER_* symbols).  */
> +
> +static void
> +cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
> +				 const char *text, const char *word)
> +{
> +  PyObject *resultobj = NULL;
> +  struct cleanup *cleanup;
> +
> +  cleanup = ensure_python_env (get_current_arch (), current_language);
> +
> +  /* Calling our helper to obtain the PyObject of the Python
> +     function.  */
> +  resultobj = cmdpy_completer_helper (command, text, word, 1);
> +
> +  /* Check if there was an error.  */
> +  if (resultobj == NULL)
> +    goto done;
> +
> +  if (PyInt_Check (resultobj))
> +    {
> +      /* User code may also return one of the completion constants,
> +	 thus requesting that sort of completion.  We are only
> +	 interested in this kind of return.  */
> +      long value;
> +
> +      if (!gdb_py_int_as_long (resultobj, &value))
> +	{
> +	  /* Ignore.  */
> +	  PyErr_Clear ();
> +	}
> +      else if (value >= 0 && value < (long) N_COMPLETERS)
> +	{
> +	  /* This is the core of this function.  Depending on which
> +	     completer type the Python function returns, we have to
> +	     adjust the break characters accordingly.  */
> +	  set_gdb_completion_word_break_characters
> +	    (completers[value].completer);
> +	}
> +    }
> +
> + done:
> +
> +  /* We do not call Py_XDECREF here because RESULTOBJ will be used in
> +     the subsequent call to cmdpy_completer function.  */
> +  do_cleanups (cleanup);
> +}
> +
>  /* Called by gdb for command completion.  */
>  
>  static VEC (char_ptr) *
>  cmdpy_completer (struct cmd_list_element *command,
>  		 const char *text, const char *word)
>  {
> -  cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
> -  PyObject *textobj, *wordobj, *resultobj = NULL;
> +  PyObject *resultobj = NULL;
>    VEC (char_ptr) *result = NULL;
>    struct cleanup *cleanup;
>  
>    cleanup = ensure_python_env (get_current_arch (), current_language);
>  
> -  if (! obj)
> -    error (_("Invalid invocation of Python command object."));
> -  if (! PyObject_HasAttr ((PyObject *) obj, complete_cst))
> -    {
> -      /* If there is no complete method, don't error -- instead, just
> -	 say that there are no completions.  */
> -      goto done;
> -    }
> +  /* Calling our helper to obtain the PyObject of the Python
> +     function.  */
> +  resultobj = cmdpy_completer_helper (command, text, word, 0);
>  
> -  textobj = PyUnicode_Decode (text, strlen (text), host_charset (), NULL);
> -  if (! textobj)
> -    error (_("Could not convert argument to Python string."));
> -  wordobj = PyUnicode_Decode (word, strlen (word), host_charset (), NULL);
> -  if (! wordobj)
> -    error (_("Could not convert argument to Python string."));
> -
> -  resultobj = PyObject_CallMethodObjArgs ((PyObject *) obj, complete_cst,
> -					  textobj, wordobj, NULL);
> -  Py_DECREF (textobj);
> -  Py_DECREF (wordobj);
> -  if (! resultobj)
> -    {
> -      /* Just swallow errors here.  */
> -      PyErr_Clear ();
> -      goto done;
> -    }
> +  /* If the result object of calling the Python function is NULL, it
> +     means that there was an error.  In this case, just give up and
> +     return NULL.  */
> +  if (resultobj == NULL)
> +    goto done;
>  
>    result = NULL;
>    if (PyInt_Check (resultobj))
> @@ -302,7 +412,6 @@ cmdpy_completer (struct cmd_list_element *command,
>  
>   done:
>  
> -  Py_XDECREF (resultobj);
>    do_cleanups (cleanup);
>  
>    return result;
> @@ -548,6 +657,9 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
>        set_cmd_context (cmd, self);
>        set_cmd_completer (cmd, ((completetype == -1) ? cmdpy_completer
>  			       : completers[completetype].completer));
> +      if (completetype == -1)
> +	set_cmd_completer_handle_brkchars (cmd,
> +					   cmdpy_completer_handle_brkchars);
>      }
>    if (except.reason < 0)
>      {
> diff --git a/gdb/testsuite/gdb.python/py-completion.exp b/gdb/testsuite/gdb.python/py-completion.exp
> new file mode 100644
> index 0000000..a3bac8b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-completion.exp
> @@ -0,0 +1,69 @@
> +# Copyright (C) 2014 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +set testfile "py-completion"
> +
> +load_lib gdb-python.exp
> +
> +gdb_exit
> +gdb_start
> +
> +# Skip all tests if Python scripting is not enabled.
> +if { [skip_python_tests] } { continue }
> +
> +gdb_test_no_output "source ${srcdir}/${subdir}/${testfile}.py"
> +
> +# Create a temporary directory
> +set testdir "[standard_output_file "py-completion-testdir"]/"
> +set testdir_regex [string_to_regexp $testdir]
> +set testdir_complete [standard_output_file "py-completion-test"]
> +file mkdir $testdir
> +
> +# This one should always pass.
> +send_gdb "completefileinit ${testdir_complete}\t"
> +gdb_test_multiple "" "completefileinit completion" {
> +    -re "^completefileinit ${testdir_regex}$" {
> +        pass "completefileinit completion"
> +    }
> +}
> +
> +# Just discarding whatever we typed.
> +gdb_test " " ".*" "discard #1"
> +
> +# This is the problematic one.
> +send_gdb "completefilemethod ${testdir_complete}\t"
> +gdb_test_multiple "" "completefilemethod completion" {
> +    -re "^completefilemethod ${testdir_regex} $" {
> +        fail "completefilemethod completion (completed filename as wrong command arg)"
> +    }
> +    -re "^completefilemethod ${testdir_regex}$" {
> +        pass "completefilemethod completion"
> +    }
> +}
> +
> +# Discarding again
> +gdb_test " " ".*" "discard #2"
> +
> +# Another problematic
> +set completion_regex "[string_to_regexp [standard_output_file "py-completion-t"]]"
> +send_gdb "completefilecommandcond [standard_output_file "py-completion-t\t"]"
> +gdb_test_multiple "" "completefilecommandcond completion" {
> +    -re "^completefilecommandcond ${testdir}$" {
> +	fail "completefilecommandcond completion (completed filename instead of command)"
> +    }
> +    -re "^completefilecommandcond ${completion_regex}\007$" {
> +	pass "completefilecommandcond completion"
> +    }
> +}
> diff --git a/gdb/testsuite/gdb.python/py-completion.py b/gdb/testsuite/gdb.python/py-completion.py
> new file mode 100644
> index 0000000..23592d0
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-completion.py
> @@ -0,0 +1,58 @@
> +# Copyright (C) 2014 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# This testcase tests PR python/16699
> +
> +import gdb
> +
> +class CompleteFileInit(gdb.Command):
> +	def __init__(self):
> +		gdb.Command.__init__(self,'completefileinit',gdb.COMMAND_USER,gdb.COMPLETE_FILENAME)
> +
> +	def invoke(self,argument,from_tty):
> +		raise gdb.GdbError('not implemented')
> +
> +class CompleteFileMethod(gdb.Command):
> +	def __init__(self):
> +		gdb.Command.__init__(self,'completefilemethod',gdb.COMMAND_USER)
> +
> +	def invoke(self,argument,from_tty):
> +		raise gdb.GdbError('not implemented')
> +
> +	def complete(self,text,word):
> +		return gdb.COMPLETE_FILENAME
> +
> +class CompleteFileCommandCond(gdb.Command):
> +	def __init__(self):
> +		gdb.Command.__init__(self,'completefilecommandcond',gdb.COMMAND_USER)
> +
> +	def invoke(self,argument,from_tty):
> +		raise gdb.GdbError('not implemented')
> +
> +	def complete(self,text,word):
> +		# This is a test made to know if the command
> +		# completion still works fine.  When the user asks to
> +		# complete something like "completefilecommandcond
> +		# /path/to/py-completion-t", it should not complete to
> +		# "/path/to/py-completion-test/", but instead just
> +		# wait for input.
> +		if "py-completion-t" in text:
> +			return gdb.COMPLETE_COMMAND
> +		else:
> +			return gdb.COMPLETE_FILENAME
> +
> +CompleteFileInit()
> +CompleteFileMethod()
> +CompleteFileCommandCond()

-- 
Sergio
GPG key ID: 0x65FC5E36
Please send encrypted e-mail if possible
http://sergiodj.net/