* Re: GDB/MI and ">" prompts
[not found] ` <83mx67ikxm.fsf@gnu.org>
@ 2012-04-22 16:52 ` Eli Zaretskii
2012-04-28 14:54 ` Eli Zaretskii
0 siblings, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2012-04-22 16:52 UTC (permalink / raw)
To: brobecker; +Cc: gdb-patches
[I moved this to gdb-patches, since I propose a patch below.]
> Date: Thu, 19 Apr 2012 22:24:53 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: gdb@sourceware.org
>
> The crux of my question was why non-interactive mode does display a
> prompt while the interactive one doesn't.
To answer my own question, here's why:
char *
command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
{
...
/* Don't use fancy stuff if not talking to stdin. */
if (deprecated_readline_hook && input_from_terminal_p ())
{
rl = (*deprecated_readline_hook) (local_prompt);
}
else if (command_editing_p && input_from_terminal_p ())
{
rl = gdb_readline_wrapper (local_prompt);
}
else
{
rl = gdb_readline (local_prompt);
}
Now, the code is clear, but I cannot say I understand the logic. If
the input is from terminal, we ask (inside gdb_readline_wrapper) the
current interpreter whether to show the prompt. But if input is _not_
from terminal, we display the prompt unconditionally (inside
gdb_readline). How does this make sense?
> Date: Thu, 19 Apr 2012 11:53:29 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: gdb@sourceware.org
>
> The other thing that occured to me was that, perhaps, we should instead
> be switching the interpreter while executing the console command,
I arrived to the same conclusion, eventually. See below.
> but I doubt that would be correct.
Why not? The patch below works for me.
--- gdb/interps.c~0 2012-01-06 06:43:17.000000000 +0200
+++ gdb/interps.c 2012-04-22 08:55:27.056588400 +0300
@@ -253,6 +253,18 @@ interp_ui_out (struct interp *interp)
return current_interpreter->procs->ui_out_proc (current_interpreter);
}
+/* Temporarily overrides the current interpreter. */
+struct interp *
+interp_set_temp (const char *name)
+{
+ struct interp *interp = interp_lookup (name);
+ struct interp *old_interp = current_interpreter;
+
+ if (interp)
+ current_interpreter = interp;
+ return old_interp;
+}
+
/* Returns the interpreter's cookie. */
void *
--- gdb/interps.h~0 2012-01-06 06:43:17.000000000 +0200
+++ gdb/interps.h 2012-04-22 08:42:05.687879800 +0300
@@ -69,6 +69,7 @@
extern struct ui_out *interp_ui_out (struct interp *interp);
extern void *interp_data (struct interp *interp);
extern const char *interp_name (struct interp *interp);
+extern struct interp *interp_set_temp (const char *name);
extern int current_interp_named_p (const char *name);
extern int current_interp_display_prompt_p (void);
--- gdb/cli/cli-script.c~0 2012-01-06 06:43:32.000000000 +0200
+++ gdb/cli/cli-script.c 2012-04-22 09:04:23.533807200 +0300
@@ -1178,6 +1178,12 @@ recurse_read_control_structure (char * (
return ret;
}
+static void
+restore_interp (void *arg)
+{
+ interp_set_temp (interp_name ((struct interp *)arg));
+}
+
/* Read lines from the input stream and accumulate them in a chain of
struct command_line's, which is then returned. For input from a
terminal, the special command "end" is used to mark the end of the
@@ -1210,8 +1216,21 @@ read_command_lines (char *prompt_arg, in
}
}
- head = read_command_lines_1 (read_next_line, parse_commands,
- validator, closure);
+
+ /* Reading commands assumes the CLI behavior, so temporarily
+ override the current interpreter with CLI. */
+ if (current_interp_named_p (INTERP_CONSOLE))
+ head = read_command_lines_1 (read_next_line, parse_commands,
+ validator, closure);
+ else
+ {
+ struct interp *old_interp = interp_set_temp (INTERP_CONSOLE);
+ struct cleanup *old_chain = make_cleanup (restore_interp, old_interp);
+
+ head = read_command_lines_1 (read_next_line, parse_commands,
+ validator, closure);
+ do_cleanups (old_chain);
+ }
if (deprecated_readline_end_hook && from_tty && input_from_terminal_p ())
{
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: GDB/MI and ">" prompts
2012-04-22 16:52 ` GDB/MI and ">" prompts Eli Zaretskii
@ 2012-04-28 14:54 ` Eli Zaretskii
2012-04-30 16:26 ` Joel Brobecker
2012-04-30 16:55 ` Stan Shebs
0 siblings, 2 replies; 11+ messages in thread
From: Eli Zaretskii @ 2012-04-28 14:54 UTC (permalink / raw)
To: brobecker; +Cc: gdb-patches
Ping! Is the patch below OK for committing?
> Date: Sun, 22 Apr 2012 19:04:17 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: gdb-patches@sourceware.org
>
> [I moved this to gdb-patches, since I propose a patch below.]
>
> > Date: Thu, 19 Apr 2012 22:24:53 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: gdb@sourceware.org
> >
> > The crux of my question was why non-interactive mode does display a
> > prompt while the interactive one doesn't.
>
> To answer my own question, here's why:
>
> char *
> command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
> {
> ...
> /* Don't use fancy stuff if not talking to stdin. */
> if (deprecated_readline_hook && input_from_terminal_p ())
> {
> rl = (*deprecated_readline_hook) (local_prompt);
> }
> else if (command_editing_p && input_from_terminal_p ())
> {
> rl = gdb_readline_wrapper (local_prompt);
> }
> else
> {
> rl = gdb_readline (local_prompt);
> }
>
> Now, the code is clear, but I cannot say I understand the logic. If
> the input is from terminal, we ask (inside gdb_readline_wrapper) the
> current interpreter whether to show the prompt. But if input is _not_
> from terminal, we display the prompt unconditionally (inside
> gdb_readline). How does this make sense?
>
> > Date: Thu, 19 Apr 2012 11:53:29 -0700
> > From: Joel Brobecker <brobecker@adacore.com>
> > Cc: gdb@sourceware.org
> >
> > The other thing that occured to me was that, perhaps, we should instead
> > be switching the interpreter while executing the console command,
>
> I arrived to the same conclusion, eventually. See below.
>
> > but I doubt that would be correct.
>
> Why not? The patch below works for me.
>
> --- gdb/interps.c~0 2012-01-06 06:43:17.000000000 +0200
> +++ gdb/interps.c 2012-04-22 08:55:27.056588400 +0300
> @@ -253,6 +253,18 @@ interp_ui_out (struct interp *interp)
> return current_interpreter->procs->ui_out_proc (current_interpreter);
> }
>
> +/* Temporarily overrides the current interpreter. */
> +struct interp *
> +interp_set_temp (const char *name)
> +{
> + struct interp *interp = interp_lookup (name);
> + struct interp *old_interp = current_interpreter;
> +
> + if (interp)
> + current_interpreter = interp;
> + return old_interp;
> +}
> +
> /* Returns the interpreter's cookie. */
>
> void *
> --- gdb/interps.h~0 2012-01-06 06:43:17.000000000 +0200
> +++ gdb/interps.h 2012-04-22 08:42:05.687879800 +0300
> @@ -69,6 +69,7 @@
> extern struct ui_out *interp_ui_out (struct interp *interp);
> extern void *interp_data (struct interp *interp);
> extern const char *interp_name (struct interp *interp);
> +extern struct interp *interp_set_temp (const char *name);
>
> extern int current_interp_named_p (const char *name);
> extern int current_interp_display_prompt_p (void);
> --- gdb/cli/cli-script.c~0 2012-01-06 06:43:32.000000000 +0200
> +++ gdb/cli/cli-script.c 2012-04-22 09:04:23.533807200 +0300
> @@ -1178,6 +1178,12 @@ recurse_read_control_structure (char * (
> return ret;
> }
>
> +static void
> +restore_interp (void *arg)
> +{
> + interp_set_temp (interp_name ((struct interp *)arg));
> +}
> +
> /* Read lines from the input stream and accumulate them in a chain of
> struct command_line's, which is then returned. For input from a
> terminal, the special command "end" is used to mark the end of the
> @@ -1210,8 +1216,21 @@ read_command_lines (char *prompt_arg, in
> }
> }
>
> - head = read_command_lines_1 (read_next_line, parse_commands,
> - validator, closure);
> +
> + /* Reading commands assumes the CLI behavior, so temporarily
> + override the current interpreter with CLI. */
> + if (current_interp_named_p (INTERP_CONSOLE))
> + head = read_command_lines_1 (read_next_line, parse_commands,
> + validator, closure);
> + else
> + {
> + struct interp *old_interp = interp_set_temp (INTERP_CONSOLE);
> + struct cleanup *old_chain = make_cleanup (restore_interp, old_interp);
> +
> + head = read_command_lines_1 (read_next_line, parse_commands,
> + validator, closure);
> + do_cleanups (old_chain);
> + }
>
> if (deprecated_readline_end_hook && from_tty && input_from_terminal_p ())
> {
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: GDB/MI and ">" prompts
2012-04-28 14:54 ` Eli Zaretskii
@ 2012-04-30 16:26 ` Joel Brobecker
2012-04-30 16:55 ` Stan Shebs
1 sibling, 0 replies; 11+ messages in thread
From: Joel Brobecker @ 2012-04-30 16:26 UTC (permalink / raw)
To: Eli Zaretskii; +Cc: gdb-patches
On Sat, Apr 28, 2012 at 11:33:28AM +0300, Eli Zaretskii wrote:
>
> Ping! Is the patch below OK for committing?
I don't really know what the consequences would be to switch
temporarily to the CLI interpreter. I am hoping someone else might.
If not, there's only going to be one way to know, which it to give
it a try. Right now is a good time for this sort of attempt, as
we are still about 4 weeks away from 7.5 branching.
>
> > Date: Sun, 22 Apr 2012 19:04:17 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > Cc: gdb-patches@sourceware.org
> >
> > [I moved this to gdb-patches, since I propose a patch below.]
> >
> > > Date: Thu, 19 Apr 2012 22:24:53 +0300
> > > From: Eli Zaretskii <eliz@gnu.org>
> > > Cc: gdb@sourceware.org
> > >
> > > The crux of my question was why non-interactive mode does display a
> > > prompt while the interactive one doesn't.
> >
> > To answer my own question, here's why:
> >
> > char *
> > command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
> > {
> > ...
> > /* Don't use fancy stuff if not talking to stdin. */
> > if (deprecated_readline_hook && input_from_terminal_p ())
> > {
> > rl = (*deprecated_readline_hook) (local_prompt);
> > }
> > else if (command_editing_p && input_from_terminal_p ())
> > {
> > rl = gdb_readline_wrapper (local_prompt);
> > }
> > else
> > {
> > rl = gdb_readline (local_prompt);
> > }
> >
> > Now, the code is clear, but I cannot say I understand the logic. If
> > the input is from terminal, we ask (inside gdb_readline_wrapper) the
> > current interpreter whether to show the prompt. But if input is _not_
> > from terminal, we display the prompt unconditionally (inside
> > gdb_readline). How does this make sense?
> >
> > > Date: Thu, 19 Apr 2012 11:53:29 -0700
> > > From: Joel Brobecker <brobecker@adacore.com>
> > > Cc: gdb@sourceware.org
> > >
> > > The other thing that occured to me was that, perhaps, we should instead
> > > be switching the interpreter while executing the console command,
> >
> > I arrived to the same conclusion, eventually. See below.
> >
> > > but I doubt that would be correct.
> >
> > Why not? The patch below works for me.
> >
> > --- gdb/interps.c~0 2012-01-06 06:43:17.000000000 +0200
> > +++ gdb/interps.c 2012-04-22 08:55:27.056588400 +0300
> > @@ -253,6 +253,18 @@ interp_ui_out (struct interp *interp)
> > return current_interpreter->procs->ui_out_proc (current_interpreter);
> > }
> >
> > +/* Temporarily overrides the current interpreter. */
> > +struct interp *
> > +interp_set_temp (const char *name)
> > +{
> > + struct interp *interp = interp_lookup (name);
> > + struct interp *old_interp = current_interpreter;
> > +
> > + if (interp)
> > + current_interpreter = interp;
> > + return old_interp;
> > +}
> > +
> > /* Returns the interpreter's cookie. */
> >
> > void *
> > --- gdb/interps.h~0 2012-01-06 06:43:17.000000000 +0200
> > +++ gdb/interps.h 2012-04-22 08:42:05.687879800 +0300
> > @@ -69,6 +69,7 @@
> > extern struct ui_out *interp_ui_out (struct interp *interp);
> > extern void *interp_data (struct interp *interp);
> > extern const char *interp_name (struct interp *interp);
> > +extern struct interp *interp_set_temp (const char *name);
> >
> > extern int current_interp_named_p (const char *name);
> > extern int current_interp_display_prompt_p (void);
> > --- gdb/cli/cli-script.c~0 2012-01-06 06:43:32.000000000 +0200
> > +++ gdb/cli/cli-script.c 2012-04-22 09:04:23.533807200 +0300
> > @@ -1178,6 +1178,12 @@ recurse_read_control_structure (char * (
> > return ret;
> > }
> >
> > +static void
> > +restore_interp (void *arg)
> > +{
> > + interp_set_temp (interp_name ((struct interp *)arg));
> > +}
> > +
> > /* Read lines from the input stream and accumulate them in a chain of
> > struct command_line's, which is then returned. For input from a
> > terminal, the special command "end" is used to mark the end of the
> > @@ -1210,8 +1216,21 @@ read_command_lines (char *prompt_arg, in
> > }
> > }
> >
> > - head = read_command_lines_1 (read_next_line, parse_commands,
> > - validator, closure);
> > +
> > + /* Reading commands assumes the CLI behavior, so temporarily
> > + override the current interpreter with CLI. */
> > + if (current_interp_named_p (INTERP_CONSOLE))
> > + head = read_command_lines_1 (read_next_line, parse_commands,
> > + validator, closure);
> > + else
> > + {
> > + struct interp *old_interp = interp_set_temp (INTERP_CONSOLE);
> > + struct cleanup *old_chain = make_cleanup (restore_interp, old_interp);
> > +
> > + head = read_command_lines_1 (read_next_line, parse_commands,
> > + validator, closure);
> > + do_cleanups (old_chain);
> > + }
> >
> > if (deprecated_readline_end_hook && from_tty && input_from_terminal_p ())
> > {
> >
--
Joel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: GDB/MI and ">" prompts
2012-04-28 14:54 ` Eli Zaretskii
2012-04-30 16:26 ` Joel Brobecker
@ 2012-04-30 16:55 ` Stan Shebs
2012-04-30 17:18 ` Eli Zaretskii
2012-05-08 15:39 ` Marc Khouzam
1 sibling, 2 replies; 11+ messages in thread
From: Stan Shebs @ 2012-04-30 16:55 UTC (permalink / raw)
To: gdb-patches
On 4/28/12 1:33 AM, Eli Zaretskii wrote:
> Ping! Is the patch below OK for committing?
It seems logically correct... Did you try Eclipse? This is the kind of
thing that CDT's MI and console bits can be sensitive to, even though
it's not supposed to be. :-) If you haven't tried it, doing some
breakpoint commands (both from breakpoint window and console window)
with any recent released Eclipse should be a sufficient sniff test.
If Eclipse is good, then this is OK to commit.
Stan
>
>> Date: Sun, 22 Apr 2012 19:04:17 +0300
>> From: Eli Zaretskii<eliz@gnu.org>
>> Cc: gdb-patches@sourceware.org
>>
>> [I moved this to gdb-patches, since I propose a patch below.]
>>
>>> Date: Thu, 19 Apr 2012 22:24:53 +0300
>>> From: Eli Zaretskii<eliz@gnu.org>
>>> Cc: gdb@sourceware.org
>>>
>>> The crux of my question was why non-interactive mode does display a
>>> prompt while the interactive one doesn't.
>> To answer my own question, here's why:
>>
>> char *
>> command_line_input (char *prompt_arg, int repeat, char *annotation_suffix)
>> {
>> ...
>> /* Don't use fancy stuff if not talking to stdin. */
>> if (deprecated_readline_hook&& input_from_terminal_p ())
>> {
>> rl = (*deprecated_readline_hook) (local_prompt);
>> }
>> else if (command_editing_p&& input_from_terminal_p ())
>> {
>> rl = gdb_readline_wrapper (local_prompt);
>> }
>> else
>> {
>> rl = gdb_readline (local_prompt);
>> }
>>
>> Now, the code is clear, but I cannot say I understand the logic. If
>> the input is from terminal, we ask (inside gdb_readline_wrapper) the
>> current interpreter whether to show the prompt. But if input is _not_
>> from terminal, we display the prompt unconditionally (inside
>> gdb_readline). How does this make sense?
>>
>>> Date: Thu, 19 Apr 2012 11:53:29 -0700
>>> From: Joel Brobecker<brobecker@adacore.com>
>>> Cc: gdb@sourceware.org
>>>
>>> The other thing that occured to me was that, perhaps, we should instead
>>> be switching the interpreter while executing the console command,
>> I arrived to the same conclusion, eventually. See below.
>>
>>> but I doubt that would be correct.
>> Why not? The patch below works for me.
>>
>> --- gdb/interps.c~0 2012-01-06 06:43:17.000000000 +0200
>> +++ gdb/interps.c 2012-04-22 08:55:27.056588400 +0300
>> @@ -253,6 +253,18 @@ interp_ui_out (struct interp *interp)
>> return current_interpreter->procs->ui_out_proc (current_interpreter);
>> }
>>
>> +/* Temporarily overrides the current interpreter. */
>> +struct interp *
>> +interp_set_temp (const char *name)
>> +{
>> + struct interp *interp = interp_lookup (name);
>> + struct interp *old_interp = current_interpreter;
>> +
>> + if (interp)
>> + current_interpreter = interp;
>> + return old_interp;
>> +}
>> +
>> /* Returns the interpreter's cookie. */
>>
>> void *
>> --- gdb/interps.h~0 2012-01-06 06:43:17.000000000 +0200
>> +++ gdb/interps.h 2012-04-22 08:42:05.687879800 +0300
>> @@ -69,6 +69,7 @@
>> extern struct ui_out *interp_ui_out (struct interp *interp);
>> extern void *interp_data (struct interp *interp);
>> extern const char *interp_name (struct interp *interp);
>> +extern struct interp *interp_set_temp (const char *name);
>>
>> extern int current_interp_named_p (const char *name);
>> extern int current_interp_display_prompt_p (void);
>> --- gdb/cli/cli-script.c~0 2012-01-06 06:43:32.000000000 +0200
>> +++ gdb/cli/cli-script.c 2012-04-22 09:04:23.533807200 +0300
>> @@ -1178,6 +1178,12 @@ recurse_read_control_structure (char * (
>> return ret;
>> }
>>
>> +static void
>> +restore_interp (void *arg)
>> +{
>> + interp_set_temp (interp_name ((struct interp *)arg));
>> +}
>> +
>> /* Read lines from the input stream and accumulate them in a chain of
>> struct command_line's, which is then returned. For input from a
>> terminal, the special command "end" is used to mark the end of the
>> @@ -1210,8 +1216,21 @@ read_command_lines (char *prompt_arg, in
>> }
>> }
>>
>> - head = read_command_lines_1 (read_next_line, parse_commands,
>> - validator, closure);
>> +
>> + /* Reading commands assumes the CLI behavior, so temporarily
>> + override the current interpreter with CLI. */
>> + if (current_interp_named_p (INTERP_CONSOLE))
>> + head = read_command_lines_1 (read_next_line, parse_commands,
>> + validator, closure);
>> + else
>> + {
>> + struct interp *old_interp = interp_set_temp (INTERP_CONSOLE);
>> + struct cleanup *old_chain = make_cleanup (restore_interp, old_interp);
>> +
>> + head = read_command_lines_1 (read_next_line, parse_commands,
>> + validator, closure);
>> + do_cleanups (old_chain);
>> + }
>>
>> if (deprecated_readline_end_hook&& from_tty&& input_from_terminal_p ())
>> {
>>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: GDB/MI and ">" prompts
2012-04-30 16:55 ` Stan Shebs
@ 2012-04-30 17:18 ` Eli Zaretskii
2012-05-02 15:56 ` Marc Khouzam
2012-05-08 15:39 ` Marc Khouzam
1 sibling, 1 reply; 11+ messages in thread
From: Eli Zaretskii @ 2012-04-30 17:18 UTC (permalink / raw)
To: Stan Shebs; +Cc: gdb-patches
> Date: Mon, 30 Apr 2012 09:25:49 -0700
> From: Stan Shebs <stanshebs@earthlink.net>
>
> It seems logically correct... Did you try Eclipse? This is the kind of
> thing that CDT's MI and console bits can be sensitive to, even though
> it's not supposed to be. :-) If you haven't tried it, doing some
> breakpoint commands (both from breakpoint window and console window)
> with any recent released Eclipse should be a sufficient sniff test.
>
> If Eclipse is good, then this is OK to commit.
Thanks. No, I didn't try Eclipse, and I don't have it handy. Perhaps
someone else could.
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: GDB/MI and ">" prompts
2012-04-30 17:18 ` Eli Zaretskii
@ 2012-05-02 15:56 ` Marc Khouzam
2012-05-02 17:04 ` Eli Zaretskii
0 siblings, 1 reply; 11+ messages in thread
From: Marc Khouzam @ 2012-05-02 15:56 UTC (permalink / raw)
To: 'Eli Zaretskii', 'Stan Shebs'
Cc: 'gdb-patches@sourceware.org'
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Eli Zaretskii
> Sent: Monday, April 30, 2012 1:09 PM
> To: Stan Shebs
> Cc: gdb-patches@sourceware.org
> Subject: Re: GDB/MI and ">" prompts
>
> > Date: Mon, 30 Apr 2012 09:25:49 -0700
> > From: Stan Shebs <stanshebs@earthlink.net>
> >
> > It seems logically correct... Did you try Eclipse? This is
> the kind of
> > thing that CDT's MI and console bits can be sensitive to,
> even though
> > it's not supposed to be. :-) If you haven't tried it, doing some
> > breakpoint commands (both from breakpoint window and
> console window)
> > with any recent released Eclipse should be a sufficient sniff test.
> >
> > If Eclipse is good, then this is OK to commit.
>
> Thanks. No, I didn't try Eclipse, and I don't have it handy. Perhaps
> someone else could.
I actually had to add special code in Eclipse to handle some secondary
prompt weirdness of GDB, so I think it is worth trying your patch
with Eclipse.
I don't have time right now, but I should this week.
Feel free to ping me in a couple of days if I haven't replied yet.
Marc
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: GDB/MI and ">" prompts
2012-05-02 15:56 ` Marc Khouzam
@ 2012-05-02 17:04 ` Eli Zaretskii
0 siblings, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2012-05-02 17:04 UTC (permalink / raw)
To: Marc Khouzam; +Cc: stanshebs, gdb-patches
> From: Marc Khouzam <marc.khouzam@ericsson.com>
> CC: "'gdb-patches@sourceware.org'" <gdb-patches@sourceware.org>
> Date: Wed, 2 May 2012 11:55:43 -0400
>
> I actually had to add special code in Eclipse to handle some secondary
> prompt weirdness of GDB, so I think it is worth trying your patch
> with Eclipse.
>
> I don't have time right now, but I should this week.
Thank you very much!
^ permalink raw reply [flat|nested] 11+ messages in thread
* RE: GDB/MI and ">" prompts
2012-04-30 16:55 ` Stan Shebs
2012-04-30 17:18 ` Eli Zaretskii
@ 2012-05-08 15:39 ` Marc Khouzam
2012-05-08 15:51 ` Joel Brobecker
2012-05-08 18:51 ` Eli Zaretskii
1 sibling, 2 replies; 11+ messages in thread
From: Marc Khouzam @ 2012-05-08 15:39 UTC (permalink / raw)
To: 'Stan Shebs', 'gdb-patches@sourceware.org',
'Eli Zaretskii'
> -----Original Message-----
> From: gdb-patches-owner@sourceware.org
> [mailto:gdb-patches-owner@sourceware.org] On Behalf Of Stan Shebs
> Sent: Monday, April 30, 2012 12:26 PM
> To: gdb-patches@sourceware.org
> Subject: Re: GDB/MI and ">" prompts
>
> On 4/28/12 1:33 AM, Eli Zaretskii wrote:
> > Ping! Is the patch below OK for committing?
>
> It seems logically correct... Did you try Eclipse? This is
> the kind of
> thing that CDT's MI and console bits can be sensitive to, even though
> it's not supposed to be. :-) If you haven't tried it, doing some
> breakpoint commands (both from breakpoint window and console window)
> with any recent released Eclipse should be a sufficient sniff test.
>
> If Eclipse is good, then this is OK to commit.
I tried Eli's patch with Eclipse and I didn't see any problems.
I was concerned that changing to the CLI interpreter for a while
could make us lose some MI events, but it does not seem to be
the case. For example, I tried running a thread that will hit
a breakpoint in 5 seconds; I then got into the 'commands' secondary
prompt and waited for the 5 seconds to pass. The *stopped event
was not reported until I sent the 'end' command and got out
of the secondary prompt. This is the behavior with and without
Eli's patch. So it seems MI events are buffered in that case,
and switching to the CLI interpreter does not make use lose that
event.
So, it seems ok for an Eclipse perspective.
marc
>
> Stan
>
> >
> >> Date: Sun, 22 Apr 2012 19:04:17 +0300
> >> From: Eli Zaretskii<eliz@gnu.org>
> >> Cc: gdb-patches@sourceware.org
> >>
> >> [I moved this to gdb-patches, since I propose a patch below.]
> >>
> >>> Date: Thu, 19 Apr 2012 22:24:53 +0300
> >>> From: Eli Zaretskii<eliz@gnu.org>
> >>> Cc: gdb@sourceware.org
> >>>
> >>> The crux of my question was why non-interactive mode does
> display a
> >>> prompt while the interactive one doesn't.
> >> To answer my own question, here's why:
> >>
> >> char *
> >> command_line_input (char *prompt_arg, int repeat, char
> *annotation_suffix)
> >> {
> >> ...
> >> /* Don't use fancy stuff if not talking to stdin. */
> >> if (deprecated_readline_hook&& input_from_terminal_p ())
> >> {
> >> rl = (*deprecated_readline_hook) (local_prompt);
> >> }
> >> else if (command_editing_p&& input_from_terminal_p ())
> >> {
> >> rl = gdb_readline_wrapper (local_prompt);
> >> }
> >> else
> >> {
> >> rl = gdb_readline (local_prompt);
> >> }
> >>
> >> Now, the code is clear, but I cannot say I understand the
> logic. If
> >> the input is from terminal, we ask (inside
> gdb_readline_wrapper) the
> >> current interpreter whether to show the prompt. But if
> input is _not_
> >> from terminal, we display the prompt unconditionally (inside
> >> gdb_readline). How does this make sense?
> >>
> >>> Date: Thu, 19 Apr 2012 11:53:29 -0700
> >>> From: Joel Brobecker<brobecker@adacore.com>
> >>> Cc: gdb@sourceware.org
> >>>
> >>> The other thing that occured to me was that, perhaps, we
> should instead
> >>> be switching the interpreter while executing the console command,
> >> I arrived to the same conclusion, eventually. See below.
> >>
> >>> but I doubt that would be correct.
> >> Why not? The patch below works for me.
> >>
> >> --- gdb/interps.c~0 2012-01-06 06:43:17.000000000 +0200
> >> +++ gdb/interps.c 2012-04-22 08:55:27.056588400 +0300
> >> @@ -253,6 +253,18 @@ interp_ui_out (struct interp *interp)
> >> return current_interpreter->procs->ui_out_proc
> (current_interpreter);
> >> }
> >>
> >> +/* Temporarily overrides the current interpreter. */
> >> +struct interp *
> >> +interp_set_temp (const char *name)
> >> +{
> >> + struct interp *interp = interp_lookup (name);
> >> + struct interp *old_interp = current_interpreter;
> >> +
> >> + if (interp)
> >> + current_interpreter = interp;
> >> + return old_interp;
> >> +}
> >> +
> >> /* Returns the interpreter's cookie. */
> >>
> >> void *
> >> --- gdb/interps.h~0 2012-01-06 06:43:17.000000000 +0200
> >> +++ gdb/interps.h 2012-04-22 08:42:05.687879800 +0300
> >> @@ -69,6 +69,7 @@
> >> extern struct ui_out *interp_ui_out (struct interp *interp);
> >> extern void *interp_data (struct interp *interp);
> >> extern const char *interp_name (struct interp *interp);
> >> +extern struct interp *interp_set_temp (const char *name);
> >>
> >> extern int current_interp_named_p (const char *name);
> >> extern int current_interp_display_prompt_p (void);
> >> --- gdb/cli/cli-script.c~0 2012-01-06 06:43:32.000000000 +0200
> >> +++ gdb/cli/cli-script.c 2012-04-22 09:04:23.533807200 +0300
> >> @@ -1178,6 +1178,12 @@ recurse_read_control_structure (char * (
> >> return ret;
> >> }
> >>
> >> +static void
> >> +restore_interp (void *arg)
> >> +{
> >> + interp_set_temp (interp_name ((struct interp *)arg));
> >> +}
> >> +
> >> /* Read lines from the input stream and accumulate them
> in a chain of
> >> struct command_line's, which is then returned. For
> input from a
> >> terminal, the special command "end" is used to mark
> the end of the
> >> @@ -1210,8 +1216,21 @@ read_command_lines (char *prompt_arg, in
> >> }
> >> }
> >>
> >> - head = read_command_lines_1 (read_next_line, parse_commands,
> >> - validator, closure);
> >> +
> >> + /* Reading commands assumes the CLI behavior, so temporarily
> >> + override the current interpreter with CLI. */
> >> + if (current_interp_named_p (INTERP_CONSOLE))
> >> + head = read_command_lines_1 (read_next_line, parse_commands,
> >> + validator, closure);
> >> + else
> >> + {
> >> + struct interp *old_interp = interp_set_temp
> (INTERP_CONSOLE);
> >> + struct cleanup *old_chain = make_cleanup
> (restore_interp, old_interp);
> >> +
> >> + head = read_command_lines_1 (read_next_line, parse_commands,
> >> + validator, closure);
> >> + do_cleanups (old_chain);
> >> + }
> >>
> >> if (deprecated_readline_end_hook&& from_tty&&
> input_from_terminal_p ())
> >> {
> >>
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: GDB/MI and ">" prompts
2012-05-08 15:39 ` Marc Khouzam
@ 2012-05-08 15:51 ` Joel Brobecker
2012-05-08 18:51 ` Eli Zaretskii
2012-05-08 18:51 ` Eli Zaretskii
1 sibling, 1 reply; 11+ messages in thread
From: Joel Brobecker @ 2012-05-08 15:51 UTC (permalink / raw)
To: Marc Khouzam
Cc: 'Stan Shebs', 'gdb-patches@sourceware.org',
'Eli Zaretskii'
Thanks for doing the testing, Mark!
Stan said:
> > If Eclipse is good, then this is OK to commit.
Mark reported:
> So, it seems ok for an Eclipse perspective.
So it seems Eli is good to go!
--
Joel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: GDB/MI and ">" prompts
2012-05-08 15:51 ` Joel Brobecker
@ 2012-05-08 18:51 ` Eli Zaretskii
0 siblings, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2012-05-08 18:51 UTC (permalink / raw)
To: Joel Brobecker; +Cc: marc.khouzam, stanshebs, gdb-patches
> Date: Tue, 8 May 2012 08:51:23 -0700
> From: Joel Brobecker <brobecker@adacore.com>
> Cc: 'Stan Shebs' <stanshebs@earthlink.net>,
> "'gdb-patches@sourceware.org'" <gdb-patches@sourceware.org>,
> 'Eli Zaretskii' <eliz@gnu.org>
>
> Thanks for doing the testing, Mark!
>
> Stan said:
> > > If Eclipse is good, then this is OK to commit.
>
> Mark reported:
> > So, it seems ok for an Eclipse perspective.
>
> So it seems Eli is good to go!
Thanks. Here's what I actually committed:
Index: gdb/ChangeLog
===================================================================
RCS file: /cvs/src/src/gdb/ChangeLog,v
retrieving revision 1.14219
diff -u -p -r1.14219 ChangeLog
--- gdb/ChangeLog 8 May 2012 14:07:06 -0000 1.14219
+++ gdb/ChangeLog 8 May 2012 18:48:12 -0000
@@ -1,3 +1,16 @@
+2012-05-08 Eli Zaretskii <eliz@gnu.org>
+
+ Display the ">" prompt in interactive mode while reading canned
+ commands, even when the current interpreter is MI.
+
+ * interps.c (interp_set_temp): New function.
+
+ * interps.h (interp_set_temp): Add prototype.
+
+ * cli/cli-script.c (restore_interp): New cleanup function.
+ (read_command_lines): Temporarily override the current interpreter
+ with CLI and arrange for restoring the original one.
+
2012-05-12 Joel Sherrill <joel.sherrill@oarcorp.com>
* microblaze-rom.c (_initialize_picobug_rom): Add prototype.
Index: gdb/interps.c
===================================================================
RCS file: /cvs/src/src/gdb/interps.c,v
retrieving revision 1.44
diff -u -p -r1.44 interps.c
--- gdb/interps.c 4 Jan 2012 08:17:05 -0000 1.44
+++ gdb/interps.c 8 May 2012 18:48:12 -0000
@@ -253,6 +253,18 @@ interp_ui_out (struct interp *interp)
return current_interpreter->procs->ui_out_proc (current_interpreter);
}
+/* Temporarily overrides the current interpreter. */
+struct interp *
+interp_set_temp (const char *name)
+{
+ struct interp *interp = interp_lookup (name);
+ struct interp *old_interp = current_interpreter;
+
+ if (interp)
+ current_interpreter = interp;
+ return old_interp;
+}
+
/* Returns the interpreter's cookie. */
void *
Index: gdb/interps.h
===================================================================
RCS file: /cvs/src/src/gdb/interps.h,v
retrieving revision 1.22
diff -u -p -r1.22 interps.h
--- gdb/interps.h 4 Jan 2012 08:17:05 -0000 1.22
+++ gdb/interps.h 8 May 2012 18:48:12 -0000
@@ -69,6 +69,7 @@ extern struct interp *interp_lookup (con
extern struct ui_out *interp_ui_out (struct interp *interp);
extern void *interp_data (struct interp *interp);
extern const char *interp_name (struct interp *interp);
+extern struct interp *interp_set_temp (const char *name);
extern int current_interp_named_p (const char *name);
extern int current_interp_display_prompt_p (void);
Index: gdb/cli/cli-script.c
===================================================================
RCS file: /cvs/src/src/gdb/cli/cli-script.c,v
retrieving revision 1.74
diff -u -p -r1.74 cli-script.c
--- gdb/cli/cli-script.c 24 Jan 2012 20:56:33 -0000 1.74
+++ gdb/cli/cli-script.c 8 May 2012 18:48:12 -0000
@@ -1178,6 +1178,12 @@ recurse_read_control_structure (char * (
return ret;
}
+static void
+restore_interp (void *arg)
+{
+ interp_set_temp (interp_name ((struct interp *)arg));
+}
+
/* Read lines from the input stream and accumulate them in a chain of
struct command_line's, which is then returned. For input from a
terminal, the special command "end" is used to mark the end of the
@@ -1210,8 +1216,21 @@ read_command_lines (char *prompt_arg, in
}
}
- head = read_command_lines_1 (read_next_line, parse_commands,
- validator, closure);
+
+ /* Reading commands assumes the CLI behavior, so temporarily
+ override the current interpreter with CLI. */
+ if (current_interp_named_p (INTERP_CONSOLE))
+ head = read_command_lines_1 (read_next_line, parse_commands,
+ validator, closure);
+ else
+ {
+ struct interp *old_interp = interp_set_temp (INTERP_CONSOLE);
+ struct cleanup *old_chain = make_cleanup (restore_interp, old_interp);
+
+ head = read_command_lines_1 (read_next_line, parse_commands,
+ validator, closure);
+ do_cleanups (old_chain);
+ }
if (deprecated_readline_end_hook && from_tty && input_from_terminal_p ())
{
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: GDB/MI and ">" prompts
2012-05-08 15:39 ` Marc Khouzam
2012-05-08 15:51 ` Joel Brobecker
@ 2012-05-08 18:51 ` Eli Zaretskii
1 sibling, 0 replies; 11+ messages in thread
From: Eli Zaretskii @ 2012-05-08 18:51 UTC (permalink / raw)
To: Marc Khouzam; +Cc: stanshebs, gdb-patches
> From: Marc Khouzam <marc.khouzam@ericsson.com>
> Date: Tue, 8 May 2012 11:39:25 -0400
>
> I tried Eli's patch with Eclipse and I didn't see any problems.
>
> I was concerned that changing to the CLI interpreter for a while
> could make us lose some MI events, but it does not seem to be
> the case. For example, I tried running a thread that will hit
> a breakpoint in 5 seconds; I then got into the 'commands' secondary
> prompt and waited for the 5 seconds to pass. The *stopped event
> was not reported until I sent the 'end' command and got out
> of the secondary prompt. This is the behavior with and without
> Eli's patch. So it seems MI events are buffered in that case,
> and switching to the CLI interpreter does not make use lose that
> event.
>
> So, it seems ok for an Eclipse perspective.
Thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2012-05-08 18:51 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <83vckviv3b.fsf@gnu.org>
[not found] ` <20120419154853.GM25623@adacore.com>
[not found] ` <83sjfzitxx.fsf@gnu.org>
[not found] ` <83r4vjitnj.fsf@gnu.org>
[not found] ` <20120419185329.GO25623@adacore.com>
[not found] ` <83mx67ikxm.fsf@gnu.org>
2012-04-22 16:52 ` GDB/MI and ">" prompts Eli Zaretskii
2012-04-28 14:54 ` Eli Zaretskii
2012-04-30 16:26 ` Joel Brobecker
2012-04-30 16:55 ` Stan Shebs
2012-04-30 17:18 ` Eli Zaretskii
2012-05-02 15:56 ` Marc Khouzam
2012-05-02 17:04 ` Eli Zaretskii
2012-05-08 15:39 ` Marc Khouzam
2012-05-08 15:51 ` Joel Brobecker
2012-05-08 18:51 ` Eli Zaretskii
2012-05-08 18:51 ` Eli Zaretskii
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox