Re: [PATCH 07/25] parse-options: make some arguments optional, add callbacks.

From: Pierre Habouzit <madcoder@debian.org>
Date: Oct 16, 2007, 16:50 UTC
Message-ID: <20071016165045.GB13946@artemis.corp>
In-Reply-To: <4714E90C.80305@lsrfire.ath.cx>

On Tue, Oct 16, 2007 at 04:38:36PM +0000, René Scharfe wrote:

Show 14 quoted lines

> Pierre Habouzit schrieb:
> > This bit is to allow to aggregate options with arguments together when
> > the argument is numeric.
> > 
> >     +#if 0
> >     +		/* can be used to understand -A1B1 like -A1 -B1 */
> >     +		if (flag & OPT_SHORT && opt->opt && isdigit(*opt->opt)) {
> >     +			*(int *)opt->value = strtol(opt->opt, (char **)&opt->opt, 10);
> >     +			return 0;
> >     +		}
> >     +#endif
> 
> I don't like it, it complicates number options with unit suffixes (e.g.
> --windows-memory of git-pack-objects).

  Oh yeah, you're right, well you example is not an issue, but indeed
you pointed out a real probable issue:

  With that chunk, if an option that takes now an integer, becomes an
option with a suffix, we would then break backward compatibility. I'm
not sure I'm clear, but if you had a -S option (for size) that for now
gets sizes in kilooctet, git foo -S1000. Then if we decide that it's
worth understanting M/G/.. suffixes for this option, we would make the
type of the option be a CALLBACK using git_parse_ulong. This would mean
that with such a change:

before:
  git foo -S1000a would mean git foo -S1000 -a

After:
  git foo -S1000a would be rejected because 'a' isn't a valid size
  suffix.

  This of course become worse if we take git foo -S1000k where the
breakage would be silent.

  This is a very strong argument _against_ this chunk IMO.

-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org