[PATCH v2 0/9] completion: general cleanups

[PATCH v2 0/9] completion: general cleanups

[Felipe Contreras]

Hi,

In the previous series I didn't notice -o filenames actually did something
useful, so in this series it's still used, however, for versions older than 4,
we use a hack to enable it. This allows us to get rid of a lot of compat code.

I also got rid of some wrappers, and fixed a regression: spaces were not added
at the end of the completion.

The result is that now we have the same completion for bash < 4, bash >=4, and
zsh. Although we already had that for the last two, now the code is simpler.

The only difference is that bash < 4 doesn't add a trailing space, like before
fea16b4 (git-completion.bash: add support for path completion).

Also, add the missing tests.

Cheers.

Felipe Contreras (9):
  completion: add file completion tests
  completion: document tilde expansion failure in tests
  completion; remove unuseful comments
  completion: use __gitcompadd for __gitcomp_file
  completion: refactor diff_index wrappers
  completion: refactor __git_complete_index_file()
  completion: add hack to enable file mode in bash < 4
  completion: add space after completed filename
  completion: remove __git_index_file_list_filter()

 contrib/completion/git-completion.bash | 145 +++++++--------------------------
 t/t9902-completion.sh                  |  77 +++++++++++++++++
 2 files changed, 107 insertions(+), 115 deletions(-)

-- 
1.8.2.1.1031.g2ee5873

[PATCH v2 1/9] completion: add file completion tests

[Felipe Contreras]

The commit fea16b4 (git-completion.bash: add support for path
completion) introduced quite a few changes, without the usual tests.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 6d9d141..385e1e4 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -347,4 +347,72 @@ test_expect_success 'send-email' '
 	test_completion "git send-email ma" "master "
 '
 
+test_expect_success 'complete files' '
+	git init tmp && cd tmp &&
+	test_when_finished "cd .. && rm -rf tmp" &&
+
+	echo "expected" > .gitignore &&
+	echo "out" >> .gitignore &&
+
+	git add .gitignore &&
+	test_completion "git commit " ".gitignore" &&
+
+	git commit -m ignore &&
+
+	touch new &&
+	test_completion "git add " "new" &&
+
+	git add new &&
+	git commit -a -m new &&
+	test_completion "git add " "" &&
+
+	git mv new modified &&
+	echo modify > modified &&
+	test_completion "git add " "modified" &&
+
+	touch untracked &&
+
+	: TODO .gitignore should not be here &&
+	test_completion "git rm " <<-\EOF &&
+	.gitignore
+	modified
+	EOF
+
+	test_completion "git clean " "untracked" &&
+
+	: TODO .gitignore should not be here &&
+	test_completion "git mv " <<-\EOF &&
+	.gitignore
+	modified
+	EOF
+
+	mkdir dir &&
+	touch dir/file-in-dir &&
+	git add dir/file-in-dir &&
+	git commit -m dir &&
+
+	mkdir untracked-dir &&
+
+	: TODO .gitignore should not be here &&
+	test_completion "git mv modified " <<-\EOF &&
+	.gitignore
+	dir
+	modified
+	untracked
+	untracked-dir
+	EOF
+
+	test_completion "git commit " "modified" &&
+
+	: TODO .gitignore should not be here &&
+	test_completion "git ls-files " <<-\EOF
+	.gitignore
+	dir
+	modified
+	EOF
+
+	touch momified &&
+	test_completion "git add mom" "momified"
+'
+
 test_done
-- 
1.8.2.1.1031.g2ee5873

[PATCH v2 2/9] completion: document tilde expansion failure in tests

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t9902-completion.sh | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
index 385e1e4..81a1657 100755
--- a/t/t9902-completion.sh
+++ b/t/t9902-completion.sh
@@ -415,4 +415,13 @@ test_expect_success 'complete files' '
 	test_completion "git add mom" "momified"
 '
 
+test_expect_failure 'complete with tilde expansion' '
+	git init tmp && cd tmp &&
+	test_when_finished "cd .. && rm -rf tmp" &&
+
+	touch ~/tmp/file &&
+
+	test_completion "git add ~/tmp/" "~/tmp/file"
+'
+
 test_done
