Re: [PATCH 1/2] parse-options: Add support for dumping out long options

["SZEDER Gábor" <szeder@ira.uka.de>]

Hi,


On Thu, Apr 12, 2012 at 12:12:32AM -0700, Stephen Boyd wrote:
> On 04/11/2012 07:06 AM, SZEDER Gábor wrote:
> > On Wed, Apr 11, 2012 at 03:29:24AM -0700, Stephen Boyd wrote:
> >> The bash completion script wants to know what the long options are for a
> >> certain command at runtime. Add a magical long option that nobody could
> >> possibly ever use (--dump-raw-long-options) to get this information.
> >>
> >> Some example output:
> >>
> >>  $ git clone --dump-raw-long-options
> >>  --no-verbose --no-quiet --progress --no-progress --no-checkout
> >>  --checkout --bare --no-bare --mirror --no-mirror --local --no-local
> >>  --no-hardlinks --hardlinks --shared --no-shared --recursive
> >>  --no-recursive --recurse-submodules --no-recurse-submodules --template=
> >>  --no-template --reference= --no-reference --origin= --no-origin
> >>  --branch= --no-branch --upload-pack= --no-upload-pack --depth=
> >>  --no-depth --single-branch --no-single-branch --separate-git-dir=
> >>  --no-separate-git-dir --config= --no-config
> >>
> > 
> > I think this is a good idea; there are many completion functions that
> > fell behind and lack an option or two.
> > 
> > However, in the completion script we deliberately miss options like
> > '--force', but with your series such options will be offered, too.
> > 
> 
> Hm.. I meant to say something about that in the commit text. I'm willing
> to live with wading through some more options when I tab complete if it
> means the script never falls out of date with my git installation.
> 
> I can envision us putting more smarts into the parse options code to
> hide certain options from the raw dump but I'm not sure how useful that
> is. Do we need that?

It's not just about wading through some more options.

Parse options already has the PARSE_OPT_HIDDEN flag to omit an option
from the default usage.  Your patch already respects that option, so
e.g. 'git commit --<TAB>' won't offer '--allow-empty', which is meant
for foreign SCM interface scripts.  This is good.

However, '--force' is different, because it should be shown in the
default usage, but since it's a "dangerous" option it should be use
with great care.  That's the reason the completion script doesn't
offer it for any of the commands.

I'm not sure whether there are any such options besides '--force',
though.  If that is the only one, then maybe this is all we need:


diff --git a/parse-options.c b/parse-options.c
index 6c37497c..1a2b0328 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -371,6 +371,8 @@ static int parse_options_raw(const struct option *opts)
 			continue;
 		if (!opts->long_name)
 			continue;
+		if (!strcmp(opts->long_name, "force"))
+			continue;
 		switch (opts->type) {
 		case OPTION_BIT:
 		case OPTION_NEGBIT:


Best,
Gábor