From: Sergio Durigan Junior <sergiodj@redhat.com>
To: Simon Marchi <simon.marchi@polymtl.ca>
Cc: GDB Patches <gdb-patches@sourceware.org>,
Jerome Guitton <guitton@adacore.com>
Subject: Re: [PATCH] PR cli/21688: Fix multi-line/inline command differentiation
Date: Thu, 29 Jun 2017 22:21:00 -0000 [thread overview]
Message-ID: <87fueil9jx.fsf@redhat.com> (raw)
In-Reply-To: <e59e76927e4c52e0c0e2685bc659dfaf@polymtl.ca> (Simon Marchi's message of "Thu, 29 Jun 2017 23:06:21 +0200")
On Thursday, June 29 2017, Simon Marchi wrote:
> On 2017-06-29 21:48, Sergio Durigan Junior wrote:
>> On Thursday, June 29 2017, Simon Marchi wrote:
>>> Another (maybe simpler) way would be to check
>>>
>>> else if (command_name_equals (cmd, "python") && *cmd_name == '\0')
>>>
>>> It's not clear when expressed like this though because cmd_name is not
>>> well named at this point (it points just after the command name).
>>
>> Hm, right. Would you prefer this way instead? I don't have a strong
>> opinion on this.
>
> My opinion is the solution with the least code is probably best, if
> they are equivalent otherwise, but I don't really mind. It's just a
> suggestion.
Right. I did some more tests here, and unfortunately your solution
doesn't work for all cases. For example, if the user puts trailing
whitespace on the command name (like "python "), *cmd_name will point to
a whitespace after the call to lookup_cmd_1.
So here's second version of the patch, with the fixes you requested
except the one above. WDYT?
--
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 5c80c97c4a11ec954cc5d426e77cf209dc43f10d Mon Sep 17 00:00:00 2001
From: Sergio Durigan Junior <sergiodj@redhat.com>
Date: Wed, 28 Jun 2017 21:55:03 -0400
Subject: [PATCH] PR cli/21688: Fix multi-line/inline command differentiation
This bug is a regression caused by the following commit:
604c4576fdcfc4e7c28f569b3748a1b6b4e0dbd4 is the first bad commit
commit 604c4576fdcfc4e7c28f569b3748a1b6b4e0dbd4
Author: Jerome Guitton <guitton@adacore.com>
Date: Tue Jan 10 15:15:53 2017 +0100
The problem happens because, on cli/cli-script.c:process_next_line,
GDB is not using the command line string to identify which command to
run, but it instead using the 'struct cmd_list_element *' that is
obtained by using the mentioned string. The problem with that is that
the 'struct cmd_list_element *' doesn't have any information on
whether the command issued by the user is a multi-line or inline one.
A multi-line command is a command that will necessarily be composed of
more than 1 line. For example:
(gdb) if 1
>python
>print ('hello')
>end
>end
As can be seen in the example above, the 'python' command actually
"opens" a new command line (represented by the change in the
indentation) that will then be used to enter Python code. OTOH, an
inline command is a command that is "self-contained" in a single line,
for example:
(gdb) if 1
>python print ('hello')
>end
This Python command is a one-liner, and therefore there is no other
Python code that can be entered for this same block. There is also no
change in the indentation.
So, the fix is somewhat simple: we have to revert the change and use
the full command line string passed to process_next_line in order to
identify whether we're dealing with a multi-line or an inline command.
This commit does just that. As can be seen, this regression also
affects other languages, like guile or the compile framework. To make
things clearer, I decided to create a new helper function responsible
for identifying a non-inline command.
Testcase is attached.
gdb/ChangeLog:
2017-06-29 Sergio Durigan Junior <sergiodj@redhat.com>
PR cli/21688
* cli/cli-script.c (command_name_equals_not_inline): New function.
(process_next_line): Adjust 'if' clauses for "python", "compile"
and "guile" to use command_name_equals_not_inline.
gdb/testsuite/ChangeLog:
2017-06-29 Sergio Durigan Junior <sergiodj@redhat.com>
PR cli/21688
* gdb.python/py-cmd.exp (test_python_inline_or_multiline): New
procedure. Call it.
---
gdb/ChangeLog | 7 +++++++
gdb/cli/cli-script.c | 21 +++++++++++++++++----
gdb/testsuite/ChangeLog | 6 ++++++
gdb/testsuite/gdb.python/py-cmd.exp | 32 ++++++++++++++++++++++++++++++++
4 files changed, 62 insertions(+), 4 deletions(-)
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b47226bc..7d0f5cf 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-06-29 Sergio Durigan Junior <sergiodj@redhat.com>
+
+ PR cli/21688
+ * cli/cli-script.c (command_name_equals_not_inline): New function.
+ (process_next_line): Adjust 'if' clauses for "python", "compile"
+ and "guile" to use command_name_equals_not_inline.
+
2017-06-29 Pedro Alves <palves@redhat.com>
* completer.c (expression_completer): Call
diff --git a/gdb/cli/cli-script.c b/gdb/cli/cli-script.c
index e0e27ef..72f316f 100644
--- a/gdb/cli/cli-script.c
+++ b/gdb/cli/cli-script.c
@@ -900,6 +900,20 @@ 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. */
@@ -997,21 +1011,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 (cmd, "python"))
+ else if (command_name_equals_not_inline (p_start, p_end, "python"))
{
/* Note that we ignore the inline "python command" form
here. */
*command = build_command_line (python_control, "");
}
- else if (command_name_equals (cmd, "compile"))
+ else if (command_name_equals_not_inline (p_start, p_end, "compile"))
{
/* 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 (cmd, "guile"))
+ else if (command_name_equals_not_inline (p_start, p_end, "guile"))
{
/* 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 41c5434..51dd86d 100644
--- a/gdb/testsuite/ChangeLog
+++ b/gdb/testsuite/ChangeLog
@@ -1,3 +1,9 @@
+2017-06-29 Sergio Durigan Junior <sergiodj@redhat.com>
+
+ PR cli/21688
+ * gdb.python/py-cmd.exp (test_python_inline_or_multiline): New
+ procedure. Call it.
+
2017-06-29 Pedro Alves <palves@redhat.com>
* gdb.base/printcmds.exp: Add tests.
diff --git a/gdb/testsuite/gdb.python/py-cmd.exp b/gdb/testsuite/gdb.python/py-cmd.exp
index 2dbf23ce..39bb785 100644
--- a/gdb/testsuite/gdb.python/py-cmd.exp
+++ b/gdb/testsuite/gdb.python/py-cmd.exp
@@ -181,6 +181,38 @@ gdb_test "complete expr_test bar\." \
"expr_test bar\.bc.*expr_test bar\.ij.*" \
"test completion through complete command"
+# Test that the "python" command is correctly recognized as
+# inline/multi-line when entering a sequence of commands.
+#
+# 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 { } {
+ set define_cmd_not_inline {
+ { "if 1" " >$" "multi-line if 1" }
+ { "python" " >$" "multi-line python command" }
+ { "print ('hello')" " >$" "multi-line print" }
+ { "end" " >$" "multi-line first end" }
+ { "end" "hello\r\n" "multi-line last end" } }
+
+ 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] {
+ foreach l $t {
+ lassign $l command regex testmsg
+ gdb_test_multiple "$command" "$testmsg" {
+ -re "$regex" {
+ pass "$testmsg"
+ }
+ }
+ }
+ }
+}
+
+test_python_inline_or_multiline
+
if { [readline_is_used] } {
set test "complete 'expr_test bar.i'"
send_gdb "expr_test bar\.i\t\t"
--
2.9.3
next prev parent reply other threads:[~2017-06-29 22:21 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-06-29 2:05 Sergio Durigan Junior
2017-06-29 12:51 ` Jerome Guitton
2017-06-29 19:08 ` Simon Marchi
2017-06-29 19:48 ` Sergio Durigan Junior
2017-06-29 21:06 ` Simon Marchi
2017-06-29 22:21 ` Sergio Durigan Junior [this message]
2017-06-30 7:01 ` Simon Marchi
2017-06-30 11:16 ` Sergio Durigan Junior
2017-06-30 11:14 ` Pedro Alves
2017-06-30 11:24 ` Sergio Durigan Junior
2017-06-30 11:30 ` Pedro Alves
2017-06-30 12:33 ` Sergio Durigan Junior
2017-06-30 12:34 ` [PATCH] PR cli/21688: Detect aliases when issuing python/compile/guile commands (and fix last commit) Sergio Durigan Junior
2017-06-30 13:02 ` Pedro Alves
2017-06-30 13:33 ` Sergio Durigan Junior
2017-06-30 13:49 ` Pedro Alves
2017-06-30 13:51 ` Sergio Durigan Junior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87fueil9jx.fsf@redhat.com \
--to=sergiodj@redhat.com \
--cc=gdb-patches@sourceware.org \
--cc=guitton@adacore.com \
--cc=simon.marchi@polymtl.ca \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).