-- 
1.8.2.1.1031.g2ee5873

[PATCH v2 3/9] completion; remove unuseful comments

[Felipe Contreras]

The only caller, __git_complete_index_file() doesn't specify any limits
to the options for 'git ls-files', neither should this function.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 2 --
 1 file changed, 2 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index bc3fc9e..f7b0f3c 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -323,8 +323,6 @@ __git_diff_index_helper ()
 
 # __git_index_files accepts 1 or 2 arguments:
 # 1: Options to pass to ls-files (required).
-#    Supported options are --cached, --modified, --deleted, --others,
-#    and --directory.
 # 2: A directory path (optional).
 #    If provided, only files within the specified directory are listed.
 #    Sub directories are never recursed.  Path must have a trailing
-- 
1.8.2.1.1031.g2ee5873

[PATCH v2 4/9] completion: use __gitcompadd for __gitcomp_file

[Felipe Contreras]

Like the rest of the script does; let's not access COMPREPLY directly.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f7b0f3c..7f1ebe4 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -252,7 +252,7 @@ __gitcomp_file ()
 	# since tilde expansion is not applied.
 	# This means that COMPREPLY will be empty and Bash default
 	# completion will be used.
-	COMPREPLY=($(compgen -P "${2-}" -W "$1" -- "${3-$cur}"))
+	__gitcompadd "$1" "${2-}" "${3-$cur}" ""
 
 	# Tell Bash that compspec generates filenames.
 	compopt -o filenames 2>/dev/null
-- 
1.8.2.1.1031.g2ee5873

[PATCH v2 5/9] completion: refactor diff_index wrappers

[Felipe Contreras]

At the end of the day what we really need is to find out the files that
have been staged, or modified, because those files are the ones that
make sense to pass as arguments to 'git commit'.

We need diff-index to find those out, since 'git ls-files' doesn't do
that.

But we don't need wrappers and wrappers basically identical to the ones
used for 'git ls-files', when we can pretend it receives a --committable
option that would return what we need.

That way, we can remove a bunch of code without any functional changes.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 71 ++++++++--------------------------
 1 file changed, 16 insertions(+), 55 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7f1ebe4..2561265 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -297,30 +297,25 @@ __git_index_file_list_filter ()
 	__git_index_file_list_filter_bash
 }
 
-# Execute git ls-files, returning paths relative to the directory
-# specified in the first argument, and using the options specified in
-# the second argument.
+# 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
+# argument, and using the options specified in the second argument.
 __git_ls_files_helper ()
 {
 	(
 		test -n "${CDPATH+set}" && unset CDPATH
-		# NOTE: $2 is not quoted in order to support multiple options
-		cd "$1" && git ls-files --exclude-standard $2
+		cd "$1"
+		if [ "$2" == "--committable" ]; then
+			git diff-index --name-only --relative HEAD
+		else
+			# NOTE: $2 is not quoted in order to support multiple options
+			git ls-files --exclude-standard $2
+		fi
 	) 2>/dev/null
 }
 
 
-# Execute git diff-index, returning paths relative to the directory
-# specified in the first argument, and using the tree object id
-# specified in the second argument.
-__git_diff_index_helper ()
-{
-	(
-		test -n "${CDPATH+set}" && unset CDPATH
-		cd "$1" && git diff-index --name-only --relative "$2"
-	) 2>/dev/null
-}
-
 # __git_index_files accepts 1 or 2 arguments:
 # 1: Options to pass to ls-files (required).
 # 2: A directory path (optional).
@@ -337,22 +332,6 @@ __git_index_files ()
 	fi
 }
 
-# __git_diff_index_files accepts 1 or 2 arguments:
-# 1) The id of a tree object.
-# 2) A directory path (optional).
-#    If provided, only files within the specified directory are listed.
-#    Sub directories are never recursed.  Path must have a trailing
-#    slash.
-__git_diff_index_files ()
-{
-	local dir="$(__gitdir)" root="${2-.}"
-
-	if [ -d "$dir" ]; then
-		__git_diff_index_helper "$root" "$1" | __git_index_file_list_filter |
-			sort | uniq
-	fi
-}
-
 __git_heads ()
 {
 	local dir="$(__gitdir)"
@@ -550,8 +529,10 @@ __git_complete_revlist_file ()
 }
 
 
