Re: [PATCH v2] completion: add diff --color-moved[-ws]

[Matheus Tavares <matheus.bernardino@usp.br>]

Hi, Kir

On Fri, Feb 21, 2020 at 5:15 PM Kir Kolyshkin <kolyshkin@gmail.com> wrote:
>
> These options are available since git v2.15, but somehow
> eluded from the completion script.
>
> Note that while --color-moved-ws= accepts comma-separated
> list of values, there is no (easy?) way to make it work
> with completion (see e.g. [1]).

This puzzled me for some time, but I think I finally got a way to make the
comma-separated completion work. Bellow is the code that does the trick,
together with some tests for the new __gitcomp_cvs function. The diff is on top
of your patch, so you can incorporate it for a v3. Alternatively, if you want
to send these changes as a preparatory patch in a two-patches series, you have
my Signed-off-by :)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 348f0c0c57..e12f90b1cb 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -476,6 +476,41 @@ __gitcomp_file ()
 	true
 }
 
+# The following function is based on
+# https://unix.stackexchange.com/a/176442 (under CC BY-SA 4.0) written
+# by diffycat (https://unix.stackexchange.com/users/17452/diffycat)
+#
+# Call __gitcomp for options that accept a comma separated list of values, i.e.
+# something like '--option=val1,val2'. The caller must have already checked
+# that `$cur == --option=*`. __gitcomp_csv requires two arguments:
+# 1: The option in the format of '--option='
+# 2: The list of possible values for the said option, separated by spaces. Note
+#    that the values cannot contain commas or spaces.
+__gitcomp_csv ()
+{
+	local cur_values="${cur##$1}" available_values
+
+	# Filter out already used values from completion reply
+	for value in $2
+	do
+		if ! [[ ",$cur_values," =~ ",$value," ]]
+		then
+			available_values="$available_values $value"
+		fi
+	done
+
+	local prefix pattern
+	if [[ "$cur_values" == *,* ]]
+	then
+		prefix="${cur_values%,*},"
+		pattern="${cur_values##*,}"
+	else
+		pattern="$cur_values"
+	fi
+
+	__gitcomp "$available_values" "$prefix" "$pattern"
+}
+
 # Execute 'git ls-files', unless the --committable option is specified, in
 # which case it runs 'git diff-index' to find out the files that can be
 # committed.  It return paths relative to the directory specified in the first
@@ -1532,7 +1567,7 @@ _git_diff ()
 		return
 		;;
 	--color-moved-ws=*)
-		__gitcomp "$__git_color_moved_ws_opts" "" "${cur##--color-moved-ws=}"
+		__gitcomp_csv "--color-moved-ws=" "$__git_color_moved_ws_opts"
 		return
 		;;
 	--*)
diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 5505e5aa24..75b6afe2b7 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -122,6 +122,21 @@ test_gitcomp_nl ()
 	test_cmp expected out
 }
 
+# Test __gitcomp_csv
+# Arguments are:
+# 1: current word (cur)
+# -: the rest are passed to __gitcomp_csv
+test_gitcomp_csv ()
+{
+	local -a COMPREPLY &&
+	sed -e 's/Z$//' >expected &&
+	local cur="$1" &&
+	shift &&
+	__gitcomp_csv "$@" &&
+	print_comp &&
+	test_cmp expected out
+}
+
 invalid_variable_name='${foo.bar}'
 
 actual="$TRASH_DIRECTORY/actual"
@@ -580,6 +595,21 @@ test_expect_success '__gitcomp_nl - doesnt fail because of invalid variable name
 	__gitcomp_nl "$invalid_variable_name"
 '
 
+test_expect_success '__gitcomp_csv - display all values' '
+	test_gitcomp_csv "--opt=" "--opt=" "val1 val2 val3" <<-\EOF
+	val1 Z
+	val2 Z
+	val3 Z
+	EOF
+'
+
+test_expect_success '__gitcomp_csv - do not display values in $cur' '
+	test_gitcomp_csv "--opt=val1," "--opt=" "val1 val2 val3" <<-\EOF
+	val1,val2 Z
+	val1,val3 Z
+	EOF
+'
+
 test_expect_success '__git_remotes - list remotes from $GIT_DIR/remotes and from config file' '
 	cat >expect <<-EOF &&
 	remote_from_file_1

--->8---

> [v2: added missing ignore-space-change]

Change logs are quite useful for reviewers, so thanks for adding one :) However,
they don't really need to be part of the commit message. So the common place to
write them is between the '---' line and the diff stats. Everything you write
in this section will be available for reviewers but discarded when the patch is
applied.

> Acked-by: Matheus Tavares Bernardino <matheus.bernardino@usp.br>

If you are going to send a v3, please, also use
"Matheus Tavares <matheus.bernardino@usp.br>" instead (that's just how I
normally sign).