RE: GDB/MI and ">" prompts

[Marc Khouzam <marc.khouzam@ericsson.com>]

> -----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 ())
> >>       {
> >>
> 
>