[libvirt] [PATCH] tools: format percent strings of nodecpustats

[Alex Jia]

On 07/26/2011 11:52 PM, Eric Blake wrote:
> On 07/26/2011 01:32 AM, Alex Jia wrote:
>> * tools/virsh.c: format strings display for virsh nodecpustats 
>> --percent.
>>
>> * how to reproduce?
>>
>> % virsh nodecpustats --percent
>> usage:            2.0%
>>      user  :       1.0%
>>      system:       1.0%
>> idle  :          98.0%
>> iowait:           0.0%
>
> Yuck.  ACK that we need to fix this.
>
>>
>>
>> * after format strings
>>
>> % virsh nodecpustats --percent
>> usage :           2.0%
>> user  :           1.0%
>> system:           1.0%
>> idle  :          98.0%
>> iowait:           0.0%
>
> Still yuck.  Now we have some cases with two fields, and some with 
> three, making this harder to machine parse.  Better would be:
>
> % virsh nodecpustats --percent
> usage:            2.0%
> user:             1.0%
> system:           1.0%
> idle:            98.0%
> iowait:           0.0%
>
>>               vshPrint(ctl, "%-15s %5.1lf%%\n",
>> -                     _("usage:"), usage);
>> +                     _("usage :"), usage);
>
> By _always_ sticking the : directly by the text, translators don't 
> have to realize that the whitespace in the string being translated was 
> intended to be significant for alignment purposes (that is, alignment 
> should always be done independently of translations, whereas your 
> solution was only aligned for English).  Also, it better matches the 
> behavior of 'virsh dominfo', which also has aligned fields but does 
> not line up the colons.
>
> Here's what I squashed in before pushing:
>
>
> diff --git i/tools/virsh.c w/tools/virsh.c
> index d08b78f..113124f 100644
> --- i/tools/virsh.c
> +++ w/tools/virsh.c
> @@ -4438,9 +4438,9 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd)
>
>      if (!flag_percent) {
>          if (!flag_utilization) {
> -            vshPrint(ctl, "%-15s %20llu\n", _("user  :"), 
> cpu_stats[0].user);
> +            vshPrint(ctl, "%-15s %20llu\n", _("user:"), 
> cpu_stats[0].user);
>              vshPrint(ctl, "%-15s %20llu\n", _("system:"), 
> cpu_stats[0].sys);
> -            vshPrint(ctl, "%-15s %20llu\n", _("idle  :"), 
> cpu_stats[0].idle);
> +            vshPrint(ctl, "%-15s %20llu\n", _("idle:"), 
> cpu_stats[0].idle);
>              vshPrint(ctl, "%-15s %20llu\n", _("iowait:"), 
> cpu_stats[0].iowait);
>          }
>      } else {
> @@ -4448,7 +4448,7 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd *cmd)
>              usage = cpu_stats[0].util;
>
>              vshPrint(ctl, "%-15s %5.1lf%%\n", _("usage:"), usage);
> -            vshPrint(ctl, "%-15s %5.1lf%%\n", _("idle :"), 100 - usage);
> +            vshPrint(ctl, "%-15s %5.1lf%%\n", _("idle:"), 100 - usage);
>          } else {
>              user_time   = cpu_stats[1].user   - cpu_stats[0].user;
>              sys_time    = cpu_stats[1].sys    - cpu_stats[0].sys;
> @@ -4459,13 +4459,13 @@ cmdNodeCpuStats(vshControl *ctl, const vshCmd 
> *cmd)
>              usage = (user_time + sys_time) / total_time * 100;
>
>              vshPrint(ctl, "%-15s %5.1lf%%\n",
> -                     _("usage :"), usage);
> +                     _("usage:"), usage);
>              vshPrint(ctl, "%-15s %5.1lf%%\n",
> -                     _("user  :"), user_time / total_time * 100);
> +                     _("user:"), user_time / total_time * 100);
>              vshPrint(ctl, "%-15s %5.1lf%%\n",
>                       _("system:"), sys_time  / total_time * 100);
>              vshPrint(ctl, "%-15s %5.1lf%%\n",
> -                     _("idle  :"), idle_time     / total_time * 100);
> +                     _("idle:"), idle_time     / total_time * 100);
>              vshPrint(ctl, "%-15s %5.1lf%%\n",
>                       _("iowait:"), iowait_time   / total_time * 100);
>          }
>
Agree, thanks for your review and modification.

Regards,
Alex