[PATCH] completion: add option completion for most builtin commands

[PATCH] completion: add option completion for most builtin commands

[Nguyễn Thái Ngọc Duy]

These commands can take options and use parse-options so it's quite
easy to allow option completion. This does not pollute the command
name completion though. "git <tab>" will show you the same set as
before. This only kicks in when you type the correct command name.

Some other builtin commands are not still added because either they
don't use parse-options, or they are deprecated, or they are those
-helper commands that are used to move some logic back in C for
sh-based commands.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 276 +++++++++++++++++++++++++
 1 file changed, 276 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6da95b8095..0cd9180f48 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1202,6 +1202,15 @@ _git_bisect ()
 	esac
 }
 
+_git_blame() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin blame
+		return
+		;;
+	esac
+}
+
 _git_branch ()
 {
 	local i c=1 only_local_ref="n" has_r="n"
@@ -1254,6 +1263,42 @@ _git_bundle ()
 	esac
 }
 
+_git_cat_file() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin cat-file
+		return
+		;;
+	esac
+}
+
+_git_check_attr() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin check-attr
+		return
+		;;
+	esac
+}
+
+_git_check_ignore() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin check-ignore
+		return
+		;;
+	esac
+}
+
+_git_check_mailmap() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin check-mailmap
+		return
+		;;
+	esac
+}
+
 _git_checkout ()
 {
 	__git_has_doubledash && return
@@ -1278,6 +1323,15 @@ _git_checkout ()
 	esac
 }
 
+_git_checkout_index() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin checkout-index
+		return
+		;;
+	esac
+}
+
 _git_cherry ()
 {
 	__git_complete_refs
@@ -1326,6 +1380,15 @@ _git_clone ()
 	esac
 }
 
+_git_column() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin column
+		return
+		;;
+	esac
+}
+
 __git_untracked_file_modes="all no normal"
 
 _git_commit ()
@@ -1365,6 +1428,15 @@ _git_commit ()
 	fi
 }
 
+_git_count_objects() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin count-objects
+		return
+		;;
+	esac
+}
+
 _git_describe ()
 {
 	case "$cur" in
@@ -1446,6 +1518,15 @@ _git_difftool ()
 	__git_complete_revlist_file
 }
 
+_git_fast_export() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin fast-export
+		return
+		;;
+	esac
+}
+
 __git_fetch_recurse_submodules="yes on-demand no"
 
 _git_fetch ()
@@ -1573,6 +1654,15 @@ _git_grep ()
 	__git_complete_refs
 }
 
+_git_hash_object() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin hash-object
+		return
+		;;
+	esac
+}
+
 _git_help ()
 {
 	case "$cur" in
@@ -1590,6 +1680,15 @@ _git_help ()
 		"
 }
 
+_git_index_pack() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin index-pack
+		return
+		;;
+	esac
+}
+
 _git_init ()
 {
 	case "$cur" in
@@ -1606,6 +1705,15 @@ _git_init ()
 	esac
 }
 
+_git_interpret_trailers() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin interpret-trailers
+		return
+		;;
+	esac
+}
+
 _git_ls_files ()
 {
 	case "$cur" in
@@ -1765,6 +1873,15 @@ _git_merge ()
 	__git_complete_refs
 }
 
+_git_merge_file() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin merge-file
+		return
+		;;
+	esac
+}
+
 _git_mergetool ()
 {
 	case "$cur" in
@@ -1790,6 +1907,15 @@ _git_merge_base ()
 	__git_complete_refs
 }
 
+_git_mktree() {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin mktree
+		return
+		;;
+	esac
+}
+
 _git_mv ()
 {
 	case "$cur" in
@@ -1853,6 +1979,46 @@ _git_notes ()
 	esac
 }
 
+_git_pack_objects ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin pack-objects
+		return
+		;;
+	esac
+}
+
+_git_pack_refs ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin pack-refs
+		return
+		;;
+	esac
+}
+
+_git_prune ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin prune
+		return
+		;;
+	esac
+}
+
+_git_prune_packed ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin prune-packed
+		return
+		;;
+	esac
+}
+
 _git_pull ()
 {
 	__git_complete_strategy && return
@@ -1925,6 +2091,16 @@ _git_push ()
 	__git_complete_remote_or_refspec
 }
 
+_git_read_tree ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin read-tree
+		return
+		;;
+	esac
+}
+
 _git_rebase ()
 {
 	__git_find_repo_path
@@ -1974,6 +2150,16 @@ _git_reflog ()
 	fi
 }
 
+_git_repack ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin repack
+		return
+		;;
+	esac
+}
+
 __git_send_email_confirm_options="always never auto cc compose"
 __git_send_email_suppresscc_options="author self cc bodycc sob cccmd body all"
 
@@ -2031,6 +2217,26 @@ _git_send_email ()
 	__git_complete_revlist
 }
 
