Re: df --output

[Pádraig Brady]

On 09/30/2012 05:43 PM, Bernhard Voelker wrote:
> On 09/29/2012 11:29 PM, Bernhard Voelker wrote:
> > Here you are (see attached):
> >
> > [PATCH 20/20] df: print "total" in grand total's target field if no source
> > [PATCH 19/20] df: cleanup comments regarding double blank after
> > [PATCH 18/20] df: add a test for the --output option
> > [PATCH 17/20] df: document the new --output option
> > [PATCH 16/20] df: plug two minor memory holes detected by valgrind
> > [PATCH 15/20] df: add new --output option
> > [PATCH 14/20] df: enhance comment describing history
> > [PATCH 13/20] df: give posix_format variable a better scope
> > [PATCH 12/20] df: cleanup header_mode handling regarding --inodes
> > [PATCH 11/20] df: minor cleanup to improve readability
> > [PATCH 10/20] df: remove remainder of pre-mixed fields support
> > [PATCH 09/20] df: add basic support for mixed block and inode fields
> > [PATCH 08/20] df: print '-' into the target column of the total line
> > [PATCH 07/20] df: only sum up grand total if required
> > [PATCH 06/20] df: remove now-obsolescent condition
> > [PATCH 05/20] df: use enum value for long option --total
> > [PATCH 04/20] df: apply ambsalign to the last field with
> > [PATCH 03/20] df: rework internal processing of output columns
> > [PATCH 02/20] df: rename some displayable fields
> > [PATCH 01/20] df: move the call of get_header from get_dev to main
> >
> > The bigger/important changes are in the patches 03, 09, and 15.
> >
> > Example:
> >
> >    $ src/df -h --tot --o=target,pcent,ipcent / /home
> >    Mounted on Use% IUse%
> >    /           65%   32%
> >    /home       65%    2%
> >    total       65%    5%
> >
> > Have a nice day,
> > Berny
>
> I have added another patch at the end:
>
>    [PATCH 21/21] df: ensure that the mount list contains a valid
>    fstype with --output
>
>    The function read_file_system_list only ensures a valid file
>    system type if the calling parameter is true. With the new
>    option --output, there is an additional condition needed:
>    when the FSTYPE_FIELD is used.
>
> I didn't notice a problem on GNU/Linux, but the documentation of
> read_file_system_list() leads to the conclusion that df needs to
> pass true also for 'df --o=fstype,...'.
>
> The new, additional condition may also have replaced the one before
>      || print_type
> because
>      || field_data[FSTYPE_FIELD].used
> also leads to true with 'df -T'; I just left it there for now.
>
> Have a nice day,
> Berny


I've finished my review. Sorry for the delay.
I've made a couple of adjustments in the attached df--output-adjustments.patch

The first is to rename --output=total to --output=size
I think having `df --total --output=total` is a bit confusing?

Also I've changed df --output to show the block size in the "size" header,
unless -h (human) is being used.  I.E. behave with this header
more like the traditional df output.  I think this is needed
as important info is lost about the units otherwise?

I've put the full rebased 24 item patch set here:
http://www.pixelbeat.org/patches/coreutils/df--output.patch.bz2

Before merging I'll probably reorder/squash to 3 patches;
logic, docs and test

thanks,
Pádraig.

df--output-adjustments.patch
Description: Text Data