This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix Python completion when using the "complete" command


On 04/07/2015 12:13 PM, Sergio Durigan Junior wrote:
>diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
>index 1d89912..29d8d90 100644
>--- a/gdb/python/py-cmd.c
>+++ b/gdb/python/py-cmd.c
>@@ -210,85 +210,70 @@ cmdpy_function (struct cmd_list_element *command, char *args, int from_tty)
>  /* 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.
>+   these arguments.
>+
>+   This function is usually called twice: one when we are figuring out

nitpick (sorry): "once" instead of "one"

>+   the break characters to be used, and another to perform the real
>+   completion itself.  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.  The only situation when this function is
>+   not called twice is when the user uses the "complete" command: in
>+   this scenario, there is no call to determine the "brkchars".
>+
>+   Ideally, it would be nice to cache the result of the first call (to
>+   determine the "brkchars") and return this value directly in the
>+   second call (to perform the actual completion).  However, due to
>+   the peculiarity of the "complete" command mentioned above, it is
>+   possible to put GDB in a bad state if you perform a TAB-completion
>+   and then a "complete"-completion sequentially.  Therefore, we just
>+   recalculate everything twice for TAB-completions.
>
>     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)
>+			const char *text, const char *word)
>  {
>    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;
>+  PyObject *resultobj;
>
>-  if (handle_brkchars_p)
>+  if (obj == NULL)
>+    error (_("Invalid invocation of Python command object."));
>+  if (!PyObject_HasAttr ((PyObject *) obj, complete_cst))
>      {
>-      /* 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."));
>-	}
>+      /* If there is no complete method, don't error.  */
>+      return NULL;
>+    }
>
>-      resultobj = PyObject_CallMethodObjArgs ((PyObject *) obj, complete_cst,
>-					      textobj, wordobj, 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, strlen (word), host_charset (), NULL);
>+  if (wordobj == NULL)
>+    {
>        Py_DECREF (textobj);
>-      Py_DECREF (wordobj);
>-      if (!resultobj)
>-	{
>-	  /* Just swallow errors here.  */
>-	  PyErr_Clear ();
>-	}
>+      error (_("Could not convert argument to Python string."));
>+    }
>
>-      Py_XINCREF (resultobj);
>+  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;
>  }
>

This looks good to me.

I have applied this patch to my completer branch, and I can verify that it fixes the (other) completion problems I've seen. I recommend that a maintainer approve this.

Thank you for taking a look at this so quickly!

Keith


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]