From patchwork Fri Jun 30 13:33:28 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Durigan Junior X-Patchwork-Id: 21366 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-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 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 09:33:28 -0400 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 X-IsSubscribed: yes 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. 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" {