Re: df --output

[Jim Meyering]

Bernhard Voelker wrote:

> Finally, I made it the whole long way to df's new --output option.
> Example use:
>
>   $ src/df --o=target,ipcent,pcent .
>   Mounted on  IUse% Use%
>   /media/sdb5    5%  75%
>
> The discussion about it started with http://bugs.gnu.org/10915.
>
> The biggest changes are in PATCH 03, 11, 17 and of course
> the new files for documentation [19] and tests [20].
>
> [PATCH 01/20] df: move the call of get_header from get_dev to main
> [PATCH 02/20] df: rename some displayable fields
> [PATCH 03/20] df: rework internal processing of output columns
> [PATCH 04/20] df: apply ambsalign to the last field with
> [PATCH 05/20] df: fix bracket opening style for structs
> [PATCH 06/20] df: use enum value for long option --total
> [PATCH 07/20] df: fix typo in comment
> [PATCH 08/20] df: remove now-obsolescent condition
> [PATCH 09/20] df: only sum up grand total if required
> [PATCH 10/20] df: print '-' into the target column of the total line
> [PATCH 11/20] df: Add basic support for mixed block and inode fields
> [PATCH 12/20] df: remove remainder of pre-mixed fields support
> [PATCH 13/20] df: minor cleanup to improve readability
> [PATCH 14/20] df: cleanup header_mode handling regarding --inodes
> [PATCH 15/20] df: give posix_format variable a better scope
> [PATCH 16/20] df: remove old comment
> [PATCH 17/20] df: add new --output option
> [PATCH 18/20] df: plug two minor memory holes detected by valgrind
> [PATCH 19/20] df: document the new --output option
> [PATCH 20/20] df: add a test for the --output option

Thanks for preparing all of that.
I don't have time for a full review now, but did a quick
pass over the top few patches.

One nit: in NEWS (in 19/20), when a period is not at the end of the line,
put two spaces after it, not just one.  Same thing in --help output.
There are several actually.  You can list them (and a few FPs) with

  git format-patch --stdout -20 |grep '\. [^ ]'

IMHO, the easiest way to fix them is to edit the git format-patch
output and re-apply it with "git am" to a fresh master-based branch.
(that works as long as inserting the space doesn't require splitting
a line in a patch -- it's fine to reformat logs when doing that, but
not patch output, of course)

Also, log typos:
s/brancket/bracket/
s/Accomodate/Accommodate/
----------------------

In your 17/20, I saw this;

  +/* Given a string, ARG, containing a comma-separated list of arguments
  +   to the --output option, add the appropriate fields to columns.  */
  +static void
  +decode_output_arg (char const *arg)
  +{
  +  static display_field_t const output_vals[] =
  +    {
  +      SOURCE_FIELD, FSTYPE_FIELD,
  +      TOTAL_FIELD,   USED_FIELD,  AVAIL_FIELD,  PCENT_FIELD,
  +      ITOTAL_FIELD, IUSED_FIELD, IAVAIL_FIELD, IPCENT_FIELD,
  +      TARGET_FIELD
  +    };
  +
  +  /* Array to mark each field as used. */
  +  static bool field_used[sizeof (output_vals)];

That bool array is unnecessarily large.
This should do what you want:

  static bool field_used[ARRAY_CARDINALITY (output_vals)];