Re: svn commit: r322252 - head/usr.bin/vmstat

2017-08-09 Thread Emmanuel Vadot
On Wed, 9 Aug 2017 18:44:19 +1000 (EST)
Bruce Evans  wrote:

> On Tue, 8 Aug 2017, Emmanuel Vadot wrote:
> 
> > On Tue, 8 Aug 2017 23:55:52 +1000 (EST)
> > Bruce Evans  wrote:
> >
> >> On Tue, 8 Aug 2017, Emmanuel Vadot wrote:
> >>
> >>> Log:
> >>>  vmstat: Always emit a space after the free-memory column
> >>>
> >>>  When displaying in non-human form, if the free-memory number
> >>>  is large (more than 7 digits), there is no space between it and
> >>>  the page fault column.
> >>>
> >>>  PR:  221290
> >>>  Submitted by:Josuah Demangeon  (Original version)
> >>>
> >>> Modified:
> >>>  head/usr.bin/vmstat/vmstat.c
> >>>
> >>> Modified: head/usr.bin/vmstat/vmstat.c
> >>> ==
> >>> --- head/usr.bin/vmstat/vmstat.c  Tue Aug  8 11:49:36 2017
> >>> (r322251)
> >>> +++ head/usr.bin/vmstat/vmstat.c  Tue Aug  8 12:18:11 2017
> >>> (r322252)
> >>> @@ -832,6 +832,7 @@ dovmstat(unsigned int interval, int reps)
> >>>   xo_emit(" ");
> >>>   xo_emit("{:free-memory/%7d}",
> >>>   vmstat_pgtok(total.t_free));
> >>> + xo_emit(" ");
> >>>   }
> >>>   xo_emit("{:total-page-faults/%5lu} ",
> >>>   (unsigned long)rate(sum.v_vm_faults -
> >>
> >> This seems to break the formatting.  There was a negative amount of space
> >> available for expansion, and since the header was not expanded to match
> >> its alignment with the fields is more random than before.  With -h, the
> >> width was 80 columns, giving ugly line wrap on 80-column terminals with
> >> auto-wrap.  Now it is 81 columns, giving uglier line wrap on all 80-
> >> column terminals.
> >
> > This break nothing, This was the case before too (with or without
> > -h), just tested in tmux with force-width 80.
> 
> Testing shows that it breaks the -H case as expected.  (I had misread the
> change as affecting the -h case too.)
> 
> Before:
> 
> X procs memorypagedisks faults cpu
> X r b w avm fre  flt  re  pi  pofr   sr ad0 ad1   insycs 
> us sy id
> X 0 0 0 12719376  977460 2167   3  28   0  2545  451   0   0  268 27162  7175 
>  9  1 91
> 
> Misformatting in the data line starts with the avm field taking 8 of the 7
> columns reserved for it.  This and all subsequent fields are misaligned with
> the header.
> 
> The header is pre-misformatted to width 85, with lots of inconsistencies in
> the first line.  Old versions were carefully formatted to width 79, with no
> inconsistencies in the first line.  The extra width for avm makes the extra
> width for the header not even work to line up.
> 
> X procs memorypagedisks faults cpu
> X r b w avm fre  flt  re  pi  pofr   sr ad0 ad1   insycs 
> us sy id
> X 0 0 0 12719380  977460  2167   3  28   0  2545  451   0   0  268 27162  
> 7175  9  1 91
> 
> The new bugs are that the extra space before the "flt" field increases the
> off-by-1 error to off-by-2 for this field and all subsequent fields.  All
> fields starting with "flt" now have an error of at least off-by-1 even
> if none are too wide, since the header was not expanded to match.  Expansion
> would increase its breakage from 6 to wide to 7 too wide.

 Thanks, I know see the problem.
 I'll see if this could be corrected easily with libxo ...

> >> The bugs were mostly in the first line of the header:
> >> - the second line of the header was correct for vmstat -h
> >> - for vmstat without -h, the second line of the header was apparently 
> >> broken
> >>by a change like the one here, that added a space after the "r b w" 
> >> fields
> >>without adding one in the "r b w" header
> >> - most of the fields in the first line of the header are misaligned with 
> >> the
> >>second lone.  Many have drifted 3 to the left of where the were in a 
> >> sort
> >>of center-justified place.  Some of these might have actually been
> >>intended to be left justified, but had an off by +1 error.  Now these
> >>have an error of off by -2 relative to left justifications.
> >>
> >> Only the "memory" header in the first line is better than in old versions.
> >> It is now left justified.  Left justifying all headers in the first line
> >> is probably best.  I couldn't find a good way to delimit the right hand
> >> side of the extents of the headers in the first line.  The second line of
> >> the headers already uses right justification consistently and this works
> >> well.
> >
> > I think that all this might be true but you might talk about the whole
> > libxo conversion that was done, not my commit right ?
> 
> I try not to look at libxo, since its misformattings are enormous starting
> in the source code.
> 
> My columnize.awk script works almost perfectly to fix the above
> misformattings:
> 
> X BEGIN {
> X columns = 79

Re: svn commit: r322252 - head/usr.bin/vmstat

2017-08-09 Thread Bruce Evans

On Tue, 8 Aug 2017, Emmanuel Vadot wrote:


On Tue, 8 Aug 2017 23:55:52 +1000 (EST)
Bruce Evans  wrote:


On Tue, 8 Aug 2017, Emmanuel Vadot wrote:


Log:
 vmstat: Always emit a space after the free-memory column

 When displaying in non-human form, if the free-memory number
 is large (more than 7 digits), there is no space between it and
 the page fault column.

 PR:221290
 Submitted by:  Josuah Demangeon  (Original version)

Modified:
 head/usr.bin/vmstat/vmstat.c

Modified: head/usr.bin/vmstat/vmstat.c
==
--- head/usr.bin/vmstat/vmstat.cTue Aug  8 11:49:36 2017
(r322251)
+++ head/usr.bin/vmstat/vmstat.cTue Aug  8 12:18:11 2017
(r322252)
@@ -832,6 +832,7 @@ dovmstat(unsigned int interval, int reps)
xo_emit(" ");
xo_emit("{:free-memory/%7d}",
vmstat_pgtok(total.t_free));
+   xo_emit(" ");
}
xo_emit("{:total-page-faults/%5lu} ",
(unsigned long)rate(sum.v_vm_faults -


This seems to break the formatting.  There was a negative amount of space
available for expansion, and since the header was not expanded to match
its alignment with the fields is more random than before.  With -h, the
width was 80 columns, giving ugly line wrap on 80-column terminals with
auto-wrap.  Now it is 81 columns, giving uglier line wrap on all 80-
column terminals.


This break nothing, This was the case before too (with or without
-h), just tested in tmux with force-width 80.


Testing shows that it breaks the -H case as expected.  (I had misread the
change as affecting the -h case too.)

Before:

X procs memorypagedisks faults cpu
X r b w avm fre  flt  re  pi  pofr   sr ad0 ad1   insycs us 
sy id
X 0 0 0 12719376  977460 2167   3  28   0  2545  451   0   0  268 27162  7175  
9  1 91

Misformatting in the data line starts with the avm field taking 8 of the 7
columns reserved for it.  This and all subsequent fields are misaligned with
the header.

The header is pre-misformatted to width 85, with lots of inconsistencies in
the first line.  Old versions were carefully formatted to width 79, with no
inconsistencies in the first line.  The extra width for avm makes the extra
width for the header not even work to line up.

X procs memorypagedisks faults cpu
X r b w avm fre  flt  re  pi  pofr   sr ad0 ad1   insycs us 
sy id
X 0 0 0 12719380  977460  2167   3  28   0  2545  451   0   0  268 27162  7175  
9  1 91

The new bugs are that the extra space before the "flt" field increases the
off-by-1 error to off-by-2 for this field and all subsequent fields.  All
fields starting with "flt" now have an error of at least off-by-1 even
if none are too wide, since the header was not expanded to match.  Expansion
would increase its breakage from 6 to wide to 7 too wide.


The bugs were mostly in the first line of the header:
- the second line of the header was correct for vmstat -h
- for vmstat without -h, the second line of the header was apparently broken
   by a change like the one here, that added a space after the "r b w" fields
   without adding one in the "r b w" header
- most of the fields in the first line of the header are misaligned with the
   second lone.  Many have drifted 3 to the left of where the were in a sort
   of center-justified place.  Some of these might have actually been
   intended to be left justified, but had an off by +1 error.  Now these
   have an error of off by -2 relative to left justifications.

Only the "memory" header in the first line is better than in old versions.
It is now left justified.  Left justifying all headers in the first line
is probably best.  I couldn't find a good way to delimit the right hand
side of the extents of the headers in the first line.  The second line of
the headers already uses right justification consistently and this works
well.


I think that all this might be true but you might talk about the whole
libxo conversion that was done, not my commit right ?


I try not to look at libxo, since its misformattings are enormous starting
in the source code.

My columnize.awk script works almost perfectly to fix the above
misformattings:

X BEGIN {
X   columns = 79# XXX
X }
X 
X {

X   # Determine fields and field widths for current line.
X   # awk's field splitting feature turned out to be inadequate,
X   # and this would be even easier in C.
X   n = split($0, a, "")
X   f = 1
X   j = 1
X   while (j <= n) {
X   cpw[f] = 0
X   cfw[f] = 0
X   while (j <= n && a[j] == " ") {
X   cpw[f]++# current (left) padding width
X   j++
X   }
X   if (

Re: svn commit: r322252 - head/usr.bin/vmstat

2017-08-08 Thread Emmanuel Vadot
On Tue, 8 Aug 2017 23:55:52 +1000 (EST)
Bruce Evans  wrote:

> On Tue, 8 Aug 2017, Emmanuel Vadot wrote:
> 
> > Log:
> >  vmstat: Always emit a space after the free-memory column
> >
> >  When displaying in non-human form, if the free-memory number
> >  is large (more than 7 digits), there is no space between it and
> >  the page fault column.
> >
> >  PR:221290
> >  Submitted by:  Josuah Demangeon  (Original version)
> >
> > Modified:
> >  head/usr.bin/vmstat/vmstat.c
> >
> > Modified: head/usr.bin/vmstat/vmstat.c
> > ==
> > --- head/usr.bin/vmstat/vmstat.cTue Aug  8 11:49:36 2017
> > (r322251)
> > +++ head/usr.bin/vmstat/vmstat.cTue Aug  8 12:18:11 2017
> > (r322252)
> > @@ -832,6 +832,7 @@ dovmstat(unsigned int interval, int reps)
> > xo_emit(" ");
> > xo_emit("{:free-memory/%7d}",
> > vmstat_pgtok(total.t_free));
> > +   xo_emit(" ");
> > }
> > xo_emit("{:total-page-faults/%5lu} ",
> > (unsigned long)rate(sum.v_vm_faults -
> 
> This seems to break the formatting.  There was a negative amount of space
> available for expansion, and since the header was not expanded to match
> its alignment with the fields is more random than before.  With -h, the
> width was 80 columns, giving ugly line wrap on 80-column terminals with
> auto-wrap.  Now it is 81 columns, giving uglier line wrap on all 80-
> column terminals.

 This break nothing, This was the case before too (with or without
-h), just tested in tmux with force-width 80.

> The bugs were mostly in the first line of the header:
> - the second line of the header was correct for vmstat -h
> - for vmstat without -h, the second line of the header was apparently broken
>by a change like the one here, that added a space after the "r b w" fields
>without adding one in the "r b w" header
> - most of the fields in the first line of the header are misaligned with the
>second lone.  Many have drifted 3 to the left of where the were in a sort
>of center-justified place.  Some of these might have actually been
>intended to be left justified, but had an off by +1 error.  Now these
>have an error of off by -2 relative to left justifications.
> 
> Only the "memory" header in the first line is better than in old versions.
> It is now left justified.  Left justifying all headers in the first line
> is probably best.  I couldn't find a good way to delimit the right hand
> side of the extents of the headers in the first line.  The second line of
> the headers already uses right justification consistently and this works
> well.
> 
> Bruce

 I think that all this might be true but you might talk about the whole
libxo conversion that was done, not my commit right ?

-- 
Emmanuel Vadot  
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r322252 - head/usr.bin/vmstat

2017-08-08 Thread Bruce Evans

On Tue, 8 Aug 2017, Emmanuel Vadot wrote:


Log:
 vmstat: Always emit a space after the free-memory column

 When displaying in non-human form, if the free-memory number
 is large (more than 7 digits), there is no space between it and
 the page fault column.

 PR:221290
 Submitted by:  Josuah Demangeon  (Original version)

Modified:
 head/usr.bin/vmstat/vmstat.c

Modified: head/usr.bin/vmstat/vmstat.c
==
--- head/usr.bin/vmstat/vmstat.cTue Aug  8 11:49:36 2017
(r322251)
+++ head/usr.bin/vmstat/vmstat.cTue Aug  8 12:18:11 2017
(r322252)
@@ -832,6 +832,7 @@ dovmstat(unsigned int interval, int reps)
xo_emit(" ");
xo_emit("{:free-memory/%7d}",
vmstat_pgtok(total.t_free));
+   xo_emit(" ");
}
xo_emit("{:total-page-faults/%5lu} ",
(unsigned long)rate(sum.v_vm_faults -


This seems to break the formatting.  There was a negative amount of space
available for expansion, and since the header was not expanded to match
its alignment with the fields is more random than before.  With -h, the
width was 80 columns, giving ugly line wrap on 80-column terminals with
auto-wrap.  Now it is 81 columns, giving uglier line wrap on all 80-
column terminals.

The bugs were mostly in the first line of the header:
- the second line of the header was correct for vmstat -h
- for vmstat without -h, the second line of the header was apparently broken
  by a change like the one here, that added a space after the "r b w" fields
  without adding one in the "r b w" header
- most of the fields in the first line of the header are misaligned with the
  second lone.  Many have drifted 3 to the left of where the were in a sort
  of center-justified place.  Some of these might have actually been
  intended to be left justified, but had an off by +1 error.  Now these
  have an error of off by -2 relative to left justifications.

Only the "memory" header in the first line is better than in old versions.
It is now left justified.  Left justifying all headers in the first line
is probably best.  I couldn't find a good way to delimit the right hand
side of the extents of the headers in the first line.  The second line of
the headers already uses right justification consistently and this works
well.

Bruce
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"