Re: [PATCH 01/20] git.c: update NO_PARSEOPT markings

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

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@gmail.com>
> ---
>  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", 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).

But as far as this goes this looks good to me.