-# __git_complete_index_file requires 1 argument: the options to pass to
-# ls-file
+# __git_complete_index_file requires 1 argument:
+# 1: the options to pass to ls-file
+#
+# The exception is --committable, which finds the files appropriate commit.
 __git_complete_index_file ()
 {
 	local pfx cur_="$cur"
@@ -570,26 +551,6 @@ __git_complete_index_file ()
 	esac
 }
 
-# __git_complete_diff_index_file requires 1 argument: the id of a tree
-# object
-__git_complete_diff_index_file ()
-{
-	local pfx cur_="$cur"
-
-	case "$cur_" in
-	?*/*)
-		pfx="${cur_%/*}"
-		cur_="${cur_##*/}"
-		pfx="${pfx}/"
-
-		__gitcomp_file "$(__git_diff_index_files "$1" "$pfx")" "$pfx" "$cur_"
-		;;
-	*)
-		__gitcomp_file "$(__git_diff_index_files "$1")" "" "$cur_"
-		;;
-	esac
-}
-
 __git_complete_file ()
 {
 	__git_complete_revlist_file
@@ -1211,7 +1172,7 @@ _git_commit ()
 	esac
 
 	if git rev-parse --verify --quiet HEAD >/dev/null; then
-		__git_complete_diff_index_file "HEAD"
+		__git_complete_index_file "--committable"
 	else
 		# This is the first commit
 		__git_complete_index_file "--cached"
-- 
1.8.2.1.1031.g2ee5873

[PATCH v2 6/9] completion: refactor __git_complete_index_file()

[Felipe Contreras]

The calls to __gitcomp_file() are essentially the same, but with
different prefix.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 2561265..9cea170 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -535,20 +535,17 @@ __git_complete_revlist_file ()
 # The exception is --committable, which finds the files appropriate commit.
 __git_complete_index_file ()
 {
-	local pfx cur_="$cur"
+	local pfx="" cur_="$cur"
 
 	case "$cur_" in
 	?*/*)
 		pfx="${cur_%/*}"
 		cur_="${cur_##*/}"
 		pfx="${pfx}/"
-
-		__gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_"
-		;;
-	*)
-		__gitcomp_file "$(__git_index_files "$1")" "" "$cur_"
 		;;
 	esac
+
+	__gitcomp_file "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_"
 }
 
 __git_complete_file ()
-- 
1.8.2.1.1031.g2ee5873

[PATCH v2 7/9] completion: add hack to enable file mode in bash < 4

[Felipe Contreras]

This way we don't need all the compat stuff, different filters, and so
on. Also, now we complete exactly the same in bash 3 and bash 4.

This is the way bash-completion did it for quite some time, when bash 3
was supported. For more information about the hack:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=272660#64

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 43 ++++++----------------------------
 1 file changed, 7 insertions(+), 36 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 9cea170..f9e8e7d 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -254,33 +254,21 @@ __gitcomp_file ()
 	# completion will be used.
 	__gitcompadd "$1" "${2-}" "${3-$cur}" ""
 
-	# Tell Bash that compspec generates filenames.
-	compopt -o filenames 2>/dev/null
+	# use a hack to enable file mode in bash < 4
+	compopt -o filenames 2>/dev/null ||
+	compgen -f /non-existing-dir/ > /dev/null
 }
 
-__git_index_file_list_filter_compat ()
-{
-	local path
-
-	while read -r path; do
-		case "$path" in
-		?*/*) echo "${path%%/*}/" ;;
-		*) echo "$path" ;;
-		esac
-	done
-}
-
-__git_index_file_list_filter_bash ()
+# Process path list returned by "ls-files" and "diff-index --name-only"
+# commands, in order to list only file names relative to a specified
+# directory, and append a slash to directory names.
+__git_index_file_list_filter ()
 {
 	local path
 
 	while read -r path; do
 		case "$path" in
 		?*/*)
