gdb patch to suppress empty lines, re-visited

gdb patch to suppress empty lines, re-visited

[Per Bothner]

[-- Attachment #1: Type: text/plain, Size: 353 bytes --]

This is a revision of a patch originally from 1999:
http://sources.redhat.com/ml/gdb-patches/1999-q2/msg00093.html

I'm not sure if the patch in top.c does anything, since I don't
know when/if readline is called from command_line_input.

OK to check in? With or without the top.c change?
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

[-- Attachment #2: gdb-erase-empty-line.patch --]
[-- Type: text/plain, Size: 1486 bytes --]

Index: top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.68
diff -u -p -r1.68 top.c
--- top.c	26 Sep 2002 17:46:04 -0000	1.68
+++ top.c	7 Dec 2002 06:21:11 -0000
@@ -1211,7 +1211,12 @@ command_line_input (char *prompt_arg, in
 	}
       else if (command_editing_p && instream == stdin && ISATTY (instream))
 	{
+	  int can_repeat = repeat && *line != NULL;
+	  if (can_repeat)
+	    rl_erase_empty_line++;
 	  rl = gdb_readline_wrapper (local_prompt);
+	  if (can_repeat)
+	    rl_erase_empty_line--;
 	}
       else
 	{
Index: event-top.c
===================================================================
RCS file: /cvs/src/src/gdb/event-top.c,v
retrieving revision 1.24
diff -u -p -r1.24 event-top.c
--- event-top.c	5 Nov 2002 22:38:11 -0000	1.24
+++ event-top.c	7 Dec 2002 06:21:12 -0000
@@ -191,6 +191,7 @@ cli_command_loop (void)
       strcpy (a_prompt, PREFIX (0));
       strcat (a_prompt, gdb_prompt);
       strcat (a_prompt, SUFFIX (0));
+      rl_erase_empty_line = *line != NULL;
       rl_callback_handler_install (a_prompt, input_handler);
     }
   else
@@ -296,6 +297,7 @@ display_gdb_prompt (char *new_prompt)
   if (async_command_editing_p)
     {
       rl_callback_handler_remove ();
+      rl_erase_empty_line = *line != NULL;
       rl_callback_handler_install (new_prompt, input_handler);
     }
   /* new_prompt at this point can be the top of the stack or the one passed in */

Re: gdb patch to suppress empty lines, re-visited

[Michael Snyder]

Per Bothner wrote:
> 
> This is a revision of a patch originally from 1999:
> http://sources.redhat.com/ml/gdb-patches/1999-q2/msg00093.html
> 
> I'm not sure if the patch in top.c does anything, since I don't
> know when/if readline is called from command_line_input.
> 
> OK to check in? With or without the top.c change?
> --
>         --Per Bothner

For posterity, this changes the console output when you are
repeating a command a bunch of times by hitting return.
It changes, eg.
   1038 <main+8>:       call  0x138818 <get_run_time>
   (gdb) 
   0xe103c <main+12>:      st  %i1, [ %fp + 0x48 ]
   (gdb) 
   0xe1040 <main+16>:      st  %o0, [ %fp + -464 ]
   (gdb) 

to

   (gdb) x/i $pc
   0xe1038 <main+8>:       call  0x138818 <get_run_time>
   0xe103c <main+12>:      st  %i1, [ %fp + 0x48 ]
   0xe1040 <main+16>:      st  %o0, [ %fp + -464 ]
   (gdb)

Per, I think the first discussion needs to be, 
do we agree that we want this change?  This changes
visible behavior, quite broadly.

Michael

> per@bothner.com   http://www.bothner.com/per/
> 
>   -------------------------------------------------------------------------------
> Index: top.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/top.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 top.c
> --- top.c       26 Sep 2002 17:46:04 -0000      1.68
> +++ top.c       7 Dec 2002 06:21:11 -0000
> @@ -1211,7 +1211,12 @@ command_line_input (char *prompt_arg, in
>         }
>        else if (command_editing_p && instream == stdin && ISATTY (instream))
>         {
> +         int can_repeat = repeat && *line != NULL;
> +         if (can_repeat)
> +           rl_erase_empty_line++;
>           rl = gdb_readline_wrapper (local_prompt);
> +         if (can_repeat)
> +           rl_erase_empty_line--;
>         }
>        else
>         {
> Index: event-top.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/event-top.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 event-top.c
> --- event-top.c 5 Nov 2002 22:38:11 -0000       1.24
> +++ event-top.c 7 Dec 2002 06:21:12 -0000
> @@ -191,6 +191,7 @@ cli_command_loop (void)
>        strcpy (a_prompt, PREFIX (0));
>        strcat (a_prompt, gdb_prompt);
>        strcat (a_prompt, SUFFIX (0));
> +      rl_erase_empty_line = *line != NULL;
>        rl_callback_handler_install (a_prompt, input_handler);
>      }
>    else
> @@ -296,6 +297,7 @@ display_gdb_prompt (char *new_prompt)
>    if (async_command_editing_p)
>      {
>        rl_callback_handler_remove ();
> +      rl_erase_empty_line = *line != NULL;
>        rl_callback_handler_install (new_prompt, input_handler);
>      }
>    /* new_prompt at this point can be the top of the stack or the one passed in */

Re: gdb patch to suppress empty lines, re-visited

[Per Bothner]

Michael Snyder wrote:
> For posterity, this changes the console output when you are
> repeating a command a bunch of times by hitting return.

Yes, that is the point.  Perhaps I should have repeated that.

> Per, I think the first discussion needs to be, 
> do we agree that we want this change?  This changes
> visible behavior, quite broadly.

Of course.  I should have made that more explicit.
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: gdb patch to suppress empty lines, re-visited

[Daniel Jacobowitz]

On Wed, Dec 11, 2002 at 03:27:59PM -0800, Michael Snyder wrote:
> Per Bothner wrote:
> > 
> > This is a revision of a patch originally from 1999:
> > http://sources.redhat.com/ml/gdb-patches/1999-q2/msg00093.html
> > 
> > I'm not sure if the patch in top.c does anything, since I don't
> > know when/if readline is called from command_line_input.
> > 
> > OK to check in? With or without the top.c change?
> > --
> >         --Per Bothner
> 
> For posterity, this changes the console output when you are
> repeating a command a bunch of times by hitting return.
> It changes, eg.
>    1038 <main+8>:       call  0x138818 <get_run_time>
>    (gdb) 
>    0xe103c <main+12>:      st  %i1, [ %fp + 0x48 ]
>    (gdb) 
>    0xe1040 <main+16>:      st  %o0, [ %fp + -464 ]
>    (gdb) 
> 
> to
> 
>    (gdb) x/i $pc
>    0xe1038 <main+8>:       call  0x138818 <get_run_time>
>    0xe103c <main+12>:      st  %i1, [ %fp + 0x48 ]
>    0xe1040 <main+16>:      st  %o0, [ %fp + -464 ]
>    (gdb)
> 
> Per, I think the first discussion needs to be, 
> do we agree that we want this change?  This changes
> visible behavior, quite broadly.

I'll put in my vote for "yes, or at least as an option".

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: gdb patch to suppress empty lines, re-visited

[Per Bothner]

A few people have expresses themselves in favor, and none
have been opposed.  I don't know if that counts as a consensus ...
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: gdb patch to suppress empty lines, re-visited

[Andrew Cagney]

> A few people have expresses themselves in favor, and none
> have been opposed.  I don't know if that counts as a consensus ...

Personally, I don't like it :-)  At least not as a default.

Andrew

Re: gdb patch to suppress empty lines, re-visited

[Kevin Buettner]

On Dec 13,  3:34pm, Andrew Cagney wrote:

> > A few people have expresses themselves in favor, and none
> > have been opposed.  I don't know if that counts as a consensus ...
> 
> Personally, I don't like it :-)  At least not as a default.

I wouldn't want it as the default either.  I think it'd be great as
an option though.

Kevin

Re: gdb patch to suppress empty lines, re-visited

[Daniel Jacobowitz]

On Fri, Dec 13, 2002 at 03:34:48PM -0500, Andrew Cagney wrote:
> >A few people have expresses themselves in favor, and none
> >have been opposed.  I don't know if that counts as a consensus ...
> 
> Personally, I don't like it :-)  At least not as a default.

That's fine by me :)  I'll turn it on in gdbinit.  Per, could I talk
you into a patch with a 'set' option?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: gdb patch to suppress empty lines, re-visited

[Per Bothner]

Andrew Cagney wrote:

> Personally, I don't like it :-)

Any particular reason?  (Except that it's different from what
we're used to?)

Try it - you might like it!

> At least not as a default.

I guess we can add it as a non-default to let people try it out
without forcing them to.  But long-term, I dislike too many
options, and prefer picking the best defaults.  And obviously
I think this makes for a nicer interface.

As I said:  Try it in your tree.  It's a simple, localized patch.
It can of course be modified to only work where it makes most
sense.  Candidates include list, print, and [e]xamine.  The step
commands also work nicely with this feature.
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: gdb patch to suppress empty lines, re-visited

[David Carlton]

On Fri, 13 Dec 2002 11:30:55 -0800, Per Bothner <per@bothner.com> said:

> A few people have expresses themselves in favor, and none
> have been opposed.  I don't know if that counts as a consensus ...

I don't know if I like it or not, and I don't think I'll know until
I've actually tried it.  My first reaction is slightly negative, and
I'm not at all sure how well it will work with XEmacs's GDB mode: if I
do a bunch of steps through a function, I get output like this:

(gdb) s
(gdb) 
(gdb) 
(gdb) 
(gdb) 
(gdb) 
(gdb) 

because the mode isn't showing me the source code information in that
buffer (since it's available in another buffer), and I can't imagine
your patch doing anything good to that output.  (I have no idea what
GNU Emacs's GDB mode looks like: for some reason, XEmacs uses a
different (older?) one.)  So if your patch would change the output in
this situation, I'm dubious.  But maybe it wouldn't; I'm not sure if
the ISATTY (instream) guard would protect against this situation.

I guess if it doesn't affect the output under (X)Emacs then I don't
care one way or another because I'll never see its results.

David Carlton
carlton@math.stanford.edu

Re: gdb patch to suppress empty lines, re-visited

[Per Bothner]

David Carlton wrote:

> I don't know if I like it or not, and I don't think I'll know until
> I've actually tried it.  My first reaction is slightly negative, and
> I'm not at all sure how well it will work with XEmacs's GDB mode: if I
> do a bunch of steps through a function, I get output like this:
>
> (gdb) s
> (gdb)
> (gdb)
> (gdb)
> (gdb)
> (gdb)
> (gdb)
>
> because the mode isn't showing me the source code information in that
> buffer (since it's available in another buffer), and I can't imagine
> your patch doing anything good to that output.

But it doesn't do anything bad either.  My patch just sets a readline
flag, and my understanding is that Emacs gdb mode does not use readline.
(Exception: if you use the unmaintained tgud.el, which uses term.el's
terminal emulator.  I'd be surprised if anybody even knows about it!)

> (I have no idea what
> GNU Emacs's GDB mode looks like: for some reason, XEmacs uses a
> different (older?) one.)  So if your patch would change the output in
> this situation, I'm dubious.  But maybe it wouldn't; I'm not sure if
> the ISATTY (instream) guard would protect against this situation.


It should.
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: gdb patch to suppress empty lines, re-visited

[Michael Elizabeth Chastain]

I've often wished for this feature as a user of gdb.

My vote: yes, provided that it doesn't systemically hose the test
suite.  There are some tests that explicitly check that repeated
commands work and it's fine to update those.  But I am gun shy of
readline and its mysterious refresh algorithms now and I don't want
gdb to start issuing "(gdb) ^M(gdb) p^M(gdb) pr^M(gdb) pri..."
style output.

Michael C