+_git_send_pack ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin send-pack
+		return
+		;;
+	esac
+}
+
+_git_show_ref ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin show-ref
+		return
+		;;
+	esac
+}
+
 _git_stage ()
 {
 	_git_add
@@ -2082,6 +2288,26 @@ _git_status ()
 	__git_complete_index_file "$complete_opt"
 }
 
+_git_stripspace ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin stripspace
+		return
+		;;
+	esac
+}
+
+_git_symbolic_ref ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin symbolic-ref
+		return
+		;;
+	esac
+}
+
 __git_config_get_set_variables ()
 {
 	local prevword word config_file= c=$cword
@@ -2998,6 +3224,46 @@ _git_tag ()
 	esac
 }
 
+_git_update_index ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin update-index
+		return
+		;;
+	esac
+}
+
+_git_update_ref ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin update-ref
+		return
+		;;
+	esac
+}
+
+_git_verify_commit ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin verify-commit
+		return
+		;;
+	esac
+}
+
+_git_verify_tag ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin verify-tag
+		return
+		;;
+	esac
+}
+
 _git_whatchanged ()
 {
 	_git_log
@@ -3032,6 +3298,16 @@ _git_worktree ()
 	fi
 }
 
+_git_write_tree ()
+{
+	case "$cur" in
+	--*)
+		__gitcomp_builtin write-tree
+		return
+		;;
+	esac
+}
+
 __git_main ()
 {
 	local i c=1 command __git_dir __git_repo_path
-- 
2.17.0.rc0.348.gd5a49e0b6f

Re: [PATCH] completion: add option completion for most builtin commands

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> These commands can take options and use parse-options so it's quite
> easy to allow option completion. This does not pollute the command
> name completion though. "git <tab>" will show you the same set as
> before. This only kicks in when you type the correct command name.
>
> Some other builtin commands are not still added because either they
> don't use parse-options, or they are deprecated, or they are those
> -helper commands that are used to move some logic back in C for
> sh-based commands.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 276 +++++++++++++++++++++++++
>  1 file changed, 276 insertions(+)

Wow, just wow.  It however looks a lot of boilerplates, e.g. looking
at these, we notice ...

> +_git_blame() {
> +	case "$cur" in
> +	--*)
> +		__gitcomp_builtin blame
> +		return
> +		;;
> +	esac
> +}
> +
>  
> +_git_cat_file() {
> +	case "$cur" in
> +	--*)
> +		__gitcomp_builtin cat-file
> +		return
> +		;;
> +	esac
> +}
> +
> +_git_check_attr() {
> +	case "$cur" in
> +	--*)
> +		__gitcomp_builtin check-attr
> +		return
> +		;;
> +	esac
> +}

... the only thing we need for the above three is a table that says
"use blame for blame, cat-file for cat_file, and check-attr for
check_attr".

And that pattern repeats throughout the patch.  I wonder if we can
express the same a lot more concisely by updating the caller that
calls these command specific helpers?

Re: [PATCH] completion: add option completion for most builtin commands

[Duy Nguyen]

On Wed, Mar 21, 2018 at 9:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
>
>> These commands can take options and use parse-options so it's quite
>> easy to allow option completion. This does not pollute the command
>> name completion though. "git <tab>" will show you the same set as
>> before. This only kicks in when you type the correct command name.
>>
>> Some other builtin commands are not still added because either they
>> don't use parse-options, or they are deprecated, or they are those
>> -helper commands that are used to move some logic back in C for
>> sh-based commands.
>>
>> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>> ---
>>  contrib/completion/git-completion.bash | 276 +++++++++++++++++++++++++
>>  1 file changed, 276 insertions(+)
>
> Wow, just wow.  It however looks a lot of boilerplates, e.g. looking
> at these, we notice ...
>
>> +_git_blame() {
>> +     case "$cur" in
>> +     --*)
>> +             __gitcomp_builtin blame
>> +             return
>> +             ;;
>> +     esac
>> +}
>> +
>>
>> +_git_cat_file() {
>> +     case "$cur" in
>> +     --*)
>> +             __gitcomp_builtin cat-file
>> +             return
>> +             ;;
>> +     esac
>> +}
>> +
>> +_git_check_attr() {
>> +     case "$cur" in
>> +     --*)
>> +             __gitcomp_builtin check-attr
>> +             return
>> +             ;;
>> +     esac
>> +}
>
> ... the only thing we need for the above three is a table that says
> "use blame for blame, cat-file for cat_file, and check-attr for
> check_attr".
>
> And that pattern repeats throughout the patch.  I wonder if we can
> express the same a lot more concisely by updating the caller that
> calls these command specific helpers?
>

