Re: [PATCH 1/2] gdb: don't print escape characters when a style is disabled

[Andrew Burgess <andrew.burgess@embecosm.com>]

* Tom Tromey <tom@tromey.com> [2021-01-20 12:34:43 -0700]:

> >>>>> "Andrew" == Andrew Burgess <andrew.burgess@embecosm.com> writes:
> 
> Andrew> +# A replacement for the 'style' function found in gdb-utils.exp,
> Andrew> +# filter out requests for the disabled style.
> Andrew> +proc limited_style { str style } {
> 
> Rather than rename the global 'style', I think it would be fine to just
> have a wrapper proc that is used locally.  That seems a lot less tricky.
> Same with clean_restart.
> 
> Andrew>        /* Output pmatch with the highlight style.  */
> Andrew> -      set_output_style (stream, highlight_style.style ());
> Andrew> +      if (!highlight_style.style ().is_default ())
> Andrew> +	set_output_style (stream, highlight_style.style ());
> 
> set_output_style ends up setting applied_style.
> So avoiding this call seems iffy to me.

Tom,

Thanks for the feedback.

It's not clear to me if your feedback is about this one particular use
of the 'if (!STYLE.is_default ()) { ... }' pattern, or if you are
suggesting all uses of this pattern are suspect.

It's just that you added the original uses of this pattern...

So I guess my question would be, if its this one particular case then
I don't understand why this cases is different to all the others,
or...

... if it's all uses of this pattern then I guess my questions is,
does this mean you think the original uses were a mistake?  Or has
something else changed ?

> Maybe another approach would be to change emit_style_escape so that it
> does nothing if the currently-applied style is equal to the passed-in
> style?

I'll give this a try and see if I can get this working.

Thanks,
Andrew