Hi,
On Mon, 1 Oct 2007, Kristian H?gsberg wrote:
quoted text > On Sun, 2007-09-30 at 15:11 +0200, Jonas Fonseca wrote:
>
> > One of the last things I miss from Cogito is the nice abbreviated help
> > messages that was available via '-h'. I don't know if it would be
> > acceptable (at least for the main porcelain commands) to put this
> > functionality into the option parser by adding a "description" member
> > to struct option and have parse_options print a nice:
> >
> > <error message if any>
> > <usage string>
> > <option summary>
> >
> > on failure, or, if that is regarded as too verbose, simply when -h is
> > detected.
>
> Yeah, that might be nice. We can add it in a follow-on patch, if the
> list agrees that it's a good thing, I guess.
That's a good idea; I would put the usage string there, too.
quoted text > > > +
> > > +/* Parse the given options against the list of known options. The
> > > + * order of the option structs matters, in that ambiguous
> > > + * abbreviations (eg, --in could be short for --include or
> > > + * --interactive) are matched by the first option that share the
> > > + * prefix.
> > > + */
> >
> > This prefix aware option parsing has not been ported over to the other
> > builtins when they were lifted from shell code. It might be nice to
> > have of course. Is it really needed?
>
> I don't ever use it myself and I think it's more confusing than helpful.
> I only added it to avoid introducing behavior changes in the port. I
> don't have strong feelings either way.
It might be convenient, but I think that it is really more confusing than
helpful, especially with options that share a prefix. Besides, we have
good completion for bash now (and I hear that this "zsh" thing also has
quite good completion), I recommend <TAB> over prefix DWIMery.
quoted text > > > +
> > > +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.
Ciao,
Dscho
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to
majordomo@vger.kernel.org
More majordomo info at
http://vger.kernel.org/majordomo-info.html