-			# XXX if we append a slash to directory names when using
-			# `compopt -o filenames`, Bash will append another slash.
-			# This is pretty stupid, and this the reason why we have to
-			# define a compatible version for this function.
 			echo "${path%%/*}" ;;
 		*)
 			echo "$path" ;;
@@ -288,15 +276,6 @@ __git_index_file_list_filter_bash ()
 	done
 }
 
-# Process path list returned by "ls-files" and "diff-index --name-only"
-# commands, in order to list only file names relative to a specified
-# directory, and append a slash to directory names.
-__git_index_file_list_filter ()
-{
-	# Default to Bash >= 4.x
-	__git_index_file_list_filter_bash
-}
-
 # 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
@@ -2651,14 +2630,6 @@ if [[ -n ${ZSH_VERSION-} ]]; then
 
 	compdef _git git gitk
 	return
-elif [[ -n ${BASH_VERSION-} ]]; then
-	if ((${BASH_VERSINFO[0]} < 4)); then
-		# compopt is not supported
-		__git_index_file_list_filter ()
-		{
-			__git_index_file_list_filter_compat
-		}
-	fi
 fi
 
 __git_func_wrap ()
-- 
1.8.2.1.1031.g2ee5873

[PATCH v2 8/9] completion: add space after completed filename

[Felipe Contreras]

Just like before fea16b4 (git-completion.bash: add support for path
completion).

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index f9e8e7d..20c9718 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -255,7 +255,7 @@ __gitcomp_file ()
 	__gitcompadd "$1" "${2-}" "${3-$cur}" ""
 
 	# use a hack to enable file mode in bash < 4
-	compopt -o filenames 2>/dev/null ||
+	compopt -o filenames +o nospace 2>/dev/null ||
 	compgen -f /non-existing-dir/ > /dev/null
 }
 
-- 
1.8.2.1.1031.g2ee5873

[PATCH v2 9/9] completion: remove __git_index_file_list_filter()

[Felipe Contreras]

Refactor the code into the only caller; __git_index_files().

Also, Somehow messing up with the 'path' variable messes up the 'PATH'
variable. So let's not do that.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 contrib/completion/git-completion.bash | 28 ++++++++--------------------
 1 file changed, 8 insertions(+), 20 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 20c9718..edb7428 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -259,23 +259,6 @@ __gitcomp_file ()
 	compgen -f /non-existing-dir/ > /dev/null
 }
 
-# Process path list returned by "ls-files" and "diff-index --name-only"
-# commands, in order to list only file names relative to a specified
-# directory, and append a slash to directory names.
-__git_index_file_list_filter ()
-{
-	local path
-
-	while read -r path; do
-		case "$path" in
-		?*/*)
-			echo "${path%%/*}" ;;
-		*)
-			echo "$path" ;;
-		esac
-	done
-}
-
 # 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
@@ -303,11 +286,16 @@ __git_ls_files_helper ()
 #    slash.
 __git_index_files ()
 {
-	local dir="$(__gitdir)" root="${2-.}"
+	local dir="$(__gitdir)" root="${2-.}" file
 
 	if [ -d "$dir" ]; then
-		__git_ls_files_helper "$root" "$1" | __git_index_file_list_filter |
-			sort | uniq
+		__git_ls_files_helper "$root" "$1" |
+		while read -r file; do
+			case "$file" in
+			?*/*) echo "${file%%/*}" ;;
+			*) echo "$file" ;;
+			esac
+		done | sort | uniq
 	fi
 }
 
-- 
1.8.2.1.1031.g2ee5873

Re: [PATCH v2 1/9] completion: add file completion tests

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> The commit fea16b4 (git-completion.bash: add support for path
> completion) introduced quite a few changes, without the usual tests.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  t/t9902-completion.sh | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/t/t9902-completion.sh b/t/t9902-completion.sh
> index 6d9d141..385e1e4 100755
> --- a/t/t9902-completion.sh
> +++ b/t/t9902-completion.sh
> @@ -347,4 +347,72 @@ test_expect_success 'send-email' '
>  	test_completion "git send-email ma" "master "
>  '
>  
> +test_expect_success 'complete files' '
> +	git init tmp && cd tmp &&
> +	test_when_finished "cd .. && rm -rf tmp" &&

