On Fri, Jun 14, 2019 at 7:30 AM Felipe Contreras <felipe.contreras@xxxxxxxxx> wrote: > > On Wed, Jun 12, 2019 at 3:52 AM Duy Nguyen <pclouds@xxxxxxxxx> wrote: > > > > On Sat, Jun 8, 2019 at 12:33 AM Felipe Contreras > > <felipe.contreras@xxxxxxxxx> 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. Is there any good reason to complete options when they are not going to work anyway (e.g. like checkout which needs $GIT_DIR)? Besides "it used to work before --git-completion-helper" which I don't consider a good reason given the maintenance tradeoff of --git-completion-helper. -- Duy