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

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

[Sergio Durigan Junior]

Hi,

This PR came from a Red Hat bug that was filed recently.  I checked and
it still exists on HEAD, so here's a proposed fix.  Although this is
marked as a Python backend bug, this is really about the completion
mechanism used by GDB.  Since this code reminds me of my first attempt
to make a good noodle, it took me quite some time to fix it in a
non-intrusive way.

The problem is triggered when one registers a completion method inside a
class in a Python script, rather than registering the command using a
completer class directly.  For example, consider the following script:

    class MyFirstCommand(gdb.Command):
          def __init__(self):
              gdb.Command.__init__(self,'myfirstcommand',gdb.COMMAND_USER,gdb.COMPLETE_FILENAME)

              def invoke(self,argument,from_tty):
                  raise gdb.GdbError('not implemented')

    class MySecondCommand(gdb.Command):
          def __init__(self):
              gdb.Command.__init__(self,'mysecondcommand',gdb.COMMAND_USER)

              def invoke(self,argument,from_tty):
                  raise gdb.GdbError('not implemented')

                  def complete(self,text,word):
                      return gdb.COMPLETE_FILENAME

    MyFirstCommand ()
    MySecondCommand ()

When one loads this into GDB and tries to complete filenames for both
myfirstcommand and mysecondcommand, she gets:

    (gdb) myfirstcommand /hom<TAB>
    (gdb) myfirstcommand /home/
                               ^
    ...
    (gdb) mysecondcommand /hom<TAB>
    (gdb) mysecondcommand /home 
                                ^

(The "^" marks the final position of the cursor after the TAB).

So we see that myfirstcommand honors the COMPLETE_FILENAME class (as
specified in the command creation), but mysecondcommand does not.  After
some investigation, I found that the problem lies with the set of word
break characters that is used for each case.  The set should be the same
for both commands, but it is not.

During the process of deciding which type of completion should be used,
the code in gdb/completer.c:complete_line_internal analyses the command
that requested the completion and tries to determine the type of
completion wanted by checking which completion function will be called
(e.g., filename_completer for filenames, location_completer for
locations, etc.).

This all works fine for myfirstcommand, because immediately after the
command registration the Python backend already sets its completion
function to filename_completer (which then causes the
complete_line_internal function to choose the right set of word break
chars).  However, for mysecondcommand, this decision is postponed to
when the completer function is evaluated, and the Python backend uses an
internal completer (called cmdpy_completer).  complete_line_internal
doesn't know about this internal completer, and can't choose the right
set of word break chars in time, which then leads to a bad decision when
completing the "/hom" word.

So, after a few attempts, I decided to create another callback in
"struct cmd_list_element" that will be responsible for handling the case
when there is an unknown completer function for complete_line_internal
to work with.  So far, only the Python backend uses this callback, and
only when the user provides a completer method instead of registering
the command directly with a completer class.  I think this is the best
option because it not very intrusive (all the other commands will still
work normally), but especially because the whole completion code is so
messy that it would be hard to fix this without having to redesign
things.

I have regtested this on Fedora 18 x86_64, without regressions.  I also
included a testcase.

OK to apply?

Thanks,

-- 
Sergio

gdb/
2014-03-12  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 (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_handle_brkchars): New function.
	(cmdpy_init): Set completer_handle_brkchars to
	cmdpy_completer_handle_brkchars.

gdb/testsuite/
2014-03-12  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 d36ab4e..0c4d996 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -164,6 +164,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,
+	  void (*completer_handle_brkchars) (struct cmd_list_element *self))
+{
+  cmd->completer_handle_brkchars = completer_handle_brkchars; /* Ok.  */
+}
+
 /* 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
@@ -239,6 +248,7 @@ add_cmd (const char *name, enum command_class class, void (*fun) (char *, int),
   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 c6edc87..a043734 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).  */
