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

On Wed, Jun 12, 2019 at 3:52 AM Duy Nguyen [off-list ref] wrote:

On Sat, Jun 8, 2019 at 12:33 AM Felipe Contreras
[off-list ref] wrote:

quoted

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.

-- 
Felipe Contreras