Hmm, this construct is not very nice.

Later part of the code can be modified carelessly and chdir around
even more, and "cd .." may end up at an unexpected place.  Can't
this be done in a way to make later reviews of patches that come
after this change lands doable without looking at the whole 70
lines?  For example, if the test ran the whole "these are done in a
subdirectry" part, starting from the "cd tmp", inside a subshell,
and "rm -rf tmp" is done in the original shell that ran "git init
tmp" to create it, it would make it much harder to make mistakes for
anybody who add new code to the body of the test to affect the
clean-up part.

> +
> +	echo "expected" > .gitignore &&
> +	echo "out" >> .gitignore &&

Style: we tend to omit SP between redireciton operator and targets.

> +	git add .gitignore &&
> +	test_completion "git commit " ".gitignore" &&
> +
> +	git commit -m ignore &&
> +
> +	touch new &&
> +	test_completion "git add " "new" &&
> +
> +	git add new &&
> +	git commit -a -m new &&
> +	test_completion "git add " "" &&
> +
> +	git mv new modified &&
> +	echo modify > modified &&
> +	test_completion "git add " "modified" &&
> +
> +	touch untracked &&
> +
> +	: TODO .gitignore should not be here &&
> +	test_completion "git rm " <<-\EOF &&
> +	.gitignore
> +	modified
> +	EOF
> +
> +	test_completion "git clean " "untracked" &&
> +
> +	: TODO .gitignore should not be here &&
> +	test_completion "git mv " <<-\EOF &&
> +	.gitignore
> +	modified
> +	EOF
> +
> +	mkdir dir &&
> +	touch dir/file-in-dir &&
> +	git add dir/file-in-dir &&
> +	git commit -m dir &&
> +
> +	mkdir untracked-dir &&
> +
> +	: TODO .gitignore should not be here &&
> +	test_completion "git mv modified " <<-\EOF &&
> +	.gitignore
> +	dir
> +	modified
> +	untracked
> +	untracked-dir
> +	EOF
> +
> +	test_completion "git commit " "modified" &&
> +
> +	: TODO .gitignore should not be here &&
> +	test_completion "git ls-files " <<-\EOF
> +	.gitignore
> +	dir
> +	modified
> +	EOF
> +
> +	touch momified &&
> +	test_completion "git add mom" "momified"
> +'
> +
>  test_done

Re: [PATCH v2 5/9] completion: refactor diff_index wrappers

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> At the end of the day what we really need is to find out the files that
> have been staged, or modified, because those files are the ones that
> make sense to pass as arguments to 'git commit'.
>
> We need diff-index to find those out, since 'git ls-files' doesn't do
> that.
>
> But we don't need wrappers and wrappers basically identical to the ones
> used for 'git ls-files', when we can pretend it receives a --committable
> option that would return what we need.
>
> That way, we can remove a bunch of code without any functional changes.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  contrib/completion/git-completion.bash | 71 ++++++++--------------------------
>  1 file changed, 16 insertions(+), 55 deletions(-)

Nice line reduction.


