From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 9826 invoked by alias); 30 Jun 2017 13:33:34 -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 8199 invoked by uid 89); 30 Jun 2017 13:33:33 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy= 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; Fri, 30 Jun 2017 13:33:30 +0000 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 52A2F30EF6B; Fri, 30 Jun 2017 13:33:29 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 52A2F30EF6B Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=sergiodj@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com 52A2F30EF6B Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 0B3B160A98; Fri, 30 Jun 2017 13:33:28 +0000 (UTC) From: Sergio Durigan Junior To: Pedro Alves Cc: GDB Patches , Simon Marchi , Jerome Guitton Subject: Re: [PATCH] PR cli/21688: Detect aliases when issuing python/compile/guile commands (and fix last commit) References: <2f8ebe75-29a9-71bb-008a-d4272441acac@redhat.com> <20170630123426.2124-1-sergiodj@redhat.com> <68b70606-368f-63ba-b410-5d03865337df@redhat.com> Date: Fri, 30 Jun 2017 13:33:00 -0000 In-Reply-To: <68b70606-368f-63ba-b410-5d03865337df@redhat.com> (Pedro Alves's message of "Fri, 30 Jun 2017 14:01:51 +0100") Message-ID: <87mv8pk3br.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2017-06/txt/msg00824.txt.bz2 On Friday, June 30 2017, Pedro Alves wrote: > On 06/30/2017 01:34 PM, Sergio Durigan Junior wrote: > >> @@ -966,6 +952,8 @@ process_next_line (char *p, struct command_line **command, int parse_commands, >> const char *cmd_name = p; >> struct cmd_list_element *cmd >> = lookup_cmd_1 (&cmd_name, cmdlist, NULL, 1); >> + const char *lookup_cmd = skip_spaces_const (cmd_name); >> + bool inline_cmd = *lookup_cmd != '\0'; > > The "lookup_cmd" in my suggestion: > > ~~~~~ > lookup_cmd = skip_spaces_const (cmd_name); > ~~~~~ > > was a typo/pasto from "lookup_cmd_1"... I meant: > > cmd_name = skip_spaces_const (cmd_name); > > Fine with me to use a new variable like you had, but > it should have a name that actually means something > related to the task at hand. "cmd_arg" or something. Right. I'll use cmd_name then, since we're not using it anywhere else after that point. >> /* Note that we ignore the inline "guile command" form here. */ >> *command = build_command_line (guile_control, ""); >> diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog >> index 06bf5a4..6160c4b 100644 >> --- a/gdb/testsuite/ChangeLog >> +++ b/gdb/testsuite/ChangeLog >> @@ -1,6 +1,12 @@ >> 2017-06-30 Sergio Durigan Junior >> >> PR cli/21688 >> + * gdb.python/py-cmd.exp (test_python_inline_or_multiline): Add new >> + tests for alias commands and trailing whitespace. >> + >> +2017-06-30 Sergio Durigan Junior >> + >> + PR cli/21688 >> * gdb.python/py-cmd.exp (test_python_inline_or_multiline): New >> procedure. Call it. >> >> diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp >> index 39bb785..287ecda 100644 >> --- a/gdb/testsuite/gdb.python/py-cmd.exp >> +++ b/gdb/testsuite/gdb.python/py-cmd.exp >> @@ -194,12 +194,25 @@ proc test_python_inline_or_multiline { } { >> { "end" " >$" "multi-line first end" } >> { "end" "hello\r\n" "multi-line last end" } } >> >> + # This also tests trailing whitespace on the command. >> + set define_cmd_alias_not_inline { >> + { "if 1" " >$" "multi-line if 1 alias" } >> + { "py " " >$" "multi-line python command alias" } >> + { "print ('hello')" " >$" "multi-line print alias" } >> + { "end" " >$" "multi-line first end alias" } >> + { "end" "hello\r\n" "multi-line last end alias" } } >> + >> set define_cmd_inline { >> { "if 1" " >$" "inline if 1" } >> { "python print ('hello')" " >$" "inline python command" } >> { "end" "hello\r\n" "inline end" } } >> >> - foreach t [list $define_cmd_not_inline $define_cmd_inline] { >> + set define_cmd_alias_inline { >> + { "if 1" " >$" "inline if 1 alias" } >> + { "py print ('hello')" " >$" "inline python command alias" } >> + { "end" "hello\r\n" "inline end alias" } } >> + > > Any reason you didn't add a test for the "alias foo=python" case? > We want to be sure that aliases that are not abbreviations are > handled too. "py" is probably really implemented as a > disambiguating alias, due to "python-interactive", but it could be an > abbreviation too [py, pyt, pyth], etc. I think an explicit test for > a non-abbreviation alias would be good, to be sure the code isn't just > doing a "startswith"-like check. Otherwise, I'm going to ask for a > test that exercises abbreviations, like for example "compil", but you > don't want that. :-) I didn't think about including a test for explicitly set aliases at first, but thanks for pointing that out. It's now included. Here's the final version of the patch. -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/ >From dc4bde35d16df749e529229657b3468417937cfc Mon Sep 17 00:00:00 2001 From: Sergio Durigan Junior Date: Fri, 30 Jun 2017 08:27:29 -0400 Subject: [PATCH] PR cli/21688: Detect aliases when issuing python/compile/guile commands (and fix last commit) My last commit fixed a regression that happened when using inline/multi-line commands for Python/Compile/Guile, but introduced another regression: it is now not possible to use aliases for the commands mentioned above. The fix is to almost revert the change I've made and go back to using the 'struct cmd_list_element *', but at the same time make sure that we advance the 'cmd_name' variable past all the whitespace characters after the command name. If, after skipping the whitespace, we encounter a '\0', it means that the command is not inline. Otherwise, it is. This patch also expands the testcase in order to check for aliases and for trailing whitespace after the command name. gdb/ChangeLog: 2017-06-30 Sergio Durigan Junior Pedro Alves PR cli/21688 * cli/cli-script.c (command_name_equals_not_inline): Remove function. (process_next_line): New variable 'inline_cmd'. Adjust 'if' clauses for "python", "compile" and "guile" to use 'command_name_equals' and check for '!inline_cmd'. gdb/testsuite/ChangeLog: 2017-06-30 Sergio Durigan Junior PR cli/21688 * gdb.python/py-cmd.exp (test_python_inline_or_multiline): Add new tests for alias commands and trailing whitespace. --- gdb/ChangeLog | 9 +++++++++ gdb/cli/cli-script.c | 22 +++++----------------- gdb/testsuite/ChangeLog | 6 ++++++ gdb/testsuite/gdb.python/py-cmd.exp | 35 ++++++++++++++++++++++++++++++++++- 4 files changed, 54 insertions(+), 18 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index 4cd7aad..7080256 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,4 +1,13 @@ 2017-06-30 Sergio Durigan Junior + Pedro Alves + + PR cli/21688 + * cli/cli-script.c (command_name_equals_not_inline): Remove function. + (process_next_line): New variable 'inline_cmd'. + Adjust 'if' clauses for "python", "compile" and "guile" to use + 'command_name_equals' and check for '!inline_cmd'. + +2017-06-30 Sergio Durigan Junior PR cli/21688 * cli/cli-script.c (command_name_equals_not_inline): New function. diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c index 72f316f..5674404 100644 --- a/gdb/cli/cli-script.c +++ b/gdb/cli/cli-script.c @@ -900,20 +900,6 @@ command_name_equals (struct cmd_list_element *cmd, const char *name) && strcmp (cmd->name, name) == 0); } -/* Return true if NAME is the only command between COMMAND_START and - COMMAND_END. This is useful when we want to know whether the - command is inline (i.e., has arguments like 'python command1') or - is the start of a multi-line command block. */ - -static bool -command_name_equals_not_inline (const char *command_start, - const char *command_end, - const char *name) -{ - return (command_end - command_start == strlen (name) - && startswith (command_start, name)); -} - /* Given an input line P, skip the command and return a pointer to the first argument. */ @@ -966,6 +952,8 @@ process_next_line (char *p, struct command_line **command, int parse_commands, const char *cmd_name = p; struct cmd_list_element *cmd = lookup_cmd_1 (&cmd_name, cmdlist, NULL, 1); + cmd_name = skip_spaces_const (cmd_name); + bool inline_cmd = *cmd_name != '\0'; /* If commands are parsed, we skip initial spaces. Otherwise, which is the case for Python commands and documentation @@ -1011,20 +999,20 @@ process_next_line (char *p, struct command_line **command, int parse_commands, { *command = build_command_line (commands_control, line_first_arg (p)); } - else if (command_name_equals_not_inline (p_start, p_end, "python")) + else if (command_name_equals (cmd, "python") && !inline_cmd) { /* Note that we ignore the inline "python command" form here. */ *command = build_command_line (python_control, ""); } - else if (command_name_equals_not_inline (p_start, p_end, "compile")) + else if (command_name_equals (cmd, "compile") && !inline_cmd) { /* Note that we ignore the inline "compile command" form here. */ *command = build_command_line (compile_control, ""); (*command)->control_u.compile.scope = COMPILE_I_INVALID_SCOPE; } - else if (command_name_equals_not_inline (p_start, p_end, "guile")) + else if (command_name_equals (cmd, "guile") && !inline_cmd) { /* Note that we ignore the inline "guile command" form here. */ *command = build_command_line (guile_control, ""); diff --git a/gdb/testsuite/ChangeLog b/gdb/testsuite/ChangeLog index 06bf5a4..6160c4b 100644 --- a/gdb/testsuite/ChangeLog +++ b/gdb/testsuite/ChangeLog @@ -1,6 +1,12 @@ 2017-06-30 Sergio Durigan Junior PR cli/21688 + * gdb.python/py-cmd.exp (test_python_inline_or_multiline): Add new + tests for alias commands and trailing whitespace. + +2017-06-30 Sergio Durigan Junior + + PR cli/21688 * gdb.python/py-cmd.exp (test_python_inline_or_multiline): New procedure. Call it. diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp index 39bb785..953d52a 100644 --- a/gdb/testsuite/gdb.python/py-cmd.exp +++ b/gdb/testsuite/gdb.python/py-cmd.exp @@ -187,6 +187,8 @@ gdb_test "complete expr_test bar\." \ # This proc tests PR cli/21688. The PR is not language-specific, but # the easiest way is just to test with Python. proc test_python_inline_or_multiline { } { + global gdb_prompt + set define_cmd_not_inline { { "if 1" " >$" "multi-line if 1" } { "python" " >$" "multi-line python command" } @@ -194,12 +196,43 @@ proc test_python_inline_or_multiline { } { { "end" " >$" "multi-line first end" } { "end" "hello\r\n" "multi-line last end" } } + # This also tests trailing whitespace on the command. + set define_cmd_alias_not_inline { + { "if 1" " >$" "multi-line if 1 alias" } + { "py " " >$" "multi-line python command alias" } + { "print ('hello')" " >$" "multi-line print alias" } + { "end" " >$" "multi-line first end alias" } + { "end" "hello\r\n" "multi-line last end alias" } } + + set define_cmd_alias_foo_not_inline { + { "alias foo=python" "\r\n" "multi-line alias foo" } + { "if 1" " >$" "multi-line if 1 alias foo" } + { "foo " " >$" "multi-line python command alias foo" } + { "print ('hello')" " >$" "multi-line print alias foo" } + { "end" " >$" "multi-line first end alias foo" } + { "end" "hello\r\n" "multi-line last end alias foo" } } + set define_cmd_inline { { "if 1" " >$" "inline if 1" } { "python print ('hello')" " >$" "inline python command" } { "end" "hello\r\n" "inline end" } } - foreach t [list $define_cmd_not_inline $define_cmd_inline] { + set define_cmd_alias_inline { + { "if 1" " >$" "inline if 1 alias" } + { "py print ('hello')" " >$" "inline python command alias" } + { "end" "hello\r\n" "inline end alias" } } + + set define_cmd_alias_foo_inline { + { "if 1" " >$" "inline if 1 alias foo" } + { "foo print ('hello')" " >$" "inline python command alias foo" } + { "end" "hello\r\n" "inline end alias foo" } } + + foreach t [list $define_cmd_not_inline \ + $define_cmd_alias_not_inline \ + $define_cmd_alias_foo_not_inline \ + $define_cmd_inline \ + $define_cmd_alias_inline \ + $define_cmd_alias_foo_inline] { foreach l $t { lassign $l command regex testmsg gdb_test_multiple "$command" "$testmsg" { -- 2.9.3