From patchwork Sat May 3 00:04:15 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 796 Return-Path: X-Original-To: siddhesh@wilcox.dreamhost.com Delivered-To: siddhesh@wilcox.dreamhost.com Received: from homiemail-mx22.g.dreamhost.com (mx2.sub5.homie.mail.dreamhost.com [208.113.200.128]) by wilcox.dreamhost.com (Postfix) with ESMTP id 6CF0A360765 for ; Fri, 2 May 2014 17:04:28 -0700 (PDT) Received: by homiemail-mx22.g.dreamhost.com (Postfix, from userid 14314964) id EB5C35006245; Fri, 2 May 2014 17:04:27 -0700 (PDT) X-Original-To: gdb@patchwork.siddhesh.in Delivered-To: x14314964@homiemail-mx22.g.dreamhost.com Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by homiemail-mx22.g.dreamhost.com (Postfix) with ESMTPS id 6A9DB500379D for ; Fri, 2 May 2014 17:04:26 -0700 (PDT) DomainKey-Signature: a=rsa-sha1; c=nofws; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:references:date:message-id :mime-version:content-type; q=dns; s=default; b=SM7RqNWL08jH0OVc nWnlYOJlttnM/t7a+pUSshpVAP2aR+XIbGYHgrpzcD2tn9JWy+wWkV/9iqo5588y 23L1GvHe7GxOgXqOg39VlLBA7rPA9Bl9X0UIuyqK76CAK5hc+N6+XsUvx4bnihMW 8OPRAsBDyUZnBt03UPW1sl73xOA= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=sourceware.org; h=list-id :list-unsubscribe:list-subscribe:list-archive:list-post :list-help:sender:from:to:cc:subject:references:date:message-id :mime-version:content-type; s=default; bh=wshqYEFgdK7FaLWUZ2V+pT 4ySoI=; b=g7Y8V8RBGWCBSp1IvQP8N27l0f9AyTkqp0IlPOpAkIVUVNOyMAu3k1 swwWyCH44nEvzxwADx8E3pmWbvTJ1lwGdX5A4YRJ7j+PlTv9XVYzF/IRwD9dDGJn HuB7CJGyqoiepN2YJHCFLPJGenSaxvQ7Ubt+Gy2GU8loEaURMjjPo= Received: (qmail 17062 invoked by alias); 3 May 2014 00:04:24 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 17053 invoked by uid 89); 3 May 2014 00:04:23 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.6 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS 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 ESMTP; Sat, 03 May 2014 00:04:21 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s4304KPm001171 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Fri, 2 May 2014 20:04:20 -0400 Received: from psique ([10.3.113.6]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s4304Gvr022476 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Fri, 2 May 2014 20:04:17 -0400 From: Sergio Durigan Junior To: Tom Tromey Cc: GDB Patches , Phil Muldoon Subject: Re: [PATCH] PR python/16699: GDB Python command completion with overriden complete vs. completer class References: <87a9btqhe3.fsf@fleche.redhat.com> X-URL: http://www.redhat.com Date: Fri, 02 May 2014 21:04:15 -0300 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.1 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes X-DH-Original-To: gdb@patchwork.siddhesh.in 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, 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 . + +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 . + +# 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()