Still some style problems with 8.3 branch

Still some style problems with 8.3 branch

[Philippe Waroquiers]

I just encountered a styling problem with the 8.3 branch
(also on the 8.4).
Test executed with GNU gdb (GDB) 8.2.91.20190330-git

To reproduce:
  ./gdb/gdb ./gdb/gdb
  info sources
and then type return successively, to see each page.
After the initial page, the first line contains a file that
is partially printed in green color, partially printed in black color,
e.g., in the below given output.
The line that starts with '--Type <RET>' is in black, as expected.
Then the next line starts in green as expected.
But only '/bd/home/ph' is in green.
We then have 'ilippe/gdb/git/relbranch/gdb/minsyms.h,' in black.
Then all is normal till the next page.

So, that seems to point at some remaining problem in the
buffering/output of style/color codes.

Philippe


...
/usr/include/regex.h, /bd/home/philippe/gdb/git/relbranch/gdb/gdb_regex.h, /bd/home/philippe/gdb/git/relbranch/gdb/command.h, 
/bd/home/philippe/gdb/git/relbranch/gdb/cli/cli-decode.h, /bd/home/philippe/gdb/git/relbranch/gdb/completer.h, /bd/home/philippe/gdb/git/relbranch/gdb/block.h, 
--Type <RET> for more, q to quit, c to continue without paging--
/bd/home/philippe/gdb/git/relbranch/gdb/minsyms.h, /bd/home/philippe/gdb/git/relbranch/gdb/ax.h, /bd/home/philippe/gdb/git/relbranch/gdb/probe.h, 
/bd/home/philippe/gdb/git/relbranch/gdb/language.h, /bd/home/philippe/gdb/git/relbranch/gdb/expression.h, /bd/home/philippe/gdb/git/relbranch/gdb/frame.h, 
...

Re: Still some style problems with 8.3 branch

[Eli Zaretskii]

> From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> Date: Sat, 30 Mar 2019 11:36:51 +0100
> 
> I just encountered a styling problem with the 8.3 branch
> (also on the 8.4).
> Test executed with GNU gdb (GDB) 8.2.91.20190330-git
> 
> To reproduce:
>   ./gdb/gdb ./gdb/gdb
>   info sources
> and then type return successively, to see each page.
> After the initial page, the first line contains a file that
> is partially printed in green color, partially printed in black color,
> e.g., in the below given output.
> The line that starts with '--Type <RET>' is in black, as expected.
> Then the next line starts in green as expected.
> But only '/bd/home/ph' is in green.
> We then have 'ilippe/gdb/git/relbranch/gdb/minsyms.h,' in black.
> Then all is normal till the next page.
> 
> So, that seems to point at some remaining problem in the
> buffering/output of style/color codes.

I don't think it's due to buffering.  If I #ifdef away this part near
the end of prompt_for_continue:

  /* Restore the current styling.  */
  if (can_emit_style_escape (gdb_stdout))
    emit_style_escape (applied_style);

then the problem seems to go away.  So I think this style setting is
what causes the problem, probably because applied_style has the wrong
value here?  The incorrect style seems to start precisely from the
first character that exceeds the last column of a line, so I think
it's indeed due to the above snippet.

Tom, can you look into this, please?

Re: Still some style problems with 8.3 branch

[Eli Zaretskii]

Ping!

> Date: Sun, 31 Mar 2019 17:28:52 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> CC: gdb-patches@sourceware.org
> 
> > From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> > Date: Sat, 30 Mar 2019 11:36:51 +0100
> > 
> > I just encountered a styling problem with the 8.3 branch
> > (also on the 8.4).
> > Test executed with GNU gdb (GDB) 8.2.91.20190330-git
> > 
> > To reproduce:
> >   ./gdb/gdb ./gdb/gdb
> >   info sources
> > and then type return successively, to see each page.
> > After the initial page, the first line contains a file that
> > is partially printed in green color, partially printed in black color,
> > e.g., in the below given output.
> > The line that starts with '--Type <RET>' is in black, as expected.
> > Then the next line starts in green as expected.
> > But only '/bd/home/ph' is in green.
> > We then have 'ilippe/gdb/git/relbranch/gdb/minsyms.h,' in black.
> > Then all is normal till the next page.
> > 
> > So, that seems to point at some remaining problem in the
> > buffering/output of style/color codes.
> 
> I don't think it's due to buffering.  If I #ifdef away this part near
> the end of prompt_for_continue:
> 
>   /* Restore the current styling.  */
>   if (can_emit_style_escape (gdb_stdout))
>     emit_style_escape (applied_style);
> 
> then the problem seems to go away.  So I think this style setting is
> what causes the problem, probably because applied_style has the wrong
> value here?  The incorrect style seems to start precisely from the
> first character that exceeds the last column of a line, so I think
> it's indeed due to the above snippet.
> 
> Tom, can you look into this, please?
> 

