From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 85471 invoked by alias); 8 Apr 2015 18:50:36 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 85458 invoked by uid 89); 8 Apr 2015 18:50:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 08 Apr 2015 18:50:34 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id t38IoX22022651 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Wed, 8 Apr 2015 14:50:33 -0400 Received: from valrhona.uglyboxes.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t38IoVBA009727 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 8 Apr 2015 14:50:32 -0400 Message-ID: <55257877.3040604@redhat.com> Date: Wed, 08 Apr 2015 18:50:00 -0000 From: Keith Seitz User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.5.0 MIME-Version: 1.0 To: Sergio Durigan Junior , GDB Patches Subject: Re: [PATCH] Fix Python completion when using the "complete" command References: <87d23ovk55.fsf@redhat.com> <87ego4txco.fsf@redhat.com> <87vbh74uqd.fsf@redhat.com> In-Reply-To: <87vbh74uqd.fsf@redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2015-04/txt/msg00292.txt.bz2 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