Re: [PATCH] Style "pwd" output

[Pedro Alves <palves at redhat dot com>]

On 6/5/19 2:42 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> I wish we didn't have to split the lines across different calls though.
> Pedro> I know we don't officially do i18n yet (*), but these kinds of changes will
> Pedro> only make it more difficult to get there.
> 
> Pedro> Maybe with something like:
> 
> Pedro>   if (strcmp (cwd.get (), current_directory) != 0)
> Pedro>     printf_unfiltered (_("Working directory <style=filename>%s<style/>\n (canonically <style=filename>%s<style/>).\n"),
> Pedro> 		       current_directory, cwd.get ());
> Pedro>   else
> Pedro>     printf_unfiltered (_("Working directory <style=filename>%s<style/>.\n"), current_directory);
> 
> Pedro> I'm sure you've considered something like that; I think we've discussed it
> Pedro> before.  What are your current thoughts?
> 
> I think what would be nice is a gdb-specific printf extension for this,
> e.g.:
> 
>     printf_unfiltered ("Working directory %<%s%>\n",
>                        ui_out_style_kind::FILE, 
>                        current_directory);
> 
> %< takes a style argument and %> does not.
> 
> However, for that to work, we'd need a GCC enhancement to let gdb modify
> the printf format checking.

While not so good looking, instead of a GCC enhancement, we could maybe
do it the Linux way, like binutils did too:

 https://sourceware.org/ml/binutils/2018-02/msg00306.html

See 871b3ab29e87c.

So e.g., %pS would be a style, a %pN would be no style.

    printf_unfiltered ("Working directory %pS%s%pN\n",
                       ui_out_style_kind::FILE, 
                       current_directory);

> 
> 
> Note that if we want to be serious about i18n then there is also the
> problem that the whole MI approach is not very i18n-friendly.  There are
> spots that split calls like this, but which can't easily be unsplit
> (even with a printf extension) due to MI.  So maybe something bigger is
> needed.

Right.

So instead of e.g.:

 uiout->text ("\nWatchpoint ");
 uiout->field_int ("wpnum", b->number);
 uiout->text (" deleted because the program has left the block in\n"
              "which its expression is valid.\n");

We could have:

 uiout->field_fmt ("\nWatchpoint %pF deleted because the program "
                    "has left the block in\n"
                    "which its expression is valid.\n"", 
                    int_field ("wpnum", b->number).ptr ());

With ui_out_field being something like 

 struct int_field
 {
    int_field (const char *field_name, int val);

    // need this because we can't pass a reference via va_args.
    void *ptr () { return this; }

    const char *m_field_name;
    int m_val;
 };

There would then be another class for CORE_ADDR, another for strings,
etc, matching the ui_out::field_int, ui_out::field_string, etc. methods.
Or alternatively, we could have a single uiout_field class that records 
its type depending on which ctor was called.

Thanks,
Pedro Alves