Re: Still some style problems with 8.3 branch

[Tom Tromey]

Eli> Ping!

I hope to get to it this week.

Tom

Re: Still some style problems with 8.3 branch

[Philippe Waroquiers]

I have just done some tests, comparing the behaviour
of the 8.3 branch with and without the patch suggested by Eli.

It works better with the patch, but even with the patch,
I still encountered one case where the style was not
the expected style after the continuation prompt.

To reproduce (in a 80x24 terminal):

./gdb/gdb ./gdb/gdb
break cli_ui_out::do_field_string
run gdb/testsuite/outputs/gdb.ada/info_auto_lang/proc_in_ada
  (gdb) b proc_in_c
  (gdb) run
info func error
#### then type several times return at the page prompt.
#### At some point in time, you see

0x00007ffff691d360  transaction clone for std::overflow_error::overflow_error(st
--Type <RET> for more, q to quit, c to continue without paging--
d::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)

And the line after the prompt is unexpectedly in black.
This is both with and without the patch.

Philippe


On Wed, 2019-04-03 at 21:21 +0300, Eli Zaretskii wrote:
> Ping!
> 
> > Date: Sun, 31 Mar 2019 17:28:52 +0300
> > From: Eli Zaretskii <eliz@gnu.org>
> > CC: gdb-patches@sourceware.org
> > 
> > > From: Philippe Waroquiers <philippe.waroquiers@skynet.be>
> > > Date: Sat, 30 Mar 2019 11:36:51 +0100
> > > 
> > > I just encountered a styling problem with the 8.3 branch
> > > (also on the 8.4).
> > > Test executed with GNU gdb (GDB) 8.2.91.20190330-git
> > > 
> > > To reproduce:
> > >   ./gdb/gdb ./gdb/gdb
> > >   info sources
> > > and then type return successively, to see each page.
> > > After the initial page, the first line contains a file that
> > > is partially printed in green color, partially printed in black color,
> > > e.g., in the below given output.
> > > The line that starts with '--Type <RET>' is in black, as expected.
> > > Then the next line starts in green as expected.
> > > But only '/bd/home/ph' is in green.
> > > We then have 'ilippe/gdb/git/relbranch/gdb/minsyms.h,' in black.
> > > Then all is normal till the next page.
> > > 
> > > So, that seems to point at some remaining problem in the
> > > buffering/output of style/color codes.
> > 
> > I don't think it's due to buffering.  If I #ifdef away this part near
> > the end of prompt_for_continue:
> > 
> >   /* Restore the current styling.  */
> >   if (can_emit_style_escape (gdb_stdout))
> >     emit_style_escape (applied_style);
> > 
> > then the problem seems to go away.  So I think this style setting is
> > what causes the problem, probably because applied_style has the wrong
> > value here?  The incorrect style seems to start precisely from the
> > first character that exceeds the last column of a line, so I think
> > it's indeed due to the above snippet.
> > 
> > Tom, can you look into this, please?
> > 

Re: Still some style problems with 8.3 branch

[Tom Tromey]

>>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:

Eli> I don't think it's due to buffering.  If I #ifdef away this part near
Eli> the end of prompt_for_continue:

Eli>   /* Restore the current styling.  */
Eli>   if (can_emit_style_escape (gdb_stdout))
Eli>     emit_style_escape (applied_style);

