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

[Duy Nguyen <pclouds@gmail.com>]

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< --