On 11/07/2012 04:09 AM, Pádraig Brady wrote:
> I've finished my review. Sorry for the delay.
Thank you, no worries.
> 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?
I made it 'total' to better align with 'itotal' in the inode case,
but ambiguity with --total is a valid point.
Wouldn't it be worth to s/TOTAL_FIELD/SIZE_FIELD/ then?
> 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?
+1
> I've put the full rebased 24 item patch set here:
> http://www.pixelbeat.org/patches/coreutils/df--output.patch.bz2
Apart from the above note, it looks all good to me. Thanks.
BTW: during development, I was using the attached bulk test script
to catch regressions between df of version 8.19 and my version.
If you want to try it, then you'll have to adapt the variables
DF_OLD / DF_NEW at the top of the script to fit into your environment.
Have a nice day,
Berny
#!/bin/sh
# Define test candidates:
# both must start with "/home/" and end on "df" (see sed command below)
DF_OLD=/home/berny/git/coreutils-8.19/inst/bin/df
DF_NEW=/home/berny/coreutils/src/df
TEST_SH=/tmp/dftest.tmp.sh
OUT_OLD=/tmp/dftest.out.old
OUT_NEW=/tmp/dftest.out.new
function dfprep
{
printf "#!/bin/sh \n"
printf "DF=\"\$1\" \n"
for o in '' -i -h -hi -P -hP -Ph -iP -Pi -hiP -a -l '-t ext3' '-t typo' '-x
ext3' '-x typo'
do
for s in '' -k -m -Bk -BkB -BkiB -B1k -B1kB -B1kiB -Bsi -Bhuman-readable
do
for t in '' '--total'
do
for p in '' '.'
do
printf " \"\$DF\" $o $s $t $p || echo \$? \n"
printf "BLOCKSIZE=1M \"\$DF\" $o $s $t $p || echo \$? \n"
printf "BLOCK_SIZE=1M \"\$DF\" $o $s $t $p || echo \$? \n"
printf "DF_BLOCK_SIZE=1M \"\$DF\" $o $s $t $p || echo \$? \n"
done
done
done
done
}
dfprep > $TEST_SH
sync
sh -x $TEST_SH $DF_OLD > $OUT_OLD 2>&1
sh -x $TEST_SH $DF_NEW > $OUT_NEW 2>&1
for f in $OUT_OLD $OUT_NEW
do
sed -i \
-e '1n; s/\/home\/.*\/df/df/' \
-e 's/^\(total.*%\)$/\1 -/' \
$f
for x in 3 2 1 ; do
sed -i \
-e "s/ \([0-9]\{12\}\)\([^0-9]\)/ abcdefghijkl\2/;
s/ \([0-9]\{11\}\)\([^0-9]\)/ abcdefghijk\2/; \
s/ \([0-9]\{10\}\)\([^0-9]\)/ abcdefghij\2/; \
s/ \([0-9]\{9\}\)\([^0-9]\)/ abcdefghi\2/; \
s/ \([0-9]\{8\}\)\([^0-9]\)/ abcdefgh\2/; \
s/ \([0-9]\{7\}\)\([^0-9]\)/ abcdefg\2/; \
s/ \([0-9]\{6\}\)\([^0-9]\)/ abcdef\2/; \
s/ \([0-9]\{5\}\)\([^0-9]\)/ abcde\2/; \
s/ \([0-9]\{4\}\)\([^0-9]\)/ abcd\2/; \
s/ \([0-9]\{3\}\)\([^0-9]\)/ abc\2/; \
s/ \([0-9]\{2\}\)\([^0-9]\)/ ab\2/; \
s/ \([0-9]\{1\}\)\([^0-9]\)/ a\2/
" \
$f
done
done
meld $OUT_OLD $OUT_NEW &