Yeah. I almost went to just generate and eval these functions. But we
still need to keep a list of "bultin with --git-completion-helper"
support somewhere, and people may want to complete arguments without
double dashes (e.g. read-tree should take a ref...) which can't be
helped by --git-completion-helper.
-- 
Duy

Re: [PATCH] completion: add option completion for most builtin commands

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

>> And that pattern repeats throughout the patch.  I wonder if we can
>> express the same a lot more concisely by updating the caller that
>> calls these command specific helpers?
>
> Yeah. I almost went to just generate and eval these functions. But we
> still need to keep a list of "bultin with --git-completion-helper"
> support somewhere, and people may want to complete arguments without
> double dashes (e.g. read-tree should take a ref...) which can't be
> helped by --git-completion-helper.

Hmph, I actually did not have 'eval' in mind.

Rather, I was wondering if it is cleaner to update __git_main where
it computes $completion_func by mangling $command and then calls
it---instead call __gitcomp_builtin directly when the $command
appears in such a "list of builtins that knows --completion-helper
and no custom completion".

Re: [PATCH] completion: add option completion for most builtin commands

[Duy Nguyen]

On Thu, Mar 22, 2018 at 10:11:53AM -0700, Junio C Hamano wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
> 
> >> And that pattern repeats throughout the patch.  I wonder if we can
> >> express the same a lot more concisely by updating the caller that
> >> calls these command specific helpers?
> >
> > Yeah. I almost went to just generate and eval these functions. But we
> > still need to keep a list of "bultin with --git-completion-helper"
> > support somewhere, and people may want to complete arguments without
> > double dashes (e.g. read-tree should take a ref...) which can't be
> > helped by --git-completion-helper.
> 
> Hmph, I actually did not have 'eval' in mind.
> 
> Rather, I was wondering if it is cleaner to update __git_main where
> it computes $completion_func by mangling $command and then calls
> it---instead call __gitcomp_builtin directly when the $command
> appears in such a "list of builtins that knows --completion-helper
> and no custom completion".

Ah. Something like this? Seems to work fine and definitely not as ugly
as eval.

-- 8< --
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 6da95b8095..960e26e09d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3032,6 +3032,32 @@ _git_worktree ()
 	fi
 }
 
+__git_main_with_parseopt_helper='
+	blame cat-file check-attr check-ignore
+	check-mailmap checkout-index column count-objects fast-export
+	hash-object index-pack interpret-trailers merge-file mktree
+	pack-objects pack-refs prune prune-packed read-tree repack
+	send-pack show-ref stripspace symbolic-ref update-index
+	update-ref verify-commit verify-tag write-tree
+'
+__git_complete_command() {
+	local command="$1"
+	local completion_func="_git_${command//-/_}"
+	if declare -f $completion_func >/dev/null 2>/dev/null; then
+		$completion_func
+	elif echo $__git_main_with_parseopt_helper | git grep -w "$command" >/dev/null; then
+		case "$cur" in
+		--*)
+			__gitcomp_builtin "$command"
+			return 0
+			;;
+		esac
+		return 0
+	else
+		return 1
+	fi
+}
+
 __git_main ()
 {
 	local i c=1 command __git_dir __git_repo_path
@@ -3091,14 +3117,12 @@ __git_main ()
 		return
 	fi
 
-	local completion_func="_git_${command//-/_}"
-	declare -f $completion_func >/dev/null 2>/dev/null && $completion_func && return
+	__git_complete_command "$command" && return
 
 	local expansion=$(__git_aliased_command "$command")
 	if [ -n "$expansion" ]; then
 		words[1]=$expansion
-		completion_func="_git_${expansion//-/_}"
-		declare -f $completion_func >/dev/null 2>/dev/null && $completion_func
+		__git_complete_command "$expansion"
 	fi
 }
 
-- 8< --

Re: [PATCH] completion: add option completion for most builtin commands

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> +__git_main_with_parseopt_helper='
> +	blame cat-file check-attr check-ignore
> +	check-mailmap checkout-index column count-objects fast-export
> +	hash-object index-pack interpret-trailers merge-file mktree
> +	pack-objects pack-refs prune prune-packed read-tree repack
> +	send-pack show-ref stripspace symbolic-ref update-index
> +	update-ref verify-commit verify-tag write-tree
> +'
> +__git_complete_command() {
> +	local command="$1"
> +	local completion_func="_git_${command//-/_}"
> +	if declare -f $completion_func >/dev/null 2>/dev/null; then
> +		$completion_func
> +	elif echo $__git_main_with_parseopt_helper | git grep -w "$command" >/dev/null; then

"git grep"???

I imagined that you'd keep an associative shell array (we are not
constrained by POSIX here) that can be used like so

	elif test -n "${__git_main_with_parseopt_helper[$command]}"; then

