Re: ui_out format strings for fields and styles (Re: [PATCH] Style "pwd" output)

[Pedro Alves <palves@redhat.com>]

On 6/5/19 11:21 PM, Tom Tromey wrote:
>>> We could just remove the enum, move to passing pointers everywhere, and
>>> convert the *_style objects to be pointers.  This would streamline
>>> things a bit; 
> 
> Pedro> ...
> 
>>> and we could use nullptr to mean "keep the default".
> 
> Pedro> Yeah, that would %pN / %p] redundant/unnecessary...
> 
> I didn't go quite as far as making everything take pointers, partly
> because it made it harder to preserve the distinction between the CLI
> layer and the ui-file styling layer.  So instead I just removed the
> enum.
> 
> My patch also removes (really just comments out) %pS and %pN in favor of
> requiring %ps.  Those could be restored with a bit of effort I guess.

Yes, I think so.  I think the main issue is that cli_style_option::style()
returns a new temporary ui_file_style, so we can't take its address, like in:

       current_uiout->message (_("Reading symbols from %pS%s%pS...\n"),
                               &file_name_style.style (), name, nullptr);

But that could be fixed by adding a m_style field to cli_style_option,
and making cli_style_option::style() return a const reference to that.
We'd either need to make cli_style_option register set command hooks
to update that m_size variable whenever the users changes the corresponding
foreground/background setting, or always update m_style when
cli_style_option::style() is called.

> 
> Pedro> If you'd like to give this a try, please do feel free to push to
> Pedro> the branch.
> 
> I didn't push but my patch is appended.  Let me know what you think.  I
> can push tomorrow if you want it.  I'm not sure now if what remains of
> the patch is a good idea or not; or if moving fully to pointers would be
> better.
> 

Please push.  I don't know either, but undoing a patch on a branch is
simple enough if we find that we want to do that.  :-)

Thanks,
Pedro Alves