Re: [PATCH v2 01/41] parse-options: support --git-completion-helper

[Duy Nguyen <pclouds@gmail.com>]

On Thu, Feb 01, 2018 at 02:16:46PM -0500, Eric Sunshine wrote:
> On Thu, Feb 1, 2018 at 5:21 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> > On Thu, Feb 1, 2018 at 4:54 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> >> I don't see that as convincing argument for two classes of "no
> >> complete". Since git-completion.bash already special-cases
> >> rebase/am/cherry-pick for --continue|--abort|--skip, it is not far
> >> fetched that that special-case treatment can be extended slightly to
> >> also filter out those three options from the list returned by
> >> --git-completion-helper.
> >
> > I agree that is possible, but it's a bit tricky to do the filtering
> > right in bash (all options are sent back as one line instead of one
> > per line, which is easier to process by command line tools).
> 
> Perhaps I'm missing something, but wouldn't filtering out those
> options directly in Bash require only this?
> 
>     % x='--foo --bar --snoo'
>     % echo ${x/--bar}
>     --foo --snoo

OK how about some thing like this fixup patch? __gitcomp_builtin now
allows to add extra options as well as remove some.

-- 8< --
Subject: [PATCH] fixup! git-completion.bash: introduce __gitcomp_builtin

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index cfd24c5764..ecd5896064 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -284,24 +284,31 @@ __gitcomp ()
 #
 #    __gitcomp "$(git xxx --git-completion-helper) ..."
 #
-# except that the value from $(git) is cached
+# except that the output is cached. Accept 1-3 arguments:
+# 1: the git command to execute, this is also the cache key
+# 2: extra options to be added on top (e.g. negative forms)
+# 3: options to be excluded
 __gitcomp_builtin ()
 {
 	# spaces must be replaced with underscore for multi-word
 	# commands, e.g. "git remote add" becomes remote_add.
 	local cmd="$1"
-	shift
+	local incl="$2"
+	local excl="$3"
 
 	local var=__gitcomp_builtin_"${cmd/-/_}"
 	local options
 	eval "options=\$$var"
 
 	if [ -z "$options" ]; then
-		declare -g "$var=$(__git ${cmd/_/ } --git-completion-helper)"
-		eval "options=\$$var"
+		options="$(__git ${cmd/_/ } --git-completion-helper) $incl "
+		for i in $excl; do
+			options="${options/$i /}"
+		done
+		eval "$var=\"$options\""
 	fi
 
-	__gitcomp "$options $*"
+	__gitcomp "$options"
 }
 
 # Variation of __gitcomp_nl () that appends to the existing list of
-- 
2.16.1.207.gedba492059

-- 8< --

Usage would be something like this

-- 8< --
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index ecd5896064..049bfc3320 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1103,12 +1103,13 @@ __git_count_arguments ()
 }
 
 __git_whitespacelist="nowarn warn error error-all fix"
+__git_am_inprogress_options="--skip --continue --resolved --abort"
 
 _git_am ()
 {
 	__git_find_repo_path
 	if [ -d "$__git_repo_path"/rebase-apply ]; then
-		__gitcomp "--skip --continue --resolved --abort"
+		__gitcomp "$__git_am_inprogress_options"
 		return
 	fi
 	case "$cur" in
@@ -1117,7 +1118,8 @@ _git_am ()
 		return
 		;;
 	--*)
-		__gitcomp_builtin am "--no-utf8"
+		__gitcomp_builtin am "--no-utf8" \
+			"$__git_am_inprogress_options"
 		return
 	esac
 }
-- 8< --

I think I'm keeping the flag name PARSE_OPT_NOCOMPLETE for now
though. There are a few options that don't fit in "dangerous"
category, e.g.

- "gc --auto" is hidden because it does not make sense to type it
  directly. Same story for "push --thin".

- "grep --ext-grep" is no-op

- --null, --exit-code and others are for scripting and not that often
  typed.
--
Duy