Of course, a more traditional way to write it without spawning grep
or pipe is

	case " $__git_main_with_parseopt_helper " in
	*" $command "*)
		... Yes, $command is on the list ...
		;;
	*)
		... No, $command is not on the list ...
		;;
	esac

Re: [PATCH] completion: add option completion for most builtin commands

[Duy Nguyen]

On Thu, Mar 22, 2018 at 6:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> +__git_main_with_parseopt_helper='
>> +     blame cat-file check-attr check-ignore
>> +     check-mailmap checkout-index column count-objects fast-export
>> +     hash-object index-pack interpret-trailers merge-file mktree
>> +     pack-objects pack-refs prune prune-packed read-tree repack
>> +     send-pack show-ref stripspace symbolic-ref update-index
>> +     update-ref verify-commit verify-tag write-tree
>> +'
>> +__git_complete_command() {
>> +     local command="$1"
>> +     local completion_func="_git_${command//-/_}"
>> +     if declare -f $completion_func >/dev/null 2>/dev/null; then
>> +             $completion_func
>> +     elif echo $__git_main_with_parseopt_helper | git grep -w "$command" >/dev/null; then
>
> "git grep"???

GIt has taught my fingers some bad habit.

>
> I imagined that you'd keep an associative shell array (we are not
> constrained by POSIX here) that can be used like so
>
>         elif test -n "${__git_main_with_parseopt_helper[$command]}"; then
>

Great. We could even kill two existing _git_xxx functions because they
are too simple they could be replaced with this new code. I'll send
out a series later.
-- 
Duy

Re: [PATCH] completion: add option completion for most builtin commands

[Duy Nguyen]

On Thu, Mar 22, 2018 at 6:56 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> +__git_main_with_parseopt_helper='
>> +     blame cat-file check-attr check-ignore
>> +     check-mailmap checkout-index column count-objects fast-export
>> +     hash-object index-pack interpret-trailers merge-file mktree
>> +     pack-objects pack-refs prune prune-packed read-tree repack
>> +     send-pack show-ref stripspace symbolic-ref update-index
>> +     update-ref verify-commit verify-tag write-tree
>> +'
>> +__git_complete_command() {
>> +     local command="$1"
>> +     local completion_func="_git_${command//-/_}"
>> +     if declare -f $completion_func >/dev/null 2>/dev/null; then
>> +             $completion_func
>> +     elif echo $__git_main_with_parseopt_helper | git grep -w "$command" >/dev/null; then
>
> "git grep"???
>
> I imagined that you'd keep an associative shell array (we are not
> constrained by POSIX here) that can be used like so
>
>         elif test -n "${__git_main_with_parseopt_helper[$command]}"; then

Nope. We are not constrained by POSIX, but MacOS still runs the
ancient bash 3.x while associative arrays are in 4.x
-- 
Duy

[PATCH v2 0/8] completion: add option completion for most builtin commands

[Nguyễn Thái Ngọc Duy]

This series adds option completion for many more commands. More
importantly it introduces a generic completion function
__git_complete_common(). With --git-completion-helper providing more
and more information in future, this function can start to replace
many other _git_xxx() in this file.

Nguyễn Thái Ngọc Duy (8):
  git.c: move cmd_struct declaration up
  git.c: add hidden option --list-parseopt-builtins
  completion: mention the oldest version we need to support
  completion: factor out _git_xxx calling code
  completion: add --option completion for most builtin commands
  completion: delete option-only completion commands
  completion: use __gitcomp_builtin in _git_ls_tree
  completion: use __gitcomp_builtin in _git_cherry

 contrib/completion/git-completion.bash |  75 +++++++++++----
 git.c                                  | 122 +++++++++++++------------
 t/t9902-completion.sh                  |   6 ++
 3 files changed, 127 insertions(+), 76 deletions(-)

-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH v2 1/8] git.c: move cmd_struct declaration up

[Nguyễn Thái Ngọc Duy]

In a later patch we need access to one of these command option
constants near the top of this file. Move this block up so we will be
able to access the command options.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 git.c | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/git.c b/git.c
index ceaa58ef40..bc4a5459d3 100644
--- a/git.c
+++ b/git.c
@@ -4,6 +4,23 @@
 #include "help.h"
 #include "run-command.h"
 
+#define RUN_SETUP		(1<<0)
+#define RUN_SETUP_GENTLY	(1<<1)
+#define USE_PAGER		(1<<2)
+/*
+ * require working tree to be present -- anything uses this needs
+ * RUN_SETUP for reading from the configuration file.
+ */
+#define NEED_WORK_TREE		(1<<3)
+#define SUPPORT_SUPER_PREFIX	(1<<4)
+#define DELAY_PAGER_CONFIG	(1<<5)
+
+struct cmd_struct {
+	const char *cmd;
+	int (*fn)(int, const char **, const char *);
+	int option;
+};
+
 const char git_usage_string[] =
 	N_("git [--version] [--help] [-C <path>] [-c <name>=<value>]\n"
 	   "           [--exec-path[=<path>]] [--html-path] [--man-path] [--info-path]\n"
@@ -288,23 +305,6 @@ static int handle_alias(int *argcp, const char ***argv)
 	return ret;
 }
 
