Re: cvs commit: src/usr.sbin/pkg_install/add main.c pkg_add.1 src/usr.sbin/pkg_install/create main.c pkg_create.1 src/usr.sbin/pkg_install/delete main.c pkg_delete.1 src/usr.sbin/pkg_install/info main.c pkg_info.1 ...

From: Coleman Kane <cokane_at_FreeBSD.org>
Date: Thu, 05 Jun 2008 08:15:41 -0400
On Wed, 2008-06-04 at 15:29 -0700, Steve Kargl wrote:
> On Wed, Jun 04, 2008 at 05:24:23PM -0400, Coleman Kane wrote:
> > It's probably premature to begin shooting that down. I merely picked
> > "cp" as the first tool that came to mind which lives in /bin. Maybe I
> > should have picked on /bin/pax or /usr/bin/lex instead, or something
> > else with 20+ getopt args :P.
> > 
> > My point was that it was pretty absurd to start treating this like it
> > will turn into a giant mad party of getopt_long conversions. I don't
> > really have the inclination to go out and add long options to any tool
> > myself, because I have better things to do with my time. As I said
> > earlier, I think it's a better policy to discuss this when the
> > patch/commit comes along, and see what people's feelings are on the
> > matter then. I don't think it is anybody's place at the moment (except
> > maybe core_at_) to try and deter a committer from making what some feel is
> > a beneficial change to software.
> > 
> 
> Please commit.  The patch gives conformance to Posix 
> (ie., IEEE Std 1003.1, 2004 Edition).
> 
> As a bonus, I've included long options for compatibility
> with GNU wc.
> 
> Index: wc.1
> ===================================================================
> RCS file: /home/ncvs/src/usr.bin/wc/wc.1,v
> retrieving revision 1.25
> diff -u -p -r1.25 wc.1
> --- wc.1	21 Dec 2006 22:59:07 -0000	1.25
> +++ wc.1	4 Jun 2008 22:27:15 -0000
> _at__at_ -71,25 +71,19 _at__at_ the last file.
>  .Pp
>  The following options are available:
>  .Bl -tag -width indent
> -.It Fl c
> +.It Fl c , -bytes
>  The number of bytes in each input file
>  is written to the standard output.
> -This will cancel out any prior usage of the
> -.Fl m
> -option.
> -.It Fl l
> +.It Fl l , -lines
>  The number of lines in each input file
>  is written to the standard output.
> -.It Fl m
> +.It Fl m , -chars
>  The number of characters in each input file is written to the standard output.
>  If the current locale does not support multibyte characters, this
>  is equivalent to the
>  .Fl c
>  option.
> -This will cancel out any prior usage of the
> -.Fl c
> -option.
> -.It Fl w
> +.It Fl w , -words
>  The number of words in each input file
>  is written to the standard output.
>  .El
> Index: wc.c
> ===================================================================
> RCS file: /home/ncvs/src/usr.bin/wc/wc.c,v
> retrieving revision 1.21
> diff -u -p -r1.21 wc.c
> --- wc.c	27 Dec 2004 22:27:56 -0000	1.21
> +++ wc.c	4 Jun 2008 22:27:15 -0000
> _at__at_ -53,6 +53,7 _at__at_ __FBSDID("$FreeBSD: src/usr.bin/wc/wc.c,
>  #include <err.h>
>  #include <errno.h>
>  #include <fcntl.h>
> +#include <getopt.h>
>  #include <locale.h>
>  #include <stdint.h>
>  #include <stdio.h>
> _at__at_ -68,6 +69,15 _at__at_ int doline, doword, dochar, domulti;
>  static int	cnt(const char *);
>  static void	usage(void);
>  
> +static char opts[] = "clmw";
> +static struct option longopts[] = {
> +	{ "bytes",	no_argument,		NULL,		'c' },
> +	{ "lines",	no_argument,		NULL,		'l' },
> +	{ "chars",	no_argument,		NULL,		'm' },
> +	{ "words",	no_argument,		NULL,		'w' },
> +	{ NULL,		0,			NULL,		0 },
> +};
> +
>  int
>  main(int argc, char *argv[])
>  {
> _at__at_ -75,7 +85,7 _at__at_ main(int argc, char *argv[])
>  
>  	(void) setlocale(LC_CTYPE, "");
>  
> -	while ((ch = getopt(argc, argv, "clmw")) != -1)
> +	while ((ch = getopt_long(argc, argv, opts, longopts, NULL)) != -1)
>  		switch((char)ch) {
>  		case 'l':
>  			doline = 1;
> _at__at_ -85,11 +95,13 _at__at_ main(int argc, char *argv[])
>  			break;
>  		case 'c':
>  			dochar = 1;
> -			domulti = 0;
> +			if (domulti)
> +				errx(1, "-c conflicts with -m");
>  			break;
>  		case 'm':
>  			domulti = 1;
> -			dochar = 0;
> +			if (dochar)
> +				errx(1, "-m conflicts with -c");
>  			break;
>  		case '?':
>  		default:
> _at__at_ -263,6 +275,6 _at__at_ word:	gotsp = 1;
>  static void
>  usage()
>  {
> -	(void)fprintf(stderr, "usage: wc [-clmw] [file ...]\n");
> +	(void)fprintf(stderr, "usage: wc [-c|-m][-lw] [file ...]\n");
>  	exit(1);
>  }
> 

First of all, your patch removes functionality in our wc that deviates
from POSIX in that the last -c or -m option takes precedence over the
priors. This functionality enables there to be something like a "wc -c"
alias to "wc", and then if the user executes "wc -m" they will still get
what they want.

Yes, the POSIX manual says that you cannot specify these two options
together. No, *I* am not going to "fix" our behavior, because I don't
care about that particular case. If someone else does (with a commit bit
who is willing to take the fall), let them speak up. I also don't
personally care about adding long options to all of the core tools,
especially the ones that only have 4 or 5 options.

Second (as is obvious), you are merely posting this out of spite, and
not because you actually have any interest in improving the tools
through adding strict POSIX conformance and long options. Don't send me
any more patches.

Surely you have something better to do with your time.

-- 
Coleman Kane

Received on Thu Jun 05 2008 - 12:17:27 UTC