Re: [PATCH 5/5] diff: parse U/u/unified options with an optional integer arg

From: Eric Wong <normalperson@yhbt.net>
Date: May 14, 2006, 23:36 UTC
Message-ID: <20060514233606.GA10249@hand.yhbt.net>
In-Reply-To: <Pine.LNX.4.64.0605140931410.3866@g5.osdl.org>

Linus Torvalds <torvalds@osdl.org> wrote:

Show 18 quoted lines

> 
> 
> On Sun, 14 May 2006, Eric Wong wrote:
> > 
> > -u (lowercase) now accepts an optional arg, like -U (GNU diff
> > -u also has this behavior).
> 
> Actually, modern GNU "diff -u5" will say
> 
> 	diff: `-5' option is obsolete; use `-U 5'
> 	diff: Try `diff --help' for more information.
> 
> and exit.
> 
> I'm not entirely sure why, but I think it's because "u" can be mixed (ie 
> with something like "-urN"), while "U" cannot. The GNU diff rule seems to 
> be that simple arguments can be mixed together, but arguments with 
> parameters cannot. Which sounds sane.

Interesting. With _POSIX2_VERSION < 200112, diff is happy to accept -u5. Setting _POSIX2_VERSION >= 200112 causes it to burp, however.

It turns out -u5 is actually short for -u -5, but I (and apparently some other people) have always assumed -u5 worked like -U5. POSIX doesn't like -<num> switches, however, but we support it regardless in rev-list.

I prefer that it works the -u5 way, of course :)

Also, I find integer arguments being bundled with other parameters to be pretty nice feature, but it's easy to disable if users don't like it:

diff --git a/gitopt.c b/gitopt.c
index 9e85247..139cd2d 100644
--- a/gitopt.c
+++ b/gitopt.c
@@ -270,17 +270,9 @@ static const char * parse_bundled(struct
        } else if (s->arg_fl) {
                if (*cur) {
                        /* no space between the arg and opt switch: */
-                       if (s->arg_fl & ARG_IS_INT) {
-                               /* we know to handle stuff like:
-                                * -h24w80 => -h=24 -w=80 */
-                               char *endptr = (char *)cur;
-                               strtol(cur, &endptr, 10);
-
-                               while (cur < endptr)
-                                       *c++ = *cur++;
-                       } else if (s->arg_fl & ARG_ONE) {
-                               /* unfortunately, other args are less
-                                * clear-cut */
+                       if (s->arg_fl & ARG_ONE) {
+                               /* don't attempt further unbundling, extra
+                                * chars are arguments */
                                while (*cur)
                                        *c++ = *cur++;
                        }

-- 
Eric Wong