>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 7f1ebe4..2561265 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -297,30 +297,25 @@ __git_index_file_list_filter ()
>  	__git_index_file_list_filter_bash
>  }
>  
> -# Execute git ls-files, returning paths relative to the directory
> -# specified in the first argument, and using the options specified in
> -# the second argument.
> +# 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
> +# argument, and using the options specified in the second argument.
>  __git_ls_files_helper ()
>  {
>  	(
>  		test -n "${CDPATH+set}" && unset CDPATH
> -		# NOTE: $2 is not quoted in order to support multiple options
> -		cd "$1" && git ls-files --exclude-standard $2
> +		cd "$1"
> +		if [ "$2" == "--committable" ]; then
> +			git diff-index --name-only --relative HEAD
> +		else
> +			# NOTE: $2 is not quoted in order to support multiple options
> +			git ls-files --exclude-standard $2
> +		fi
>  	) 2>/dev/null
>  }
>  
>  
> -# Execute git diff-index, returning paths relative to the directory
> -# specified in the first argument, and using the tree object id
> -# specified in the second argument.
> -__git_diff_index_helper ()
> -{
> -	(
> -		test -n "${CDPATH+set}" && unset CDPATH
> -		cd "$1" && git diff-index --name-only --relative "$2"
> -	) 2>/dev/null
> -}
> -
>  # __git_index_files accepts 1 or 2 arguments:
>  # 1: Options to pass to ls-files (required).
>  # 2: A directory path (optional).
> @@ -337,22 +332,6 @@ __git_index_files ()
>  	fi
>  }
>  
> -# __git_diff_index_files accepts 1 or 2 arguments:
> -# 1) The id of a tree object.
> -# 2) A directory path (optional).
> -#    If provided, only files within the specified directory are listed.
> -#    Sub directories are never recursed.  Path must have a trailing
> -#    slash.
> -__git_diff_index_files ()
> -{
> -	local dir="$(__gitdir)" root="${2-.}"
> -
> -	if [ -d "$dir" ]; then
> -		__git_diff_index_helper "$root" "$1" | __git_index_file_list_filter |
> -			sort | uniq
> -	fi
> -}
> -
>  __git_heads ()
>  {
>  	local dir="$(__gitdir)"
> @@ -550,8 +529,10 @@ __git_complete_revlist_file ()
>  }
>  
>  
> -# __git_complete_index_file requires 1 argument: the options to pass to
> -# ls-file
> +# __git_complete_index_file requires 1 argument:
> +# 1: the options to pass to ls-file
> +#
> +# The exception is --committable, which finds the files appropriate commit.
>  __git_complete_index_file ()
>  {
>  	local pfx cur_="$cur"
> @@ -570,26 +551,6 @@ __git_complete_index_file ()
>  	esac
>  }
>  
> -# __git_complete_diff_index_file requires 1 argument: the id of a tree
> -# object
> -__git_complete_diff_index_file ()
> -{
> -	local pfx cur_="$cur"
> -
> -	case "$cur_" in
> -	?*/*)
> -		pfx="${cur_%/*}"
> -		cur_="${cur_##*/}"
> -		pfx="${pfx}/"
> -
> -		__gitcomp_file "$(__git_diff_index_files "$1" "$pfx")" "$pfx" "$cur_"
> -		;;
> -	*)
> -		__gitcomp_file "$(__git_diff_index_files "$1")" "" "$cur_"
> -		;;
> -	esac
> -}
> -
>  __git_complete_file ()
>  {
>  	__git_complete_revlist_file
> @@ -1211,7 +1172,7 @@ _git_commit ()
>  	esac
>  
>  	if git rev-parse --verify --quiet HEAD >/dev/null; then
> -		__git_complete_diff_index_file "HEAD"
> +		__git_complete_index_file "--committable"
>  	else
>  		# This is the first commit
>  		__git_complete_index_file "--cached"

Re: [PATCH v2 0/9] completion: general cleanups

[Junio C Hamano]

Overall this looks like a nice clean-up.  I didn't look at the 5/9
carefully to verify its claim that the change is a no-op, though.

Will queue.

Re: [PATCH v2 9/9] completion: remove __git_index_file_list_filter()

[Eric Sunshine]

On Sat, Apr 27, 2013 at 4:10 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> Refactor the code into the only caller; __git_index_files().
>
> Also, Somehow messing up with the 'path' variable messes up the 'PATH'

s/Somehow/somehow/
s/messing up/messing/

> variable. So let's not do that.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

Re: [PATCH v2 9/9] completion: remove __git_index_file_list_filter()

[Felipe Contreras]

On Sat, Apr 27, 2013 at 8:41 PM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Apr 27, 2013 at 4:10 PM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> Refactor the code into the only caller; __git_index_files().
>>
>> Also, Somehow messing up with the 'path' variable messes up the 'PATH'
>
> s/Somehow/somehow/
> s/messing up/messing/

More important than that is the fact that the problem is only on zsh.

-- 
Felipe Contreras