-#define RUN_SETUP		(1<<0)
-#define RUN_SETUP_GENTLY	(1<<1)
-#define USE_PAGER		(1<<2)
-/*
- * require working tree to be present -- anything uses this needs
- * RUN_SETUP for reading from the configuration file.
- */
-#define NEED_WORK_TREE		(1<<3)
-#define SUPPORT_SUPER_PREFIX	(1<<4)
-#define DELAY_PAGER_CONFIG	(1<<5)
-
-struct cmd_struct {
-	const char *cmd;
-	int (*fn)(int, const char **, const char *);
-	int option;
-};
-
 static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 {
 	int status, help;
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH v2 2/8] git.c: add hidden option --list-parseopt-builtins

[Nguyễn Thái Ngọc Duy]

This is another step to help automate git-completion.bash. This option
gives a list of all builtin commands that do use parse_options(),
which supports another hidden option --git-completion-helper. The
output is prepared for easy consumption by git-completion.bash and
separates items by spaces instead of \n

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 git.c | 90 ++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 49 insertions(+), 41 deletions(-)

diff --git a/git.c b/git.c
index bc4a5459d3..3a89893712 100644
--- a/git.c
+++ b/git.c
@@ -14,11 +14,12 @@
 #define NEED_WORK_TREE		(1<<3)
 #define SUPPORT_SUPER_PREFIX	(1<<4)
 #define DELAY_PAGER_CONFIG	(1<<5)
+#define NO_PARSEOPT		(1<<6) /* parse-options is not used */
 
 struct cmd_struct {
 	const char *cmd;
 	int (*fn)(int, const char **, const char *);
-	int option;
+	unsigned int option;
 };
 
 const char git_usage_string[] =
@@ -35,7 +36,7 @@ const char git_more_info_string[] =
 
 static int use_pager = -1;
 
