On Mon, Jul 25, 2022 at 04:31:40PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
> On Mon, Jul 25 2022, SZEDER Gábor wrote:
>
> > Our Bash completion script can complete --options for commands using
> > parse-options even when that command doesn't have a dedicated
> > completion function, but to do so the completion script must know
> > which commands use parse-options and which don't. Therefore, commands
> > not using parse-options are marked in 'git.c's command list with the
> > NO_PARSEOPT flag.
> >
> > Update this list, and remove this flag from the commands that by now
> > use parse-options.
> >
> > Signed-off-by: SZEDER Gábor <szeder.dev@xxxxxxxxx>
> > ---
> > git.c | 12 ++++++------
> > 1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/git.c b/git.c
> > index e5d62fa5a9..c4282f194a 100644
> > --- a/git.c
> > +++ b/git.c
> > @@ -489,14 +489,14 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
> > static struct cmd_struct commands[] = {
> > { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
> > { "am", cmd_am, RUN_SETUP | NEED_WORK_TREE },
> > - { "annotate", cmd_annotate, RUN_SETUP | NO_PARSEOPT },
> > + { "annotate", cmd_annotate, RUN_SETUP },
> > { "apply", cmd_apply, RUN_SETUP_GENTLY },
> > { "archive", cmd_archive, RUN_SETUP_GENTLY },
> > { "bisect--helper", cmd_bisect__helper, RUN_SETUP },
> > { "blame", cmd_blame, RUN_SETUP },
> > { "branch", cmd_branch, RUN_SETUP | DELAY_PAGER_CONFIG },
> > { "bugreport", cmd_bugreport, RUN_SETUP_GENTLY },
> > - { "bundle", cmd_bundle, RUN_SETUP_GENTLY | NO_PARSEOPT },
> > + { "bundle", cmd_bundle, RUN_SETUP_GENTLY },
> > { "cat-file", cmd_cat_file, RUN_SETUP },
> > { "check-attr", cmd_check_attr, RUN_SETUP },
> > { "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
> > @@ -514,7 +514,7 @@ static struct cmd_struct commands[] = {
> > { "column", cmd_column, RUN_SETUP_GENTLY },
> > { "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
> > { "commit-graph", cmd_commit_graph, RUN_SETUP },
> > - { "commit-tree", cmd_commit_tree, RUN_SETUP | NO_PARSEOPT },
> > + { "commit-tree", cmd_commit_tree, RUN_SETUP },
> > { "config", cmd_config, RUN_SETUP_GENTLY | DELAY_PAGER_CONFIG },
> > { "count-objects", cmd_count_objects, RUN_SETUP },
> > { "credential", cmd_credential, RUN_SETUP_GENTLY | NO_PARSEOPT },
> > @@ -553,7 +553,7 @@ static struct cmd_struct commands[] = {
> > { "ls-files", cmd_ls_files, RUN_SETUP },
> > { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
> > { "ls-tree", cmd_ls_tree, RUN_SETUP },
> > - { "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY | NO_PARSEOPT },
> > + { "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY },
> > { "mailsplit", cmd_mailsplit, NO_PARSEOPT },
> > { "maintenance", cmd_maintenance, RUN_SETUP | NO_PARSEOPT },
> > { "merge", cmd_merge, RUN_SETUP | NEED_WORK_TREE },
> > @@ -566,7 +566,7 @@ static struct cmd_struct commands[] = {
> > { "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
> > { "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
> > { "merge-tree", cmd_merge_tree, RUN_SETUP },
> > - { "mktag", cmd_mktag, RUN_SETUP | NO_PARSEOPT },
> > + { "mktag", cmd_mktag, RUN_SETUP },
> > { "mktree", cmd_mktree, RUN_SETUP },
> > { "multi-pack-index", cmd_multi_pack_index, RUN_SETUP },
> > { "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
> > @@ -627,7 +627,7 @@ static struct cmd_struct commands[] = {
> > { "verify-tag", cmd_verify_tag, RUN_SETUP },
> > { "version", cmd_version },
> > { "whatchanged", cmd_whatchanged, RUN_SETUP },
> > - { "worktree", cmd_worktree, RUN_SETUP | NO_PARSEOPT },
> > + { "worktree", cmd_worktree, RUN_SETUP },
> > { "write-tree", cmd_write_tree, RUN_SETUP },
> > };
>
> I have a superset of this change in my local tree:
> https://github.com/avar/git/commit/f0076de17fd
>
> I.e. "parseopt" here really means "handles --git-completion-helper"
Indeed, with additional subtleties, like it doesn't try to read from
stdin before handling '--git-completion-helper'.
> so
> we can also address the diff family here. See the test that's associated
> with the change (which you might want to grab in any case, i.e. to have
> tests in this area).
Dunno. I don't like this NO_PARSEOPT thing, and instead of testing it
I'm thinking about removing it altogether. It seems to be doable, but
of course conflicts with the upcoming 'completion-subcommand' patch
series, so I'll leave it for later.
https://github.com/szeder/git/commits/completion-poc-builtins
