Re: [PATCH] completion: do not cache if --git-completion-helper fails

>
> On Wed, Jun 12, 2019 at 3:52 AM Duy Nguyen <pclouds@gmail.com> wrote:
> >
> > On Sat, Jun 8, 2019 at 12:33 AM Felipe Contreras
> > <felipe.contreras@gmail.com> wrote:
>
> > > Something like this should work:
> > >
> > > struct command checkout_command = {
> > > .name = "checkout",
> > > .function = cmd_checkout,
> > > .run_options = RUN_SETUP | NEED_WORK_TREE,
> > > .help = N_("Switch branches or restore working tree files"),
> > > .options = {
> > > OPT__QUIET(&opts.quiet, N_("suppress progress reporting")),
> > > ...
> > > },
> > > }
> > >
> > > This way we could run parse_options_show_gitcomp() from git.c and not
> > > worry about whatever cmd_checkout() needs.
> >
> > This only works for a few commands. Those with subcommands already
> > have struct option[] array scattered in different places. And some new
> > ones also have struct option array dynamically created.
> >
> > It's not impossible to do. But I feel there's a lot of reorganizing
> > for little gain. Maybe when we pass 'struct repository *' to all
> > commands, which means we hit all commmands at once anyway, we can
> > reconsider this (and having config parser in a more declarative form
> > like cmd option parser).
>
> Well yes, there is little *functional* gain at the moment, but this
> (or some version of this) must be done eventually.
>
> For the moment we still have an issue, but I see there's already a
> hack present for '-h', maybe we can re-utilize it. Something like
> this:
>
> --- a/git.c
> +++ b/git.c
> @@ -408,6 +408,8 @@ static int run_builtin(struct cmd_struct *p, int
> argc, const char **argv)
>
>         prefix = NULL;
>         help = argc == 2 && !strcmp(argv[1], "-h");
> +       if (!help)
> +               help = argc == 2 && !strcmp(argv[1], "--git-completion-helper");
>         if (!help) {
>                 if (p->option & RUN_SETUP)
>                         prefix = setup_git_directory();
>
> One way or the other, shouldn't my tests be merged? The issue is still
> there, and it's nice to have tests for that.