Re: [PATCH] Make builtin-tag.c use parse_options.

On Mon, Nov 12, 2007 at 01:09:37PM +0000, Carlos Rica wrote:

2007/11/10, Junio C Hamano [off-list ref]:

quoted

Carlos Rica [off-list ref] writes:

quoted

Also, this removes those tests ensuring that repeated
-m options don't allocate memory more than once, because now
this is done after parsing options, using the last one
when more are given. The same for -F.

The reason for this change is...?  Is this because it is
cumbersome to detect and refuse multiple -m options using the
parseopt API?  If so, the API may be what needs to be fixed.
Taking the last one and discarding earlier ones feels to me an
arbitrary choice.

You can do many things with repeated options.
Here in git-tag we considered two different ways to manage them:
Concatenating values for the option and/or refusing more than one.
I found that current option-parser can do both from the client
using callbacks, as Pierre shows me, so I think it is the right way to do it.

Pierre, by default, I think that the parser should print an error
when more than one option of the same type is given,

  I beg to differ. It makes sense for OPTION_STRING options, but not for
other. Though you cannot always detect that.

Also note that:
(1) repeating options was already silent in many git commands, so it's
    not really a regression ;
(2) for many commands it actually make sense to allow repeating (for
    _BOOLEAN e.g.). And I'd argue that for OPTION_BIT it also makes
    sense as well.

in order to report it to the command-line user, but make this
behaviour optional for the programmer.  Specifically, I thought in
this last option:

enum parse_opt_option_flags {
	PARSE_OPT_OPTARG   = 1,
	PARSE_OPT_NOARG    = 2,
	PARSE_OPT_ALLOWREP = 4
};

  To do that you need to keep a list of the triggered commands to do
that, there is no way to achieve that reliably right now. As taking the
last one and discarding the other is the usual way for option parsers I
never saw this as a big issue.

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