Patchwork gdb: Don't flush line wrap buffer before adding newline

login
register
mail settings
Submitter Andrew Burgess
Date Feb. 5, 2019, 3:55 p.m.
Message ID <20190205155545.GC2829@embecosm.com>
Download mbox | patch
Permalink /patch/31314/
State New
Headers show

Comments

Andrew Burgess - Feb. 5, 2019, 3:55 p.m.
* Tom Tromey <tom@tromey.com> [2019-02-05 08:27:29 -0700]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> 	* utils.c (fputs_maybe_filtered): Don't flush the line wrap buffer
> Andrew> 	before inserting a newline for line-wrapping.
> 
> Thanks, this is ok.

Really, I was just teasing - this patch is rubbish :-/

(Not really, I spotted my mistake immediately after hitting send.)

The code used to look like this:

    if (wrap_column)
    {
      emit_style_escape (ui_file_style ());
      flush_wrap_buffer (stream);
      fputc_unfiltered ('\n', stream);
    }

The problem is that 'emit_style_escape' sends the escape string to the
wrap_buffer, while the newline is sent directly to the console.

This means that GDB is still styled when the newline is sent, which I
think means that if we hit a pager prompt this could be styled.

I did consider having a second version of 'emit_style_escape' that
by-passed the wrap_buffer, but I worried that this might be too open
to incorrect use.  Instead I thought a better approach would be backup
and clear the wrap_buffer before calling 'emit_style_escape' in the
above code, the 'flush_wrap_buffer' can then be used safely.

The patch below shows a new proposal for utils.c.

Do you agree with the issues I see above?

What do you think of the new approach?

Thanks,
Andrew


---
Tom Tromey - Feb. 5, 2019, 4:11 p.m.
>>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:

Andrew> The patch below shows a new proposal for utils.c.
Andrew> Do you agree with the issues I see above?
Andrew> What do you think of the new approach?

Could you try my branch?  It fixes this problem and a few more besides.
I am in the process -- kind of slow due to travel etc -- of writing the
last couple of test cases to make sure it is all correct... Philippe
found a number of bugs in this area a while back and the branch attempts
to fix them all.

It's the branch submit/fix-pager on my github.
It's in an ugly form right now (I didn't squash the patches yet), but
that shouldn't matter for trying it.

Tom
Andrew Burgess - Feb. 5, 2019, 5:15 p.m.
* Tom Tromey <tom@tromey.com> [2019-02-05 09:11:52 -0700]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> The patch below shows a new proposal for utils.c.
> Andrew> Do you agree with the issues I see above?
> Andrew> What do you think of the new approach?
> 
> Could you try my branch?  It fixes this problem and a few more
> besides.

Yes, this fixes the original issue, and a second issue that I had an
almost complete patch for.

I'll wait for your fixes to land.

Thanks,
Andrew


> I am in the process -- kind of slow due to travel etc -- of writing the
> last couple of test cases to make sure it is all correct... Philippe
> found a number of bugs in this area a while back and the branch attempts
> to fix them all.
> 
> It's the branch submit/fix-pager on my github.
> It's in an ugly form right now (I didn't squash the patches yet), but
> that shouldn't matter for trying it.
> 
> Tom

Patch

diff --git a/gdb/utils.c b/gdb/utils.c
index 6fb5736abb5..153de6cd0f8 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -1787,9 +1787,12 @@  fputs_maybe_filtered (const char *linebuffer, struct ui_file *stream,
 	         anyway; if it's wrong, let us keep going.  */
 	      if (wrap_column)
 		{
+		  std::string tmp = wrap_buffer;
+		  wrap_buffer.clear ();
 		  emit_style_escape (ui_file_style ());
 		  flush_wrap_buffer (stream);
 		  fputc_unfiltered ('\n', stream);
+		  wrap_buffer = tmp;
 		}
 
 	      /* Possible new page.  Note that