+
+    void (*completer_handle_brkchars) (struct cmd_list_element *self);
+
     /* 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 a5040a4..058d133 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -160,6 +160,12 @@ typedef VEC (char_ptr) *completer_ftype (struct cmd_list_element *,
 
 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 *,
+					       void (*f)
+					       (struct cmd_list_element *));
+
 /* 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 94f70a9..a9809a2 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -450,6 +450,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.
@@ -481,7 +496,6 @@ typedef enum
 }
 complete_line_internal_reason;
 
-
 /* Internal function used to handle completions.
 
 
@@ -678,6 +692,9 @@ complete_line_internal (const char *text,
 			   p--)
 			;
 		    }
+		  if (reason == handle_brkchars
+		      && c->completer_handle_brkchars != NULL)
+		    (*c->completer_handle_brkchars) (c);
 		  if (reason != handle_brkchars && c->completer != NULL)
 		    list = (*c->completer) (c, p, word);
 		}
@@ -751,6 +768,9 @@ complete_line_internal (const char *text,
 		       p--)
 		    ;
 		}
+	      if (reason == handle_brkchars
+		  && c->completer_handle_brkchars != NULL)
+		(*c->completer_handle_brkchars) (c);
 	      if (reason != handle_brkchars && c->completer != NULL)
 		list = (*c->completer) (c, p, word);
 	    }
diff --git a/gdb/completer.h b/gdb/completer.h
index 5b90773..cb9c389 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,
 				      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 c24bca7..df0aeed 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -206,6 +206,79 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
   do_cleanups (cleanup);
 }
 
+/* Python function called to determine the break characters of a
+   certain completer.  We use dummy variables for the "text" and
+   "word" arguments for the completer, and call it.  We're actually
+   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)
+{
+  cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
+  PyObject *textobj, *wordobj, *resultobj = NULL;
+  const char dummy_text[] = "dummytext";
+  const char dummy_word[] = "dummyword";
+  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.  */
+      goto done;
+    }
+
+  textobj = PyUnicode_Decode (dummy_text, sizeof (dummy_text),
+			      host_charset (), NULL);
+  if (!textobj)
+    error (_("Could not convert argument to Python string."));
+  wordobj = PyUnicode_Decode (dummy_word, sizeof (dummy_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 (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:
+
+  Py_XDECREF (resultobj);
+  do_cleanups (cleanup);
+}
+
 /* Called by gdb for command completion.  */
 
 static VEC (char_ptr) *
@@ -546,6 +619,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..9f8a5b2
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-completion.exp
@@ -0,0 +1,49 @@
+# 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"
+
+# This one should always pass.
+send_gdb "myfirstcommand /hom\t"
+gdb_test_multiple "" "myfirstcommand completion" {
+    -re "^myfirstcommand /home/$" {
+        pass "myfirstcommand completion"
+    }
+}
+
+# Just discarding whatever we typed.
+send_gdb "\n"
+gdb_test ".*"
+
+# This is the problematic one.
+send_gdb "mysecondcommand /hom\t"
+gdb_test_multiple "" "mysecondcommand completion" {
+    -re "^mysecondcommand /home $" {
+        fail "mysecondcommand completion (did not correctly complete filename)"
+    }
+    -re "^mysecondcommand /home/$" {
+        pass "mysecondcommand 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..bf7f77b
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-completion.py
@@ -0,0 +1,38 @@
+# 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 MyFirstCommand(gdb.Command):
+	def __init__(self):
+		gdb.Command.__init__(self,'myfirstcommand',gdb.COMMAND_USER,gdb.COMPLETE_FILENAME)
+
+	def invoke(self,argument,from_tty):
+		raise gdb.GdbError('not implemented')
+
+class MySecondCommand(gdb.Command):
+	def __init__(self):
+		gdb.Command.__init__(self,'mysecondcommand',gdb.COMMAND_USER)
+
+	def invoke(self,argument,from_tty):
+		raise gdb.GdbError('not implemented')
+
+	def complete(self,text,word):
+		return gdb.COMPLETE_FILENAME
+
+MyFirstCommand()
+MySecondCommand()

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

[Sergio Durigan Junior]

On Wednesday, March 12 2014, I wrote:

> Hi,

Ping.

> This PR came from a Red Hat bug that was filed recently.  I checked and
> it still exists on HEAD, so here's a proposed fix.  Although this is
> marked as a Python backend bug, this is really about the completion
> mechanism used by GDB.  Since this code reminds me of my first attempt
> to make a good noodle, it took me quite some time to fix it in a
> non-intrusive way.
>
> The problem is triggered when one registers a completion method inside a
> class in a Python script, rather than registering the command using a
> completer class directly.  For example, consider the following script:
>
>     class MyFirstCommand(gdb.Command):
>           def __init__(self):
>               gdb.Command.__init__(self,'myfirstcommand',gdb.COMMAND_USER,gdb.COMPLETE_FILENAME)
>
>               def invoke(self,argument,from_tty):
>                   raise gdb.GdbError('not implemented')
>
>     class MySecondCommand(gdb.Command):
>           def __init__(self):
>               gdb.Command.__init__(self,'mysecondcommand',gdb.COMMAND_USER)
>
>               def invoke(self,argument,from_tty):
>                   raise gdb.GdbError('not implemented')
>
>                   def complete(self,text,word):
>                       return gdb.COMPLETE_FILENAME
>
>     MyFirstCommand ()
>     MySecondCommand ()
>
> When one loads this into GDB and tries to complete filenames for both
> myfirstcommand and mysecondcommand, she gets:
>
>     (gdb) myfirstcommand /hom<TAB>
>     (gdb) myfirstcommand /home/
>                                ^
>     ...
>     (gdb) mysecondcommand /hom<TAB>
>     (gdb) mysecondcommand /home 
>                                 ^
>
> (The "^" marks the final position of the cursor after the TAB).
>
> So we see that myfirstcommand honors the COMPLETE_FILENAME class (as
> specified in the command creation), but mysecondcommand does not.  After
> some investigation, I found that the problem lies with the set of word
> break characters that is used for each case.  The set should be the same
> for both commands, but it is not.
>
> During the process of deciding which type of completion should be used,
> the code in gdb/completer.c:complete_line_internal analyses the command
> that requested the completion and tries to determine the type of
> completion wanted by checking which completion function will be called
> (e.g., filename_completer for filenames, location_completer for
> locations, etc.).
>
> This all works fine for myfirstcommand, because immediately after the
> command registration the Python backend already sets its completion
> function to filename_completer (which then causes the
> complete_line_internal function to choose the right set of word break
> chars).  However, for mysecondcommand, this decision is postponed to
> when the completer function is evaluated, and the Python backend uses an
> internal completer (called cmdpy_completer).  complete_line_internal
> doesn't know about this internal completer, and can't choose the right
> set of word break chars in time, which then leads to a bad decision when
> completing the "/hom" word.
>
> So, after a few attempts, I decided to create another callback in
> "struct cmd_list_element" that will be responsible for handling the case
> when there is an unknown completer function for complete_line_internal
> to work with.  So far, only the Python backend uses this callback, and
> only when the user provides a completer method instead of registering
> the command directly with a completer class.  I think this is the best
> option because it not very intrusive (all the other commands will still
> work normally), but especially because the whole completion code is so
> messy that it would be hard to fix this without having to redesign
> things.
>
> I have regtested this on Fedora 18 x86_64, without regressions.  I also
> included a testcase.
>
> OK to apply?
>
> Thanks,
>
> -- 
> Sergio
>
> gdb/
> 2014-03-12  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 (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_handle_brkchars): New function.
> 	(cmdpy_init): Set completer_handle_brkchars to
> 	cmdpy_completer_handle_brkchars.
>
> gdb/testsuite/
> 2014-03-12  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 d36ab4e..0c4d996 100644
> --- a/gdb/cli/cli-decode.c
> +++ b/gdb/cli/cli-decode.c
> @@ -164,6 +164,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,
> +	  void (*completer_handle_brkchars) (struct cmd_list_element *self))
> +{
> +  cmd->completer_handle_brkchars = completer_handle_brkchars; /* Ok.  */
> +}
> +
>  /* 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
> @@ -239,6 +248,7 @@ add_cmd (const char *name, enum command_class class, void (*fun) (char *, int),
>    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 c6edc87..a043734 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).  */
> +
> +    void (*completer_handle_brkchars) (struct cmd_list_element *self);
> +
>      /* 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 a5040a4..058d133 100644
> --- a/gdb/command.h
> +++ b/gdb/command.h
> @@ -160,6 +160,12 @@ typedef VEC (char_ptr) *completer_ftype (struct cmd_list_element *,
>  
>  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 *,
> +					       void (*f)
> +					       (struct cmd_list_element *));
> +
>  /* 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 94f70a9..a9809a2 100644
> --- a/gdb/completer.c
> +++ b/gdb/completer.c
> @@ -450,6 +450,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.
> @@ -481,7 +496,6 @@ typedef enum
>  }
>  complete_line_internal_reason;
>  
> -
>  /* Internal function used to handle completions.
>  
>  
> @@ -678,6 +692,9 @@ complete_line_internal (const char *text,
>  			   p--)
>  			;
>  		    }
> +		  if (reason == handle_brkchars
> +		      && c->completer_handle_brkchars != NULL)
> +		    (*c->completer_handle_brkchars) (c);
>  		  if (reason != handle_brkchars && c->completer != NULL)
>  		    list = (*c->completer) (c, p, word);
>  		}
> @@ -751,6 +768,9 @@ complete_line_internal (const char *text,
>  		       p--)
>  		    ;
>  		}
> +	      if (reason == handle_brkchars
> +		  && c->completer_handle_brkchars != NULL)
> +		(*c->completer_handle_brkchars) (c);
>  	      if (reason != handle_brkchars && c->completer != NULL)
>  		list = (*c->completer) (c, p, word);
>  	    }
> diff --git a/gdb/completer.h b/gdb/completer.h
> index 5b90773..cb9c389 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,
>  				      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 c24bca7..df0aeed 100644
> --- a/gdb/python/py-cmd.c
> +++ b/gdb/python/py-cmd.c
> @@ -206,6 +206,79 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
>    do_cleanups (cleanup);
>  }
>  
> +/* Python function called to determine the break characters of a
> +   certain completer.  We use dummy variables for the "text" and
> +   "word" arguments for the completer, and call it.  We're actually
> +   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)
> +{
> +  cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
> +  PyObject *textobj, *wordobj, *resultobj = NULL;
> +  const char dummy_text[] = "dummytext";
> +  const char dummy_word[] = "dummyword";
> +  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.  */
> +      goto done;
> +    }
> +
> +  textobj = PyUnicode_Decode (dummy_text, sizeof (dummy_text),
> +			      host_charset (), NULL);
> +  if (!textobj)
> +    error (_("Could not convert argument to Python string."));
> +  wordobj = PyUnicode_Decode (dummy_word, sizeof (dummy_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 (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:
> +
> +  Py_XDECREF (resultobj);
> +  do_cleanups (cleanup);
> +}
> +
>  /* Called by gdb for command completion.  */
>  
>  static VEC (char_ptr) *
> @@ -546,6 +619,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..9f8a5b2
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-completion.exp
> @@ -0,0 +1,49 @@
> +# 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"
> +
> +# This one should always pass.
> +send_gdb "myfirstcommand /hom\t"
> +gdb_test_multiple "" "myfirstcommand completion" {
> +    -re "^myfirstcommand /home/$" {
> +        pass "myfirstcommand completion"
> +    }
> +}
> +
> +# Just discarding whatever we typed.
> +send_gdb "\n"
> +gdb_test ".*"
> +
> +# This is the problematic one.
> +send_gdb "mysecondcommand /hom\t"
> +gdb_test_multiple "" "mysecondcommand completion" {
> +    -re "^mysecondcommand /home $" {
> +        fail "mysecondcommand completion (did not correctly complete filename)"
> +    }
> +    -re "^mysecondcommand /home/$" {
> +        pass "mysecondcommand 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..bf7f77b
> --- /dev/null
> +++ b/gdb/testsuite/gdb.python/py-completion.py
> @@ -0,0 +1,38 @@
> +# 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 MyFirstCommand(gdb.Command):
> +	def __init__(self):
> +		gdb.Command.__init__(self,'myfirstcommand',gdb.COMMAND_USER,gdb.COMPLETE_FILENAME)
> +
> +	def invoke(self,argument,from_tty):
> +		raise gdb.GdbError('not implemented')
> +
> +class MySecondCommand(gdb.Command):
> +	def __init__(self):
> +		gdb.Command.__init__(self,'mysecondcommand',gdb.COMMAND_USER)
> +
> +	def invoke(self,argument,from_tty):
> +		raise gdb.GdbError('not implemented')
> +
> +	def complete(self,text,word):
> +		return gdb.COMPLETE_FILENAME
> +
> +MyFirstCommand()
> +MySecondCommand()

-- 
Sergio

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

[Sergio Durigan Junior]

On Friday, March 21 2014, I wrote:

> On Wednesday, March 12 2014, I wrote:
>
>> Hi,
>
> Ping.

Ping^2.

>> This PR came from a Red Hat bug that was filed recently.  I checked and
>> it still exists on HEAD, so here's a proposed fix.  Although this is
>> marked as a Python backend bug, this is really about the completion
>> mechanism used by GDB.  Since this code reminds me of my first attempt
>> to make a good noodle, it took me quite some time to fix it in a
>> non-intrusive way.
>>
>> The problem is triggered when one registers a completion method inside a
>> class in a Python script, rather than registering the command using a
>> completer class directly.  For example, consider the following script:
>>
>>     class MyFirstCommand(gdb.Command):
>>           def __init__(self):
>>               gdb.Command.__init__(self,'myfirstcommand',gdb.COMMAND_USER,gdb.COMPLETE_FILENAME)
>>
>>               def invoke(self,argument,from_tty):
>>                   raise gdb.GdbError('not implemented')
>>
>>     class MySecondCommand(gdb.Command):
>>           def __init__(self):
>>               gdb.Command.__init__(self,'mysecondcommand',gdb.COMMAND_USER)
>>
>>               def invoke(self,argument,from_tty):
>>                   raise gdb.GdbError('not implemented')
>>
>>                   def complete(self,text,word):
>>                       return gdb.COMPLETE_FILENAME
>>
>>     MyFirstCommand ()
>>     MySecondCommand ()
>>
>> When one loads this into GDB and tries to complete filenames for both
>> myfirstcommand and mysecondcommand, she gets:
>>
>>     (gdb) myfirstcommand /hom<TAB>
>>     (gdb) myfirstcommand /home/
>>                                ^
>>     ...
>>     (gdb) mysecondcommand /hom<TAB>
>>     (gdb) mysecondcommand /home 
>>                                 ^
>>
>> (The "^" marks the final position of the cursor after the TAB).
>>
>> So we see that myfirstcommand honors the COMPLETE_FILENAME class (as
>> specified in the command creation), but mysecondcommand does not.  After
>> some investigation, I found that the problem lies with the set of word
>> break characters that is used for each case.  The set should be the same
>> for both commands, but it is not.
>>
>> During the process of deciding which type of completion should be used,
>> the code in gdb/completer.c:complete_line_internal analyses the command
>> that requested the completion and tries to determine the type of
>> completion wanted by checking which completion function will be called
>> (e.g., filename_completer for filenames, location_completer for
>> locations, etc.).
>>
>> This all works fine for myfirstcommand, because immediately after the
>> command registration the Python backend already sets its completion
>> function to filename_completer (which then causes the
>> complete_line_internal function to choose the right set of word break
>> chars).  However, for mysecondcommand, this decision is postponed to
>> when the completer function is evaluated, and the Python backend uses an
>> internal completer (called cmdpy_completer).  complete_line_internal
>> doesn't know about this internal completer, and can't choose the right
>> set of word break chars in time, which then leads to a bad decision when
>> completing the "/hom" word.
>>
>> So, after a few attempts, I decided to create another callback in
>> "struct cmd_list_element" that will be responsible for handling the case
>> when there is an unknown completer function for complete_line_internal
>> to work with.  So far, only the Python backend uses this callback, and
>> only when the user provides a completer method instead of registering
>> the command directly with a completer class.  I think this is the best
>> option because it not very intrusive (all the other commands will still
>> work normally), but especially because the whole completion code is so
>> messy that it would be hard to fix this without having to redesign
>> things.
>>
>> I have regtested this on Fedora 18 x86_64, without regressions.  I also
>> included a testcase.
>>
>> OK to apply?
>>
>> Thanks,
>>
>> -- 
>> Sergio
>>
>> gdb/
>> 2014-03-12  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 (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_handle_brkchars): New function.
>> 	(cmdpy_init): Set completer_handle_brkchars to
>> 	cmdpy_completer_handle_brkchars.
>>
>> gdb/testsuite/
>> 2014-03-12  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 d36ab4e..0c4d996 100644
>> --- a/gdb/cli/cli-decode.c
>> +++ b/gdb/cli/cli-decode.c
>> @@ -164,6 +164,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,
>> +	  void (*completer_handle_brkchars) (struct cmd_list_element *self))
>> +{
>> +  cmd->completer_handle_brkchars = completer_handle_brkchars; /* Ok.  */
>> +}
>> +
>>  /* 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
>> @@ -239,6 +248,7 @@ add_cmd (const char *name, enum command_class class, void (*fun) (char *, int),
>>    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 c6edc87..a043734 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).  */
>> +
>> +    void (*completer_handle_brkchars) (struct cmd_list_element *self);
>> +
>>      /* 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 a5040a4..058d133 100644
>> --- a/gdb/command.h
>> +++ b/gdb/command.h
>> @@ -160,6 +160,12 @@ typedef VEC (char_ptr) *completer_ftype (struct cmd_list_element *,
>>  
>>  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 *,
>> +					       void (*f)
>> +					       (struct cmd_list_element *));
>> +
>>  /* 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 94f70a9..a9809a2 100644
>> --- a/gdb/completer.c
>> +++ b/gdb/completer.c
>> @@ -450,6 +450,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.
>> @@ -481,7 +496,6 @@ typedef enum
>>  }
>>  complete_line_internal_reason;
>>  
>> -
>>  /* Internal function used to handle completions.
>>  
>>  
>> @@ -678,6 +692,9 @@ complete_line_internal (const char *text,
>>  			   p--)
>>  			;
>>  		    }
>> +		  if (reason == handle_brkchars
>> +		      && c->completer_handle_brkchars != NULL)
>> +		    (*c->completer_handle_brkchars) (c);
>>  		  if (reason != handle_brkchars && c->completer != NULL)
>>  		    list = (*c->completer) (c, p, word);
>>  		}
>> @@ -751,6 +768,9 @@ complete_line_internal (const char *text,
>>  		       p--)
>>  		    ;
>>  		}
>> +	      if (reason == handle_brkchars
>> +		  && c->completer_handle_brkchars != NULL)
>> +		(*c->completer_handle_brkchars) (c);
>>  	      if (reason != handle_brkchars && c->completer != NULL)
>>  		list = (*c->completer) (c, p, word);
>>  	    }
>> diff --git a/gdb/completer.h b/gdb/completer.h
>> index 5b90773..cb9c389 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,
>>  				      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 c24bca7..df0aeed 100644
>> --- a/gdb/python/py-cmd.c
>> +++ b/gdb/python/py-cmd.c
>> @@ -206,6 +206,79 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
>>    do_cleanups (cleanup);
>>  }
>>  
>> +/* Python function called to determine the break characters of a
>> +   certain completer.  We use dummy variables for the "text" and
>> +   "word" arguments for the completer, and call it.  We're actually
>> +   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)
>> +{
>> +  cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
>> +  PyObject *textobj, *wordobj, *resultobj = NULL;
>> +  const char dummy_text[] = "dummytext";
>> +  const char dummy_word[] = "dummyword";
>> +  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.  */
>> +      goto done;
>> +    }
>> +
>> +  textobj = PyUnicode_Decode (dummy_text, sizeof (dummy_text),
>> +			      host_charset (), NULL);
>> +  if (!textobj)
>> +    error (_("Could not convert argument to Python string."));
>> +  wordobj = PyUnicode_Decode (dummy_word, sizeof (dummy_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 (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:
>> +
>> +  Py_XDECREF (resultobj);
>> +  do_cleanups (cleanup);
>> +}
>> +
>>  /* Called by gdb for command completion.  */
>>  
>>  static VEC (char_ptr) *
>> @@ -546,6 +619,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..9f8a5b2
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.python/py-completion.exp
>> @@ -0,0 +1,49 @@
>> +# 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"
>> +
>> +# This one should always pass.
>> +send_gdb "myfirstcommand /hom\t"
>> +gdb_test_multiple "" "myfirstcommand completion" {
>> +    -re "^myfirstcommand /home/$" {
>> +        pass "myfirstcommand completion"
>> +    }
>> +}
>> +
>> +# Just discarding whatever we typed.
>> +send_gdb "\n"
>> +gdb_test ".*"
>> +
>> +# This is the problematic one.
>> +send_gdb "mysecondcommand /hom\t"
>> +gdb_test_multiple "" "mysecondcommand completion" {
>> +    -re "^mysecondcommand /home $" {
>> +        fail "mysecondcommand completion (did not correctly complete filename)"
>> +    }
>> +    -re "^mysecondcommand /home/$" {
>> +        pass "mysecondcommand 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..bf7f77b
>> --- /dev/null
>> +++ b/gdb/testsuite/gdb.python/py-completion.py
>> @@ -0,0 +1,38 @@
>> +# 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 MyFirstCommand(gdb.Command):
>> +	def __init__(self):
>> +		gdb.Command.__init__(self,'myfirstcommand',gdb.COMMAND_USER,gdb.COMPLETE_FILENAME)
>> +
>> +	def invoke(self,argument,from_tty):
>> +		raise gdb.GdbError('not implemented')
>> +
>> +class MySecondCommand(gdb.Command):
>> +	def __init__(self):
>> +		gdb.Command.__init__(self,'mysecondcommand',gdb.COMMAND_USER)
>> +
>> +	def invoke(self,argument,from_tty):
>> +		raise gdb.GdbError('not implemented')
>> +
>> +	def complete(self,text,word):
>> +		return gdb.COMPLETE_FILENAME
>> +
>> +MyFirstCommand()
>> +MySecondCommand()
>
> -- 
> Sergio

-- 
Sergio

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

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> This PR came from a Red Hat bug that was filed recently.  I checked and
Sergio> it still exists on HEAD, so here's a proposed fix.  Although this is
Sergio> marked as a Python backend bug, this is really about the completion
Sergio> mechanism used by GDB.  Since this code reminds me of my first attempt
Sergio> to make a good noodle, it took me quite some time to fix it in a
Sergio> non-intrusive way.

Thanks, Sergio.

Sergio> +/* Python function called to determine the break characters of a
Sergio> +   certain completer.  We use dummy variables for the "text" and
Sergio> +   "word" arguments for the completer, and call it.  We're actually
Sergio> +   only interested in knowing if the completer registered by the user
Sergio> +   will return one of the integer codes (see COMPLETER_* symbols).  */
Sergio> +
Sergio> +static void
Sergio> +cmdpy_completer_handle_brkchars (struct cmd_list_element *command)
Sergio> +{

IIUC this is calling the command's complete method with some dummy strings
to see what it will do.

I don't think this approach will work since in general the reason to
write a complete method is to be able to adapt to different arguments.
That is, there's no reason to expect that passing dummy arguments will
yield the same result.

I didn't look deeply into the problem but is there a way to "reparse"
the text when the complete method returns one of the enum values?

Tom

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

[Sergio Durigan Junior]

On Thursday, April 10 2014, Tom Tromey wrote:

> Sergio> +/* Python function called to determine the break characters of a
> Sergio> +   certain completer.  We use dummy variables for the "text" and
> Sergio> +   "word" arguments for the completer, and call it.  We're actually
> Sergio> +   only interested in knowing if the completer registered by the user
> Sergio> +   will return one of the integer codes (see COMPLETER_* symbols).  */
> Sergio> +
> Sergio> +static void
> Sergio> +cmdpy_completer_handle_brkchars (struct cmd_list_element *command)
> Sergio> +{

Thanks for the review, Tom.

> IIUC this is calling the command's complete method with some dummy strings
> to see what it will do.
>
> I don't think this approach will work since in general the reason to
> write a complete method is to be able to adapt to different arguments.
> That is, there's no reason to expect that passing dummy arguments will
> yield the same result.

Hm, you are right, and I even managed to extend the testcase in order to
trigger a failure because of this bug in my patch.

> I didn't look deeply into the problem but is there a way to "reparse"
> the text when the complete method returns one of the enum values?

Unfortunately I couldn't come up with a good way to do that.  readline
messes with the text/word being parsed while it is parsing them, so it
is hard to restart the parsing because we may not have the full context
in all situations.  Or at least that's what I found.

But I fixed my patch according to your comment above, and I think now it
is right.  What I did is simple: instead of providing dummy arguments to
the completer, I am now passing the real arguments.  As far as I have
tested, it works.

One extra comment worth making is that now the gdb.COMPLETE_COMMAND case
also works as expected, i.e., it doesn't complete filenames if it is not
told to do so.  The reported behavior on the bugzilla, which said that
"c /hom" was being completed as "c /home " is also wrong.

What do you think?  Is it too hacky?

Thank you,

-- 
Sergio

gdb/
2014-05-02  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 (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_handle_brkchars): New function.
	(cmdpy_init): Set completer_handle_brkchars to
	cmdpy_completer_handle_brkchars.

gdb/testsuite/
2014-05-02  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 d36ab4e..b6365ba 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -164,6 +164,17 @@ 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,
+	  void (*completer_handle_brkchars) (struct cmd_list_element *self,
+					     const char *text,
+					     const char *word))
+{
+  cmd->completer_handle_brkchars = completer_handle_brkchars; /* Ok.  */
+}
+
 /* 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
@@ -239,6 +250,7 @@ add_cmd (const char *name, enum command_class class, void (*fun) (char *, int),
   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 c6edc87..d7ec9b0 100644
--- a/gdb/cli/cli-decode.h
+++ b/gdb/cli/cli-decode.h
@@ -176,6 +176,16 @@ 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).  */
+
+    void (*completer_handle_brkchars) (struct cmd_list_element *self,
+				       const char *text, const char *word);
+
     /* 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 a5040a4..c3b267c 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -160,6 +160,14 @@ typedef VEC (char_ptr) *completer_ftype (struct cmd_list_element *,
 
 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 *,
+					       void (*f)
+					       (struct cmd_list_element *,
+						const char *,
+						const char *));
+
 /* 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 94f70a9..db074af 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -450,6 +450,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.
@@ -678,6 +693,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);
 		}
@@ -751,6 +769,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 5b90773..cb9c389 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,
 				      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 c24bca7..a022b4e 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -206,6 +206,78 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
   do_cleanups (cleanup);
 }
 
+/* Python function called to determine the break characters of a
+   certain completer.  We use dummy variables for the "text" and
+   "word" arguments for the completer, and call it.  We're actually
+   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)
+{
+  cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
+  PyObject *textobj, *wordobj, *resultobj = NULL;
+  /*  const char dummy_text[] = "dummytext";
+      const char dummy_word[] = "dummyword"; */
+  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.  */
+      goto done;
+    }
+
+  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)
+    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 (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:
+
+  Py_XDECREF (resultobj);
+  do_cleanups (cleanup);
+}
+
 /* Called by gdb for command completion.  */
 
 static VEC (char_ptr) *
@@ -546,6 +618,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..b8b821e
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-completion.exp
@@ -0,0 +1,70 @@
+# 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 "${objdir}/${subdir}/py-completion-testdir/"
+set testdir_regex [string_to_regexp $testdir]
+set testdir_complete "${objdir}/${subdir}/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.
+send_gdb "\n"
+gdb_test "print" ".*"
+
+# 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
+send_gdb "\n"
+gdb_test "print" ".*"
+
+# Another problematic
+send_gdb "completefilecommandcond ${objdir}/${subdir}/py-completion-t\t"
+gdb_test_multiple "" "completefilecommandcond completion" {
+    -re "^completefilecommandcond ${testdir}$" {
+	fail "completefilecommandcond completion (completed filename instead of command)"
+    }
+    -re "^completefilecommandcond ${objdir}/${subdir}/py-completion-t$" {
+	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()

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

[Tom Tromey]

>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:

Sergio> Unfortunately I couldn't come up with a good way to do that.  readline
Sergio> messes with the text/word being parsed while it is parsing them, so it
Sergio> is hard to restart the parsing because we may not have the full context
Sergio> in all situations.  Or at least that's what I found.

I looked through the readline docs here.
Ok, I see now, the completion API is irredeemably wacky.

Sergio> But I fixed my patch according to your comment above, and I think now it
Sergio> is right.  What I did is simple: instead of providing dummy arguments to
Sergio> the completer, I am now passing the real arguments.  As far as I have
Sergio> tested, it works.

Cute.

Sergio> What do you think?  Is it too hacky?

Someday we will invent a hackiness metric to let us know for sure.
"M(h) is greater than 0.98!  Patch rejected by the robot."


Seriously, at first I thought this was probably a bad idea.
And it is a little weird, since first it word-breaks some random way,
then redoes the breaking later.

Is there a way to call the Python function just once and store the
results in the non-enum-return case?  Since otherwise it seems that
every completion request requires two calls to a
possibly-expensive-though-we-hope-not completer.

Anyway I'm ok-enough with it I suppose.

Sergio> +void
Sergio> +set_cmd_completer_handle_brkchars (struct cmd_list_element *cmd,
Sergio> +	  void (*completer_handle_brkchars) (struct cmd_list_element *self,
Sergio> +					     const char *text,
Sergio> +					     const char *word))
Sergio> +{
Sergio> +  cmd->completer_handle_brkchars = completer_handle_brkchars; /* Ok.  */

I think the "Ok" comment usually is there as a note to the ARI.
However, does ARI actually check this line?
If not -> no comment needed.

Sergio> +/* Set the completer_handle_brkchars callback.  */
Sergio> +
Sergio> +extern void set_cmd_completer_handle_brkchars (struct cmd_list_element *,
Sergio> +					       void (*f)
Sergio> +					       (struct cmd_list_element *,
Sergio> +						const char *,
Sergio> +						const char *));

I think the "f" argument should have type "completer_ftype *" rather
than being spelled out.

Sergio> +static void
Sergio> +cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
Sergio> +				 const char *text, const char *word)
Sergio> +{
Sergio> +  cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
Sergio> +  PyObject *textobj, *wordobj, *resultobj = NULL;
Sergio> +  /*  const char dummy_text[] = "dummytext";
Sergio> +      const char dummy_word[] = "dummyword"; */

No need for the commented-out bits.

Sergio> +# This one should always pass.
Sergio> +send_gdb "completefileinit ${testdir_complete}\t"
Sergio> +gdb_test_multiple "" "completefileinit completion" {
Sergio> +    -re "^completefileinit ${testdir_regex}$" {
Sergio> +        pass "completefileinit completion"
Sergio> +    }

FWIW I generally find it simpler to test the "complete" command rather
than the send_gdb dance.

Either way is ok though.

Tom

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

[Sergio Durigan Junior]

On Tuesday, May 20 2014, Tom Tromey wrote:

>>>>>> "Sergio" == Sergio Durigan Junior <sergiodj@redhat.com> writes:
>
> Sergio> Unfortunately I couldn't come up with a good way to do that.  readline
> Sergio> messes with the text/word being parsed while it is parsing them, so it
> Sergio> is hard to restart the parsing because we may not have the full context
> Sergio> in all situations.  Or at least that's what I found.
>
> I looked through the readline docs here.
> Ok, I see now, the completion API is irredeemably wacky.

:-/

> Sergio> But I fixed my patch according to your comment above, and I think now it
> Sergio> is right.  What I did is simple: instead of providing dummy arguments to
> Sergio> the completer, I am now passing the real arguments.  As far as I have
> Sergio> tested, it works.
>
> Cute.
>
> Sergio> What do you think?  Is it too hacky?
>
> Someday we will invent a hackiness metric to let us know for sure.
> "M(h) is greater than 0.98!  Patch rejected by the robot."

Haha, I can only imagine how the source code of this bot would be...
Probably too hacky?  ;-)

> Seriously, at first I thought this was probably a bad idea.
> And it is a little weird, since first it word-breaks some random way,
> then redoes the breaking later.
>
> Is there a way to call the Python function just once and store the
> results in the non-enum-return case?  Since otherwise it seems that
> every completion request requires two calls to a
> possibly-expensive-though-we-hope-not completer.

Hm, OK, I came up with the following solution.  Basically, the code that
calls the Python function is isolated and, through a static variable and
a flag to inform whether we are handling brkchars or doing a completion
itself, the function now "caches" the result object of the function
call.

This code is built on the assumption that every completion will handle
brkchars first, and then do the completion itself second, in that order.
I guess it's a safe assumption given the logic behind the code.

> Anyway I'm ok-enough with it I suppose.
>
> Sergio> +void
> Sergio> +set_cmd_completer_handle_brkchars (struct cmd_list_element *cmd,
> Sergio> +	  void (*completer_handle_brkchars) (struct cmd_list_element *self,
> Sergio> +					     const char *text,
> Sergio> +					     const char *word))
> Sergio> +{
> Sergio> +  cmd->completer_handle_brkchars = completer_handle_brkchars; /* Ok.  */
>
> I think the "Ok" comment usually is there as a note to the ARI.
> However, does ARI actually check this line?
> If not -> no comment needed.

Done.

> Sergio> +/* Set the completer_handle_brkchars callback.  */
> Sergio> +
> Sergio> +extern void set_cmd_completer_handle_brkchars (struct cmd_list_element *,
> Sergio> +					       void (*f)
> Sergio> +					       (struct cmd_list_element *,
> Sergio> +						const char *,
> Sergio> +						const char *));
>
> I think the "f" argument should have type "completer_ftype *" rather
> than being spelled out.

I created a new "completer_ftype_void", because the "handle_brkchars"
function does not return anything.

> Sergio> +static void
> Sergio> +cmdpy_completer_handle_brkchars (struct cmd_list_element *command,
> Sergio> +				 const char *text, const char *word)
> Sergio> +{
> Sergio> +  cmdpy_object *obj = (cmdpy_object *) get_cmd_context (command);
> Sergio> +  PyObject *textobj, *wordobj, *resultobj = NULL;
> Sergio> +  /*  const char dummy_text[] = "dummytext";
> Sergio> +      const char dummy_word[] = "dummyword"; */
>
> No need for the commented-out bits.

Ops, sorry.  Removed.

> Sergio> +# This one should always pass.
> Sergio> +send_gdb "completefileinit ${testdir_complete}\t"
> Sergio> +gdb_test_multiple "" "completefileinit completion" {
> Sergio> +    -re "^completefileinit ${testdir_regex}$" {
> Sergio> +        pass "completefileinit completion"
> Sergio> +    }
>
> FWIW I generally find it simpler to test the "complete" command rather
> than the send_gdb dance.

Me too, but given:

<https://sourceware.org/bugzilla/show_bug.cgi?id=16265>

And:

<https://sourceware.org/ml/gdb-patches/2013-05/msg00567.html>

I am now using TAB whenever I can in the testcases.

> Either way is ok though.

Thanks.  WDYT of the following patch?

-- 
Sergio

gdb/
2014-05-20  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 (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-05-20  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 d36ab4e..ba61fa8 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -164,6 +164,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
@@ -239,6 +248,7 @@ add_cmd (const char *name, enum command_class class, void (*fun) (char *, int),
   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 c6edc87..684fea8 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 a5040a4..05b286b 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -158,8 +158,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 94f70a9..db074af 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -450,6 +450,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.
@@ -678,6 +693,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);
 		}
@@ -751,6 +769,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 5b90773..cb9c389 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,
 				      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 524ba5a..7d89131 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -208,45 +208,149 @@ 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.  */
+      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)
+	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 ();
+	}
+    }
+
+  /* The caller is responsible for deciding whether to call Py_XDECREF
+     or not.  */
+  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))
@@ -548,6 +652,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..b8b821e
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-completion.exp
@@ -0,0 +1,70 @@
+# 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 "${objdir}/${subdir}/py-completion-testdir/"
+set testdir_regex [string_to_regexp $testdir]
+set testdir_complete "${objdir}/${subdir}/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.
+send_gdb "\n"
+gdb_test "print" ".*"
+
+# 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
+send_gdb "\n"
+gdb_test "print" ".*"
+
+# Another problematic
+send_gdb "completefilecommandcond ${objdir}/${subdir}/py-completion-t\t"
+gdb_test_multiple "" "completefilecommandcond completion" {
+    -re "^completefilecommandcond ${testdir}$" {
+	fail "completefilecommandcond completion (completed filename instead of command)"
+    }
+    -re "^completefilecommandcond ${objdir}/${subdir}/py-completion-t$" {
+	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()

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

[Doug Evans]

Sergio Durigan Junior <sergiodj@redhat.com> writes:
> [...]
> Thanks.  WDYT of the following patch?

Hi.

fwiw it's too bad the ability to plug in different completers isn't more,
I dunno, parameterized (couldn't pick a better term, apologies -
I thought of "object oriented" but that carries its own baggage).
Performing completion obviously involves specifying more than a just
single function (witness the comparison of the completer with specific
functions).

Plus it's more than specifying brkchars.
Witness code like this:

		      /* Many commands which want to complete on
			 file names accept several file names, as
			 in "run foo bar >>baz".  So we don't want
			 to complete the entire text after the
			 command, just the last word.  To this
			 end, we need to find the beginning of the
			 file name by starting at `word' and going
			 backwards.  */
		      for (p = word;
			   p > tmp_command
			     && strchr (gdb_completer_file_name_break_characters, p[-1]) == NULL;
			   p--)
			;

IWBN if a "completer" object described how to do all these three things.
Then the special case code for filename_completer (and location_completer)
in completer.c could disappear.  But maybe that's a patch for another day.


Regarding the hack of using a static local to pass data from
handle_brkchars to handle_completion, I know it's a hacky pragmatic
choice.  To get the reference counting right the code assumes that
if the handle_brkchars phase is done then the handle_completion
phase will be done too, right?  I wonder if a SIGINT could sneak in
there between the two passes (either today or tomorrow).
Maybe the code in cmdpy_completer_helper for handle_brkchars_p could
first check whether resultobj is already non-NULL, and decrement its
reference count before setting it to NULL?  And cmdpy_completer_helper
could be defined to return a borrowed reference to resultobj?
Dunno, just thinking out loud.
Something puzzles me though: If it's ok to cache the completion result from the
handle_brkchars pass to the handle_completion pass, why have two passes?
It feels like there must be a case where this caching of the result
in a static local from one pass to the next won't work.


Another question:
I noticed complete_command doesn't do this two-phase dance
of handle_brkchars followed by handle_completions.  Should it?
It just goes straight to handle_completions.
[Maybe that explains the difference from using TAB.  Dunno off hand.]
It seems like complete_command is trying to hand-code its own
handle_brkchars handling:

static void
complete_command (char *arg, int from_tty)
{
  int argpoint;
  char *point, *arg_prefix;
  VEC (char_ptr) *completions;

  dont_repeat ();

  if (arg == NULL)
    arg = "";
  argpoint = strlen (arg);

  /* complete_line assumes that its first argument is somewhere
     within, and except for filenames at the beginning of, the word to
     be completed.  The following crude imitation of readline's
     word-breaking tries to accomodate this.  */
  point = arg + argpoint;
  while (point > arg)
    {
      if (strchr (rl_completer_word_break_characters, point[-1]) != 0)
        break;
      point--;
    }

  arg_prefix = alloca (point - arg + 1);
  memcpy (arg_prefix, arg, point - arg);
  arg_prefix[point - arg] = 0;

  completions = complete_line (point, arg, argpoint);

  ...
}

TAB and the complete command should work identically of course,
but for your testcase, maybe you should test both just to verify
both work reasonably well (even if not identically).
Given that complete_command doesn't do the two phase dance,
does it work with your patch?
Maybe it does, but IWBN to confirm that.

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

[Sergio Durigan Junior]

On Wednesday, May 21 2014, Doug Evans wrote:

> Sergio Durigan Junior <sergiodj@redhat.com> writes:
>> [...]
>> Thanks.  WDYT of the following patch?
>
> Hi.
>
> fwiw it's too bad the ability to plug in different completers isn't more,
> I dunno, parameterized (couldn't pick a better term, apologies -
> I thought of "object oriented" but that carries its own baggage).
> Performing completion obviously involves specifying more than a just
> single function (witness the comparison of the completer with specific
> functions).

Thanks for the reply, and sorry for the long delay in answering.

> Plus it's more than specifying brkchars.
> Witness code like this:
>
> 		      /* Many commands which want to complete on
> 			 file names accept several file names, as
> 			 in "run foo bar >>baz".  So we don't want
> 			 to complete the entire text after the
> 			 command, just the last word.  To this
> 			 end, we need to find the beginning of the
> 			 file name by starting at `word' and going
> 			 backwards.  */
> 		      for (p = word;
> 			   p > tmp_command
> 			     && strchr (gdb_completer_file_name_break_characters, p[-1]) == NULL;
> 			   p--)
> 			;
>
> IWBN if a "completer" object described how to do all these three things.
> Then the special case code for filename_completer (and location_completer)
> in completer.c could disappear.  But maybe that's a patch for another day.

While I agree with you that the completion mechanism could be better (I
myself had a lot of trouble with it), I unfortunately don't have enough
time to tackle this problem now.  So yeah, I think it will be a patch
for another day...

> Regarding the hack of using a static local to pass data from
> handle_brkchars to handle_completion, I know it's a hacky pragmatic
> choice.  To get the reference counting right the code assumes that
> if the handle_brkchars phase is done then the handle_completion
> phase will be done too, right?

Yeah.  This is true for the current code (well, except maybe for the
case you describe below...).

> I wonder if a SIGINT could sneak in
> there between the two passes (either today or tomorrow).
> Maybe the code in cmdpy_completer_helper for handle_brkchars_p could
> first check whether resultobj is already non-NULL, and decrement its
> reference count before setting it to NULL?

Yes, done (I think).  Thanks for raising this issue.

> And cmdpy_completer_helper
> could be defined to return a borrowed reference to resultobj?
> Dunno, just thinking out loud.

Done, I guess.

> Something puzzles me though: If it's ok to cache the completion result from the
> handle_brkchars pass to the handle_completion pass, why have two passes?
> It feels like there must be a case where this caching of the result
> in a static local from one pass to the next won't work.

I'm not sure I follow.

It's OK to cache the result because handle_brkchars and
handle_completion work on the same set of data.  In fact, we wouldn't
need to have two passes here; my previous patch didn't have two passes,
and worked fine.  I just implemented it this way because Tom (correctly)
raised the point that the completion itself might be time-consuming, and
thus we could reuse its result in the two phases.

> Another question:
> I noticed complete_command doesn't do this two-phase dance
> of handle_brkchars followed by handle_completions.  Should it?
> It just goes straight to handle_completions.

I don't think it should, because for complete_command (and
complete_filename et al) the handle_brkchars is already defined
internally by GDB.  See:

    ...
  /* Choose the default set of word break characters to break
     completions.  If we later find out that we are doing completions
     on command strings (as opposed to strings supplied by the
     individual command completer functions, which can be any string)
     then we will switch to the special word break set for command
     strings, which leaves out the '-' character used in some
     commands.  */
  rl_completer_word_break_characters =
    current_language->la_word_break_characters();
    ...

    /* It is a normal command; what comes after it is
       completed by the command's completer function.  */
    if (c->completer == filename_completer)
      {
        /* Many commands which want to complete on
           file names accept several file names, as
           in "run foo bar >>baz".  So we don't want
           to complete the entire text after the
           command, just the last word.  To this
           end, we need to find the beginning of the
           file name by starting at `word' and going
           backwards.  */
        for (p = word;
             p > tmp_command
               && strchr (gdb_completer_file_name_break_characters, p[-1]) == NULL;
             p--)
          ;
        rl_completer_word_break_characters =
          gdb_completer_file_name_break_characters;
      }
    else if (c->completer == location_completer)
      {
        /* Commands which complete on locations want to
           see the entire argument.  */
        for (p = word;
             p > tmp_command
               && p[-1] != ' ' && p[-1] != '\t';
             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);


The complete_command function will only be called by the last "if"
clause, when reason != handle_brkchars.  Otherwise,
complete_line_internal will just deal with handle_brkchars.  And for
complete_command, the right value for rl_completer_word_break_characters
is what is attributed at the beginning of the function.

My patch implements this "two-phase" dance for Python because in this
specific case (i.e., a completion method defined in the Python script)
there is no way to know the value of handle_brkchars before we actually
do the completion itself.

In fact, my patch could probably be "simplified" and be made to call
cmdpy_completer directly (without any cmdpy_completer_handle_brkchars),
assuming that this function took care of filling handle_brkchars
correctly.  What I mean is: when dealing with the handle_brkchars case,
the completer command can do anything it wants.

> [Maybe that explains the difference from using TAB.  Dunno off hand.]
> It seems like complete_command is trying to hand-code its own
> handle_brkchars handling:
>
> static void
> complete_command (char *arg, int from_tty)
> {
>   int argpoint;
>   char *point, *arg_prefix;
>   VEC (char_ptr) *completions;
>
>   dont_repeat ();
>
>   if (arg == NULL)
>     arg = "";
>   argpoint = strlen (arg);
>
>   /* complete_line assumes that its first argument is somewhere
>      within, and except for filenames at the beginning of, the word to
>      be completed.  The following crude imitation of readline's
>      word-breaking tries to accomodate this.  */
>   point = arg + argpoint;
>   while (point > arg)
>     {
>       if (strchr (rl_completer_word_break_characters, point[-1]) != 0)
>         break;
>       point--;
>     }
>
>   arg_prefix = alloca (point - arg + 1);
>   memcpy (arg_prefix, arg, point - arg);
>   arg_prefix[point - arg] = 0;
>
>   completions = complete_line (point, arg, argpoint);
>
>   ...
> }

Yes, it seems so, but I did not check further.

> TAB and the complete command should work identically of course,
> but for your testcase, maybe you should test both just to verify
> both work reasonably well (even if not identically).
> Given that complete_command doesn't do the two phase dance,
> does it work with your patch?
> Maybe it does, but IWBN to confirm that.

The 'complete' command does not work as it should with my patch:

  (gdb) complete completefileinit /hom
  completefileinit /home
  (gdb) complete completefilemethod /hom
  completefilemethod /home

But then, it also does not work without my patch either:

  (gdb) complete file /hom
  file /home

So I am not extending the testcase for now, because this bug needs to be
fixed first (and it is unrelated to the patch I am proposing).

WDYT of this version?

Thanks,

-- 
Sergio
GPG key ID: 65FC5E36
Please send encrypted e-mail if possible
http://blog.sergiodj.net/

gdb/
2014-06-30  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-06-30  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 d36ab4e..ba61fa8 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -164,6 +164,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
@@ -239,6 +248,7 @@ add_cmd (const char *name, enum command_class class, void (*fun) (char *, int),
   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 c6edc87..684fea8 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 bc9728f..5df6af7 100644
--- a/gdb/command.h
+++ b/gdb/command.h
@@ -158,8 +158,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 64b146b..a8ced65 100644
--- a/gdb/completer.c
+++ b/gdb/completer.c
@@ -450,6 +450,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.
@@ -678,6 +693,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);
 		}
@@ -751,6 +769,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 524ba5a..cfb65cb 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..b8b821e
--- /dev/null
+++ b/gdb/testsuite/gdb.python/py-completion.exp
@@ -0,0 +1,70 @@
+# 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 "${objdir}/${subdir}/py-completion-testdir/"
+set testdir_regex [string_to_regexp $testdir]
+set testdir_complete "${objdir}/${subdir}/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.
+send_gdb "\n"
+gdb_test "print" ".*"
+
+# 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
+send_gdb "\n"
+gdb_test "print" ".*"
+
+# Another problematic
+send_gdb "completefilecommandcond ${objdir}/${subdir}/py-completion-t\t"
+gdb_test_multiple "" "completefilecommandcond completion" {
+    -re "^completefilecommandcond ${testdir}$" {
+	fail "completefilecommandcond completion (completed filename instead of command)"
+    }
+    -re "^completefilecommandcond ${objdir}/${subdir}/py-completion-t$" {
+	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()

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

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

Hi Sergio,

the testcase FAILs with "read1" from:
	reproducer for races of expect incomplete reads
	http://sourceware.org/bugzilla/show_bug.cgi?id=12649

Additionally the testcase uses $objdir instead of [standard_output_file],
filed for it now tracking PR:
	$objdir and $subdir should be tainted
	https://sourceware.org/bugzilla/show_bug.cgi?id=17129


Jan

[-- Attachment #2: py-completion-race.patch --]
[-- Type: text/plain, Size: 1718 bytes --]

--- gdb.python/py-completion.exp-orig	2014-07-07 21:32:17.891045058 +0200
+++ gdb.python/py-completion.exp	2014-07-08 17:29:36.500459749 +0200
@@ -26,9 +26,9 @@ if { [skip_python_tests] } { continue }
 gdb_test_no_output "source ${srcdir}/${subdir}/${testfile}.py"
 
 # Create a temporary directory
-set testdir "${objdir}/${subdir}/py-completion-testdir/"
+set testdir "[standard_output_file "py-completion-testdir"]/"
 set testdir_regex [string_to_regexp $testdir]
-set testdir_complete "${objdir}/${subdir}/py-completion-test"
+set testdir_complete [standard_output_file "py-completion-test"]
 file mkdir $testdir
 
 # This one should always pass.
@@ -40,8 +40,7 @@ gdb_test_multiple "" "completefileinit c
 }
 
 # Just discarding whatever we typed.
-send_gdb "\n"
-gdb_test "print" ".*"
+gdb_test " " ".*" "discard #1"
 
 # This is the problematic one.
 send_gdb "completefilemethod ${testdir_complete}\t"
@@ -55,16 +54,16 @@ gdb_test_multiple "" "completefilemethod
 }
 
 # Discarding again
-send_gdb "\n"
-gdb_test "print" ".*"
+gdb_test " " ".*" "discard #2"
 
 # Another problematic
-send_gdb "completefilecommandcond ${objdir}/${subdir}/py-completion-t\t"
+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 ${objdir}/${subdir}/py-completion-t$" {
+    -re "^completefilecommandcond ${completion_regex}$" {
 	pass "completefilecommandcond completion"
     }
 }

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

[Jan Kratochvil]

[-- Attachment #1: Type: text/plain, Size: 365 bytes --]

On Tue, 08 Jul 2014 17:32:21 +0200, Jan Kratochvil wrote:
> -    -re "^completefilecommandcond ${objdir}/${subdir}/py-completion-t$" {
> +    -re "^completefilecommandcond ${completion_regex}$" {

There was a racy bug here - and even in the former test - here should be:
  +    -re "^completefilecommandcond ${completion_regex}\007$" {

Updated fix attached.


Jan

[-- Attachment #2: py-completion-race2.patch --]
[-- Type: text/plain, Size: 1722 bytes --]

--- gdb.python/py-completion.exp-orig	2014-07-07 21:32:17.891045058 +0200
+++ gdb.python/py-completion.exp	2014-07-08 20:14:57.189791928 +0200
@@ -26,9 +26,9 @@ if { [skip_python_tests] } { continue }
 gdb_test_no_output "source ${srcdir}/${subdir}/${testfile}.py"
 
 # Create a temporary directory
-set testdir "${objdir}/${subdir}/py-completion-testdir/"
+set testdir "[standard_output_file "py-completion-testdir"]/"
 set testdir_regex [string_to_regexp $testdir]
-set testdir_complete "${objdir}/${subdir}/py-completion-test"
+set testdir_complete [standard_output_file "py-completion-test"]
 file mkdir $testdir
 
 # This one should always pass.
@@ -40,8 +40,7 @@ gdb_test_multiple "" "completefileinit c
 }
 
 # Just discarding whatever we typed.
-send_gdb "\n"
-gdb_test "print" ".*"
+gdb_test " " ".*" "discard #1"
 
 # This is the problematic one.
 send_gdb "completefilemethod ${testdir_complete}\t"
@@ -55,16 +54,16 @@ gdb_test_multiple "" "completefilemethod
 }
 
 # Discarding again
-send_gdb "\n"
-gdb_test "print" ".*"
+gdb_test " " ".*" "discard #2"
 
 # Another problematic
-send_gdb "completefilecommandcond ${objdir}/${subdir}/py-completion-t\t"
+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 ${objdir}/${subdir}/py-completion-t$" {
+    -re "^completefilecommandcond ${completion_regex}\007$" {
 	pass "completefilecommandcond completion"
     }
 }

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

[Sergio Durigan Junior]

On Tuesday, July 08 2014, Jan Kratochvil wrote:

> Hi Sergio,
>
> the testcase FAILs with "read1" from:
> 	reproducer for races of expect incomplete reads
> 	http://sourceware.org/bugzilla/show_bug.cgi?id=12649
>
> Additionally the testcase uses $objdir instead of [standard_output_file],
> filed for it now tracking PR:
> 	$objdir and $subdir should be tainted
> 	https://sourceware.org/bugzilla/show_bug.cgi?id=17129

On Tuesday, July 08 2014, Jan Kratochvil wrote:

> On Tue, 08 Jul 2014 17:32:21 +0200, Jan Kratochvil wrote:
>> -    -re "^completefilecommandcond ${objdir}/${subdir}/py-completion-t$" {
>> +    -re "^completefilecommandcond ${completion_regex}$" {
>
> There was a racy bug here - and even in the former test - here should be:
>   +    -re "^completefilecommandcond ${completion_regex}\007$" {
>
> Updated fix attached.

Hello Jan,

Thank you for the catches.  I will update my local version.

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

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

[Sergio Durigan Junior]

On Tuesday, July 08 2014, I wrote:

> Thank you for the catches.  I will update my local version.

Ping.

-- 
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()

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

[Sergio Durigan Junior]

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/

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

[Doug Evans]

Sergio Durigan Junior <sergiodj@redhat.com> writes:
> 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.

Hi.

I've spent some more time looking at the patch.

I have one style nit and one request.

Style nit: I see lots of places that do "if (!ptr)".
Convention is to write this as "if (ptr == NULL)".

Request:
The "why" of things is explained sufficiently well in your
original email:
https://sourceware.org/ml/gdb-patches/2014-03/msg00301.html
And while I can appreciate this text being added to the commit message,
I don't want to have to read commit logs to have a basic understanding
of the "why" of the code. [I'm all for more deeper understanding
being deferred to commit logs as appropriate, but I should be able
to get a basic understanding from the code itself.]

Can you add something descriptive to the code?

For example, how about something like this?

--- python/py-cmd.c=    2014-09-02 20:28:57.838267408 -0700
+++ python/py-cmd.c     2014-09-02 20:59:40.026083464 -0700
@@ -219,6 +219,14 @@ cmdpy_function (struct cmd_list_element
    returns that variable, without actually calling the Python method
    again.  This saves us one Python method call.

+   The reason for this two step dance is that we need to know the set
+   of "brkchars" to use early on, before we actually try to perform the
+   completion.  But if a Python command supplies a "complete" method then
+   we have to call that method first: it may return as its result the kind
+   of completion to perform and that will in turn specify which brkchars
+   to use.  IOW, we need the result of the "complete" method before we
+   actually perform the completion.
+
    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


I have no more comments so LGTM with those changes.

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

[Sergio Durigan Junior]

On Wednesday, September 03 2014, Doug Evans wrote:

> Hi.

Hi.

> I've spent some more time looking at the patch.

Thanks.

> I have one style nit and one request.
>
> Style nit: I see lots of places that do "if (!ptr)".
> Convention is to write this as "if (ptr == NULL)".

I know.  However, this code already existed; I am only moving parts of
it.  I was in doubt if it would be good to rewrite those bad style
checks, but decided to keep them.  Anyway, I will rewrite before I push.

> Request:
> The "why" of things is explained sufficiently well in your
> original email:
> https://sourceware.org/ml/gdb-patches/2014-03/msg00301.html
> And while I can appreciate this text being added to the commit message,
> I don't want to have to read commit logs to have a basic understanding
> of the "why" of the code. [I'm all for more deeper understanding
> being deferred to commit logs as appropriate, but I should be able
> to get a basic understanding from the code itself.]
>
> Can you add something descriptive to the code?

Fair enough.

> For example, how about something like this?
>
> --- python/py-cmd.c=    2014-09-02 20:28:57.838267408 -0700
> +++ python/py-cmd.c     2014-09-02 20:59:40.026083464 -0700
> @@ -219,6 +219,14 @@ cmdpy_function (struct cmd_list_element
>     returns that variable, without actually calling the Python method
>     again.  This saves us one Python method call.
>
> +   The reason for this two step dance is that we need to know the set
> +   of "brkchars" to use early on, before we actually try to perform the
> +   completion.  But if a Python command supplies a "complete" method then
> +   we have to call that method first: it may return as its result the kind
> +   of completion to perform and that will in turn specify which brkchars
> +   to use.  IOW, we need the result of the "complete" method before we
> +   actually perform the completion.
> +
>     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

Yeah, looks good, I will add this.

> I have no more comments so LGTM with those changes.

Thanks, this is what I pushed.

  <https://sourceware.org/ml/gdb-cvs/2014-09/msg00007.html>

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

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 80961d6..d416623 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,25 @@
+2014-09-03  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.
+
 2014-09-03  Gary Benson  <gbenson@redhat.com>
 
 	* nat/x86-dregs.h (ALL_DEBUG_REGISTERS): Renamed as...
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..8bc4bf7 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -208,45 +208,163 @@ 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.
+
+   The reason for this two step dance is that we need to know the set
+   of "brkchars" to use early on, before we actually try to perform
+   the completion.  But if a Python command supplies a "complete"
+   method then we have to call that method first: it may return as its
+   result the kind of completion to perform and that will in turn
+   specify which brkchars to use.  IOW, we need the result of the
+   "complete" method before we actually perform the completion.
+
+   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 == NULL)
+	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 == NULL)
+	error (_("Could not convert argument to Python string."));
+      wordobj = PyUnicode_Decode (word, sizeof (word), host_charset (), NULL);
+      if (wordobj == NULL)
+	{
+	  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 +420,6 @@ cmdpy_completer (struct cmd_list_element *command,
 
  done:
 
-  Py_XDECREF (resultobj);
   do_cleanups (cleanup);
 
   return result;
@@ -548,6 +665,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/ChangeLog b/gdb/testsuite/ChangeLog
index d996b7c..d6db24b 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2014-09-03  Sergio Durigan Junior  <sergiodj@redhat.com>
+
+	PR python/16699
+	* gdb.python/py-completion.exp: New file.
+	* gdb.python/py-completion.py: Likewise.
+
 2014-08-28  Doug Evans  <dje@google.com>
 
 	* gdb.arch/amd64-pseudo.c (main): Rewrite to better specify when
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()