Re: [PATCH 1/2] Update btrfs filesystem df command

[Hugo Mills <hugo@carfax.org.uk>]

[-- Attachment #1: Type: text/plain, Size: 2234 bytes --]

On Wed, Oct 03, 2012 at 11:34:00PM +0300, Ilya Dryomov wrote:
> On Wed, Oct 03, 2012 at 07:22:31PM +0200, Goffredo Baroncelli wrote:
[snip]
> > +static const char * const cmd_disk_free_usage[] = {
> > +	"btrfs filesystem df [-d|-s][-k] <path> [<path>..]",
> > +	"Show space usage information for a mount point(s).",
> > +	"",
> > +	"-k\tSet KB (1024 bytes) as unit",
> > +	"-s\tShow the summary section only",
> > +	"-d\tShow the detail section only",
> > +	NULL
> > +};
> > +
> > +static int cmd_disk_free(int argc, char **argv)
> > +{
> > +
> > +	int 	flags=DF_SHOW_SUMMARY|DF_SHOW_DETAIL|DF_HUMAN_UNIT;
> > +	int	i, more_than_one=0;
> > +
> > +	optind = 1;
> > +	while(1){
> > +		char	c = getopt(argc, argv, "dsk");
> > +		if(c<0)
> > +			break;
> > +		switch(c){
> > +			case 'd':
> > +				flags &= ~DF_SHOW_SUMMARY;
> > +				break;
> > +			case 's':
> > +				flags &= ~DF_SHOW_DETAIL;
> > +				break;
> > +			case 'k':
> > +				flags &= ~DF_HUMAN_UNIT;
> > +				break;
> > +			default:
> > +				usage(cmd_disk_free_usage);
> > +		}
> > +	}
> > +
> > +	if( !(flags & (DF_SHOW_SUMMARY|DF_SHOW_DETAIL)) ){
> > +		fprintf(stderr, "btrfs filesystem df: it is not possible to specify -s AND -d\n");
> 
> This doesn't look right at all.  You are adding two switches and
> specifying both of them is an error?  A little too much for a command
> whose job is to do some basic math and pretty-print the result.
> 
> How about displaying just the summary by default and then adding a
> *single* switch (-v or whatever) for summary+details?

   I'd prefer to see both sections by default. The reason for this is
that without both sections, people tend to get confused because they
don't know they're looking at half the story (e.g. some numbers change
twice as fast as they think they should).

   I think supplying both options should probably show both sections
again, and make it not an error to do so, but I'm happy either way.

   Hugo.

-- 
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
  PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
  --- "He's a nutcase, you know. There's no getting away from it -- ---  
                     he'll end up with a knighthood"                     

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 828 bytes --]