-static void list_builtins(void);
+static void list_builtins(unsigned int exclude_option, char sep);
 
 static void commit_pager_choice(void) {
 	switch (use_pager) {
@@ -223,7 +224,10 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
 			(*argv)++;
 			(*argc)--;
 		} else if (!strcmp(cmd, "--list-builtins")) {
-			list_builtins();
+			list_builtins(0, '\n');
+			exit(0);
+		} else if (!strcmp(cmd, "--list-parseopt-builtins")) {
+			list_builtins(NO_PARSEOPT, ' ');
 			exit(0);
 		} else {
 			fprintf(stderr, _("unknown option: %s\n"), cmd);
@@ -367,18 +371,18 @@ 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 },
+	{ "annotate", cmd_annotate, RUN_SETUP | NO_PARSEOPT },
 	{ "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 },
-	{ "bundle", cmd_bundle, RUN_SETUP_GENTLY },
+	{ "bundle", cmd_bundle, RUN_SETUP_GENTLY | NO_PARSEOPT },
 	{ "cat-file", cmd_cat_file, RUN_SETUP },
 	{ "check-attr", cmd_check_attr, RUN_SETUP },
 	{ "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
 	{ "check-mailmap", cmd_check_mailmap, RUN_SETUP },
-	{ "check-ref-format", cmd_check_ref_format },
+	{ "check-ref-format", cmd_check_ref_format, NO_PARSEOPT  },
 	{ "checkout", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
 	{ "checkout-index", cmd_checkout_index,
 		RUN_SETUP | NEED_WORK_TREE},
@@ -388,30 +392,30 @@ static struct cmd_struct commands[] = {
 	{ "clone", cmd_clone },
 	{ "column", cmd_column, RUN_SETUP_GENTLY },
 	{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
-	{ "commit-tree", cmd_commit_tree, RUN_SETUP },
+	{ "commit-tree", cmd_commit_tree, RUN_SETUP | NO_PARSEOPT },
 	{ "config", cmd_config, RUN_SETUP_GENTLY | DELAY_PAGER_CONFIG },
 	{ "count-objects", cmd_count_objects, RUN_SETUP },
-	{ "credential", cmd_credential, RUN_SETUP_GENTLY },
+	{ "credential", cmd_credential, RUN_SETUP_GENTLY | NO_PARSEOPT },
 	{ "describe", cmd_describe, RUN_SETUP },
-	{ "diff", cmd_diff },
-	{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },
-	{ "diff-index", cmd_diff_index, RUN_SETUP },
-	{ "diff-tree", cmd_diff_tree, RUN_SETUP },
+	{ "diff", cmd_diff, NO_PARSEOPT },
+	{ "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
+	{ "diff-index", cmd_diff_index, RUN_SETUP | NO_PARSEOPT },
+	{ "diff-tree", cmd_diff_tree, RUN_SETUP | NO_PARSEOPT },
 	{ "difftool", cmd_difftool, RUN_SETUP | NEED_WORK_TREE },
 	{ "fast-export", cmd_fast_export, RUN_SETUP },
 	{ "fetch", cmd_fetch, RUN_SETUP },
-	{ "fetch-pack", cmd_fetch_pack, RUN_SETUP },
+	{ "fetch-pack", cmd_fetch_pack, RUN_SETUP | NO_PARSEOPT },
 	{ "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
 	{ "for-each-ref", cmd_for_each_ref, RUN_SETUP },
 	{ "format-patch", cmd_format_patch, RUN_SETUP },
 	{ "fsck", cmd_fsck, RUN_SETUP },
 	{ "fsck-objects", cmd_fsck, RUN_SETUP },
 	{ "gc", cmd_gc, RUN_SETUP },
-	{ "get-tar-commit-id", cmd_get_tar_commit_id },
+	{ "get-tar-commit-id", cmd_get_tar_commit_id, NO_PARSEOPT },
 	{ "grep", cmd_grep, RUN_SETUP_GENTLY },
 	{ "hash-object", cmd_hash_object },
 	{ "help", cmd_help },
-	{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
+	{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT },
 	{ "init", cmd_init_db },
 	{ "init-db", cmd_init_db },
 	{ "interpret-trailers", cmd_interpret_trailers, RUN_SETUP_GENTLY },
@@ -419,27 +423,27 @@ 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 },
-	{ "mailsplit", cmd_mailsplit },
+	{ "mailinfo", cmd_mailinfo, RUN_SETUP_GENTLY | NO_PARSEOPT },
+	{ "mailsplit", cmd_mailsplit, NO_PARSEOPT },
 	{ "merge", cmd_merge, RUN_SETUP | NEED_WORK_TREE },
 	{ "merge-base", cmd_merge_base, RUN_SETUP },
 	{ "merge-file", cmd_merge_file, RUN_SETUP_GENTLY },
-	{ "merge-index", cmd_merge_index, RUN_SETUP },
-	{ "merge-ours", cmd_merge_ours, RUN_SETUP },
-	{ "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
-	{ "merge-recursive-ours", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
-	{ "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
-	{ "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
-	{ "merge-tree", cmd_merge_tree, RUN_SETUP },
-	{ "mktag", cmd_mktag, RUN_SETUP },
+	{ "merge-index", cmd_merge_index, RUN_SETUP | NO_PARSEOPT },
+	{ "merge-ours", cmd_merge_ours, RUN_SETUP | NO_PARSEOPT },
+	{ "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
+	{ "merge-recursive-ours", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE | NO_PARSEOPT },
+	{ "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 | NO_PARSEOPT },
+	{ "mktag", cmd_mktag, RUN_SETUP | NO_PARSEOPT },
 	{ "mktree", cmd_mktree, RUN_SETUP },
 	{ "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
 	{ "name-rev", cmd_name_rev, RUN_SETUP },
 	{ "notes", cmd_notes, RUN_SETUP },
 	{ "pack-objects", cmd_pack_objects, RUN_SETUP },
-	{ "pack-redundant", cmd_pack_redundant, RUN_SETUP },
+	{ "pack-redundant", cmd_pack_redundant, RUN_SETUP | NO_PARSEOPT },
 	{ "pack-refs", cmd_pack_refs, RUN_SETUP },
-	{ "patch-id", cmd_patch_id, RUN_SETUP_GENTLY },
+	{ "patch-id", cmd_patch_id, RUN_SETUP_GENTLY | NO_PARSEOPT },
 	{ "pickaxe", cmd_blame, RUN_SETUP },
 	{ "prune", cmd_prune, RUN_SETUP },
 	{ "prune-packed", cmd_prune_packed, RUN_SETUP },
@@ -450,14 +454,14 @@ static struct cmd_struct commands[] = {
 	{ "receive-pack", cmd_receive_pack },
 	{ "reflog", cmd_reflog, RUN_SETUP },
 	{ "remote", cmd_remote, RUN_SETUP },
-	{ "remote-ext", cmd_remote_ext },
-	{ "remote-fd", cmd_remote_fd },
+	{ "remote-ext", cmd_remote_ext, NO_PARSEOPT },
+	{ "remote-fd", cmd_remote_fd, NO_PARSEOPT },
 	{ "repack", cmd_repack, RUN_SETUP },
 	{ "replace", cmd_replace, RUN_SETUP },
 	{ "rerere", cmd_rerere, RUN_SETUP },
 	{ "reset", cmd_reset, RUN_SETUP },
-	{ "rev-list", cmd_rev_list, RUN_SETUP },
-	{ "rev-parse", cmd_rev_parse },
+	{ "rev-list", cmd_rev_list, RUN_SETUP | NO_PARSEOPT },
+	{ "rev-parse", cmd_rev_parse, NO_PARSEOPT },
 	{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
 	{ "rm", cmd_rm, RUN_SETUP },
 	{ "send-pack", cmd_send_pack, RUN_SETUP },
@@ -468,23 +472,23 @@ static struct cmd_struct commands[] = {
 	{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
 	{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
 	{ "stripspace", cmd_stripspace },
-	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},
+	{ "submodule--helper", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX | NO_PARSEOPT },
 	{ "symbolic-ref", cmd_symbolic_ref, RUN_SETUP },
 	{ "tag", cmd_tag, RUN_SETUP | DELAY_PAGER_CONFIG },
-	{ "unpack-file", cmd_unpack_file, RUN_SETUP },
-	{ "unpack-objects", cmd_unpack_objects, RUN_SETUP },
+	{ "unpack-file", cmd_unpack_file, RUN_SETUP | NO_PARSEOPT },
+	{ "unpack-objects", cmd_unpack_objects, RUN_SETUP | NO_PARSEOPT },
 	{ "update-index", cmd_update_index, RUN_SETUP },
 	{ "update-ref", cmd_update_ref, RUN_SETUP },
 	{ "update-server-info", cmd_update_server_info, RUN_SETUP },
-	{ "upload-archive", cmd_upload_archive },
-	{ "upload-archive--writer", cmd_upload_archive_writer },
-	{ "var", cmd_var, RUN_SETUP_GENTLY },
+	{ "upload-archive", cmd_upload_archive, NO_PARSEOPT },
+	{ "upload-archive--writer", cmd_upload_archive_writer, NO_PARSEOPT },
+	{ "var", cmd_var, RUN_SETUP_GENTLY | NO_PARSEOPT },
 	{ "verify-commit", cmd_verify_commit, RUN_SETUP },
 	{ "verify-pack", cmd_verify_pack },
 	{ "verify-tag", cmd_verify_tag, RUN_SETUP },
 	{ "version", cmd_version },
 	{ "whatchanged", cmd_whatchanged, RUN_SETUP },
-	{ "worktree", cmd_worktree, RUN_SETUP },
+	{ "worktree", cmd_worktree, RUN_SETUP | NO_PARSEOPT },
 	{ "write-tree", cmd_write_tree, RUN_SETUP },
 };
 
@@ -504,11 +508,15 @@ int is_builtin(const char *s)
 	return !!get_builtin(s);
 }
 
-static void list_builtins(void)
+static void list_builtins(unsigned int exclude_option, char sep)
 {
 	int i;
-	for (i = 0; i < ARRAY_SIZE(commands); i++)
-		printf("%s\n", commands[i].cmd);
+	for (i = 0; i < ARRAY_SIZE(commands); i++) {
+		if (exclude_option &&
+		    (commands[i].option & exclude_option))
+			continue;
+		printf("%s%c", commands[i].cmd, sep);
+	}
 }
 
 #ifdef STRIP_EXTENSION
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH v2 3/8] completion: mention the oldest version we need to support

[Nguyễn Thái Ngọc Duy]

This is more of a note for git-completion.bash contributors, not
users. The bash version is from MacOS [1]. Most Linux distros should
be 4.x at this point.

[1] https://public-inbox.org/git/%3CCAPig+cQXT1ov4MjzSzqiLBzr4wN1XcP7aSxMP+_dhtWtYwhDAA@mail.gmail.com%3E/

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c7957f0a90..9bef457792 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -29,6 +29,8 @@
 # tell the completion to use commit completion.  This also works with aliases
 # of form "!sh -c '...'".  For example, "!sh -c ': git commit ; ... '".
 #
+# Compatible with bash 3.2.57.
+#
 # You can set the following environment variables to influence the behavior of
 # the completion routines:
 #
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH v2 4/8] completion: factor out _git_xxx calling code

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9bef457792..b3a9ecfad0 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3034,6 +3034,17 @@ _git_worktree ()
 	fi
 }
 
+__git_complete_command () {
+	local command="$1"
+	local completion_func="_git_${command//-/_}"
+	if declare -f $completion_func >/dev/null 2>/dev/null; then
+		$completion_func
+		return 0
+	else
+		return 1
+	fi
+}
+
 __git_main ()
 {
 	local i c=1 command __git_dir __git_repo_path
@@ -3093,14 +3104,12 @@ __git_main ()
 		return
 	fi
 
-	local completion_func="_git_${command//-/_}"
-	declare -f $completion_func >/dev/null 2>/dev/null && $completion_func && return
+	__git_complete_command "$command" && return
 
 	local expansion=$(__git_aliased_command "$command")
 	if [ -n "$expansion" ]; then
 		words[1]=$expansion
-		completion_func="_git_${expansion//-/_}"
-		declare -f $completion_func >/dev/null 2>/dev/null && $completion_func
+		__git_complete_command "$expansion"
 	fi
 }
 
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH v2 5/8] completion: add --option completion for most builtin commands

[Nguyễn Thái Ngọc Duy]

Many builtin commands use parseopt which supports expose the option
list via --git-completion-helper but do not have explicit support in
git-completion.bash. This patch detects those commands and uses
__gitcomp_builtin for option completion.

This does not pollute the command name completion though. "git <tab>"
will show you the same set as before. This only kicks in when you type
the correct command name.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 28 ++++++++++++++++++++++++++
 t/t9902-completion.sh                  |  6 ++++++
 2 files changed, 34 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index b3a9ecfad0..a90b0e8db4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3034,12 +3034,40 @@ _git_worktree ()
 	fi
 }
 
+__git_complete_common () {
+	local command="$1"
+
+	case "$cur" in
+	--*)
+		__gitcomp_builtin "$command"
+		;;
+	esac
+}
+
+__git_cmds_with_parseopt_helper=
+__git_support_parseopt_helper () {
+	test -n "$__git_cmds_with_parseopt_helper" ||
+		__git_cmds_with_parseopt_helper="$(__git --list-parseopt-builtins)"
+
+	case " $__git_cmds_with_parseopt_helper " in
+	*" $1 "*)
+		return 0
+		;;
+	*)
+		return 1
+		;;
+	esac
+}
+
 __git_complete_command () {
 	local command="$1"
 	local completion_func="_git_${command//-/_}"
 	if declare -f $completion_func >/dev/null 2>/dev/null; then
 		$completion_func
 		return 0
+	elif __git_support_parseopt_helper "$command"; then
+		__git_complete_common "$command"
+		return 0
 	else
 		return 1
 	fi
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index e6485feb0a..d0a1e4c988 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -1454,6 +1454,12 @@ test_expect_success 'completion used <cmd> completion for alias: !f() { : git <c
 	EOF
 '
 
+test_expect_success 'completion without explicit _git_xxx function' '
+	test_completion "git version --" <<-\EOF
+	--build-options Z
+	EOF
+'
+
 test_expect_failure 'complete with tilde expansion' '
 	git init tmp && cd tmp &&
 	test_when_finished "cd .. && rm -rf tmp" &&
-- 
2.17.0.rc0.348.gd5a49e0b6f

Re: [PATCH v2 5/8] completion: add --option completion for most builtin commands

[Eric Sunshine]

On Sat, Mar 24, 2018 at 4:35 PM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Many builtin commands use parseopt which supports expose the option

s/expose/exposing/ maybe?

> list via --git-completion-helper but do not have explicit support in
> git-completion.bash. This patch detects those commands and uses
> __gitcomp_builtin for option completion.
>
> This does not pollute the command name completion though. "git <tab>"
> will show you the same set as before. This only kicks in when you type
> the correct command name.
>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

[PATCH v2 6/8] completion: delete option-only completion commands

[Nguyễn Thái Ngọc Duy]

The new function __git_complete_common can take over this job with
less code to maintain.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a90b0e8db4..0ee0ad7ac3 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1501,16 +1501,6 @@ _git_fsck ()
 	esac
 }
 
-_git_gc ()
-{
-	case "$cur" in
-	--*)
-		__gitcomp_builtin gc
-		return
-		;;
-	esac
-}
-
 _git_gitk ()
 {
 	_gitk
@@ -1810,11 +1800,6 @@ _git_mv ()
 	fi
 }
 
-_git_name_rev ()
-{
-	__gitcomp_builtin name-rev
-}
-
 _git_notes ()
 {
 	local subcommands='add append copy edit get-ref list merge prune remove show'
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH v2 7/8] completion: use __gitcomp_builtin in _git_ls_tree

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0ee0ad7ac3..2f16264413 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1625,6 +1625,13 @@ _git_ls_remote ()
 
 _git_ls_tree ()
 {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin ls-tree
+		return
+		;;
+	esac
+
 	__git_complete_file
 }
 
-- 
2.17.0.rc0.348.gd5a49e0b6f

[PATCH v2 8/8] completion: use __gitcomp_builtin in _git_cherry

[Nguyễn Thái Ngọc Duy]

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 contrib/completion/git-completion.bash | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2f16264413..0fe91d016f 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1282,6 +1282,12 @@ _git_checkout ()
 
 _git_cherry ()
 {
+	case "$cur" in
+	--*)
+		__gitcomp_builtin cherry
+		return
+	esac
+
 	__git_complete_refs
 }
 
-- 
2.17.0.rc0.348.gd5a49e0b6f