On Wed, 2007-10-03 at 22:11 +0200, Jonas Fonseca wrote:
> Johannes Schindelin <Johannes.Schindelin@xxxxxx> wrote Mon, Oct 01, 2007:
> > On Mon, 1 Oct 2007, Kristian H?gsberg wrote:
> > > On Sun, 2007-09-30 at 15:11 +0200, Jonas Fonseca wrote:
> > > > > +
> > > > > +extern int parse_options(const char ***argv,
> > > > > + struct option *options, int count,
> > > > > + const char *usage_string);
> > > >
> > > > I think the interface could be improved a bit. For example, it doesn't
> > > > need to count argument since the last entry in the options array is
> > > > OPTION_LAST and thus the size can be detected that way.
> > >
> > > Hehe, yeah, that's how I did it first. I don't have a strong preference
> > > for terminator elements vs. ARRAY_SIZE(), but Junio prefers the
> > > ARRAY_SIZE() approach, I guess. At this point I'm just trying the get
> > > the patches upstream...
> >
> > FWIW I like the ARRAY_SIZE() approach better, too, since it is less error
> > prone.
>
> OK, I must have missed that comment. Good point.
>
> Thanks for the comments both of you. It's great to have something to
> work from. However, I also fear it will also require that some extra
> flags or information is added to the option information to make it more
> generally usable. But I guess that is easier to discuss in the context
> of a patch.
I just sent an updated option parser patch that incorporates your
suggestions along with a patch that ports builtin-add.c to use it. I
looked briefly into porting over a few other builtins, but you're right,
we need a couple of extra features for this to be really worthwile:
* OPTION_SET_FLAG: sets the bit (we need to add a bit value that
the option parser can or in)
* OPTION_CLEAR_FLAG: clear the bit
* OPTION_ADD: adds the value to the destination integer
* OPTION_CALLBACK: calls the given function when the option is
matched. We'll need this for builtin-grep that has positional
args such as --and etc.
Also, the option parser should probably verify that a string option
isn't passed more than once. Bundling of single letter options would be
nice to add. But the patch I just sent out should be a good start, and
it lets us move forward with builtin-commit.c.
cheers,
Kristian
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