Eli> then the problem seems to go away.  So I think this style setting is
Eli> what causes the problem, probably because applied_style has the wrong
Eli> value here?

I think this is correct.  Certainly applied_style has the wrong value
here, since it was reset by the call to emit_style_escape near the top
of prompt_for_continue.  

So, I think a patch to remove these lines is ok.  I can do it if you want.

Tom

Re: Still some style problems with 8.3 branch

[Tom Tromey]

>>>>> "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:

Philippe> It works better with the patch, but even with the patch,
Philippe> I still encountered one case where the style was not
Philippe> the expected style after the continuation prompt.

I tried this and couldn't reproduce, but...

Philippe> To reproduce (in a 80x24 terminal):
Philippe> ./gdb/gdb ./gdb/gdb
Philippe> break cli_ui_out::do_field_string
Philippe> run gdb/testsuite/outputs/gdb.ada/info_auto_lang/proc_in_ada
Philippe>   (gdb) b proc_in_c
Philippe>   (gdb) run
Philippe> info func error
Philippe> #### then type several times return at the page prompt.
Philippe> #### At some point in time, you see

...I may not fully understand the instructions - the breakpoint on
do_field_string seems to interfere with getting paging to work in the
inferior gdb.

Tom

Re: Still some style problems with 8.3 branch

[Philippe Waroquiers]

On Thu, 2019-04-11 at 13:21 -0600, Tom Tromey wrote:
> > > > > > "Philippe" == Philippe Waroquiers <philippe.waroquiers@skynet.be> writes:
> 
> Philippe> It works better with the patch, but even with the patch,
> Philippe> I still encountered one case where the style was not
> Philippe> the expected style after the continuation prompt.
> 
> I tried this and couldn't reproduce, but...
> 
> Philippe> To reproduce (in a 80x24 terminal):
> Philippe> ./gdb/gdb ./gdb/gdb
> Philippe> break cli_ui_out::do_field_string
> Philippe> run
> gdb/testsuite/outputs/gdb.ada/info_auto_lang/proc_in_ada
> Philippe>   (gdb) b proc_in_c
> Philippe>   (gdb) run
> Philippe> info func error
> Philippe> #### then type several times return at the page prompt.
> Philippe> #### At some point in time, you see
> 
> ...I may not fully understand the instructions - the breakpoint on
> do_field_string seems to interfere with getting paging to work in the
> inferior gdb.
Maybe what is unclear is what to give to the top-gdb, and what to
give to the inferior gdb.
You should be able to reproduce it by just
typing one after each other the following lines.
I have put before the info to type which process is getting the input:

shell: ./gdb/gdb ./gdb/gdb
top-gdb: break cli_ui_out::do_field_string
top-gdb: run gdb/testsuite/outputs/gdb.ada/info_auto_lang/proc_in_ada
gdb: b proc_in_c
gdb: run
top-gdb: info func error
.... and then a bunch of return, till an output line is wrongly styled
in black, just after a prompt for continue.

Philippe

Re: Still some style problems with 8.3 branch

[Eli Zaretskii]

> From: Tom Tromey <tom@tromey.com>
> Cc: Philippe Waroquiers <philippe.waroquiers@skynet.be>,  Tom Tromey <tom@tromey.com>,  gdb-patches@sourceware.org
> Date: Thu, 11 Apr 2019 13:20:16 -0600
> 
> >>>>> "Eli" == Eli Zaretskii <eliz@gnu.org> writes:
> 
> Eli> I don't think it's due to buffering.  If I #ifdef away this part near
> Eli> the end of prompt_for_continue:
> 
> Eli>   /* Restore the current styling.  */
> Eli>   if (can_emit_style_escape (gdb_stdout))
> Eli>     emit_style_escape (applied_style);
> 
> Eli> then the problem seems to go away.  So I think this style setting is
> Eli> what causes the problem, probably because applied_style has the wrong
> Eli> value here?
> 
> I think this is correct.  Certainly applied_style has the wrong value
> here, since it was reset by the call to emit_style_escape near the top
> of prompt_for_continue.  
> 
> So, I think a patch to remove these lines is ok.  I can do it if you want.

Thanks, I pushed this now to both branches.