[PATCH v5] git-completion.bash: add support for path completion

[PATCH v5] git-completion.bash: add support for path completion

[Manlio Perillo]

The git-completion.bash script did not implemented full, git aware,
support to complete paths, for git commands that operate on files within
the current working directory or the index.

As an example:

	git add <TAB>

will suggest all files in the current working directory, including
ignored files and files that have not been modified.

Support path completion, for git commands where the non-option arguments
always refer to paths within the current working directory or the index,
as follows:

* the path completion for the "git rm" and "git ls-files"
  commands will suggest all cached files.

* the path completion for the "git add" command will suggest all
  untracked and modified files.  Ignored files are excluded.

* the path completion for the "git clean" command will suggest all
  untracked files.  Ignored files are excluded.

* the path completion for the "git mv" command will suggest all cached
  files when expanding the first argument, and all untracked and cached
  files for subsequent arguments.  In the latter case, empty directories
  are included and ignored files are excluded.

* the path completion for the "git commit" command will suggest all
  files that have been modified from the HEAD, if HEAD exists, otherwise
  it will suggest all cached files.

For all affected commands, completion will always stop at directory
boundary.  Only standard ignored files are excluded, using the
--exclude-standard option of the ls-files command.

When using a recent Bash version, Git path completion will be the same
as builtin file completion, e.g.

	git add contrib/

will suggest relative file names.

Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
---

Changes:

	* Applied Junio patch to fix completion inside a subdirectory.
	* Quoted the hopefully last incorrectly unquoted variable.
	* Fixed coding style (removed stdout file descriptor in shell
	  redirection, since it is redundant).
	* Fixed regression in path completion, when using non canonicalized
	  or absolute path names.
	  The problem has been solved making sure to chdir to the specified
	  directory before executing ls-files and diff-index commands.

	  The only issue is that there is no tilde expansion, but this is
	  harmless, since default bash completion will be used (the old
	  behaviour).
	* Improved path completion when the new compopt builtin is available
	  (Bash >= 4.x).
	  Now git paths completion is done in exactly the same way as Bash
	  builtin filenames completion.
    * Updated the zsh compatibility code to use the improved path
      completion support
	* Fixed incorrect git mv arguments count used to check the first
	  path to be renamed.
	  When options are used (unless they are git main options), -- is
	  required to separate options from non options arguments.
	  It is harmless to not use --; in this case bash will suggest
	  untracked files and directories for the first argument.

	  XXX: should I add this implementation note in the commit message?
	* Make sure to sort ls-files and diff-index filtered output before
	  removing duplicate directories.
	* Merged master.
	 
Please note that before merging this patch in next, we need to update the
zsh and tcsh completion scripts.
I have the changes ready, but I will post them later since both scripts
needs more patches (I have posted an informal patch for zsh, and changes
to tcsh should be in pu, but I need to test them).

 contrib/completion/git-completion.bash | 250 ++++++++++++++++++++++++++++++---
 1 file changed, 234 insertions(+), 16 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index a4c48e1..51b8b3b 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -13,6 +13,7 @@
 #    *) .git/remotes file names
 #    *) git 'subcommands'
 #    *) tree paths within 'ref:path/to/file' expressions
+#    *) file paths within current working directory and index
 #    *) common --long-options
 #
 # To use these routines:
@@ -233,6 +234,118 @@ __gitcomp_nl ()
 	COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
 }
 
+# Generates completion reply with compgen from newline-separated possible
+# completion filenames.
+# It accepts 1 to 3 arguments:
+# 1: List of possible completion filenames, separated by a single newline.
+# 2: A directory prefix to be added to each possible completion filename
+#    (optional).
+# 3: Generate possible completion matches for this word (optional).
+__gitcomp_file ()
+{
+	local IFS=$'\n'
+
+	# XXX does not work when the directory prefix contains a tilde,
+	# 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}"))
+
+	# Tell Bash that compspec generates filenames.
+	compopt -o filenames 2>/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 ()
+{
+	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" ;;
+		esac
+	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, returning paths relative to the directory
+# specified in the first argument, and using the options specified in
+# the second argument.
+__git_ls_files_helper ()
+{
+	# NOTE: $2 is not quoted in order to support multiple options
+	cd "$1" && git ls-files --exclude-standard $2
+} 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 ()
+{
+	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).
+#    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
+#    slash.
+__git_index_files ()
+{
+	local dir="$(__gitdir)" root="${2-.}"
+
+	if [ -d "$dir" ]; then
+		__git_ls_files_helper "$root" "$1" | __git_index_file_list_filter |
+			sort | uniq
+	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)"
@@ -430,6 +543,46 @@ __git_complete_revlist_file ()
 }
 
 
+# __git_complete_index_file requires 1 argument: the options to pass to
+# ls-file
+__git_complete_index_file ()
+{
+	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
+}
+
+# __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
@@ -722,6 +875,43 @@ __git_has_doubledash ()
 	return 1
 }
 
+# Try to count non option arguments passed on the command line for the
+# specified git command.
+# When options are used, it is necessary to use the special -- option to
+# tell the implementation were non option arguments begin.
+# XXX this can not be improved, since options can appear everywhere, as
+# an example:
+#	git mv x -n y
+#
+# __git_count_arguments requires 1 argument: the git command executed.
+__git_count_arguments ()
+{
+	local word i c=0
+
+	# Skip "git" (first argument)
+	for ((i=1; i < ${#words[@]}; i++)); do
+		word="${words[i]}"
+
+		case "$word" in
+			--)
+				# Good; we can assume that the following are only non
+				# option arguments.
+				((c = 0))
+				;;
+			"$1")
+				# Skip the specified git command and discard git
+				# main options
+				((c = 0))
+				;;
+			?*)
+				((c++))
+				;;
+		esac
+	done
+
+	printf "%d" $c
+}
+
 __git_whitespacelist="nowarn warn error error-all fix"
 
 _git_am ()
@@ -770,8 +960,6 @@ _git_apply ()
 
 _git_add ()
 {
-	__git_has_doubledash && return
-
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -780,7 +968,9 @@ _git_add ()
 			"
 		return
 	esac
-	COMPREPLY=()
+
+	# XXX should we check for --update and --all options ?
+	__git_complete_index_file "--others --modified"
 }
 
 _git_archive ()
@@ -930,15 +1120,15 @@ _git_cherry_pick ()
 
 _git_clean ()
 {
-	__git_has_doubledash && return
-
 	case "$cur" in
 	--*)
 		__gitcomp "--dry-run --quiet"
 		return
 		;;
 	esac
-	COMPREPLY=()
+
+	# XXX should we check for -x option ?
+	__git_complete_index_file "--others"
 }
 
 _git_clone ()
@@ -969,8 +1159,6 @@ _git_clone ()
 
 _git_commit ()
 {
-	__git_has_doubledash && return
-
 	case "$prev" in
 	-c|-C)
 		__gitcomp_nl "$(__git_refs)" "" "${cur}"
@@ -1005,7 +1193,13 @@ _git_commit ()
 			"
 		return
 	esac
-	COMPREPLY=()
+
+	if git rev-parse --verify --quiet HEAD >/dev/null; then
+		__git_complete_diff_index_file "HEAD"
+	else
+		# This is the first commit
+		__git_complete_index_file "--cached"
+	fi
 }
 
 _git_describe ()
@@ -1223,8 +1417,6 @@ _git_init ()
 
 _git_ls_files ()
 {
-	__git_has_doubledash && return
-
 	case "$cur" in
 	--*)
 		__gitcomp "--cached --deleted --modified --others --ignored
@@ -1237,7 +1429,10 @@ _git_ls_files ()
 		return
 		;;
 	esac
-	COMPREPLY=()
+
+	# XXX ignore options like --modified and always suggest all cached
+	# files.
+	__git_complete_index_file "--cached"
 }
 
 _git_ls_remote ()
@@ -1369,7 +1564,14 @@ _git_mv ()
 		return
 		;;
 	esac
-	COMPREPLY=()
+
+	if [ $(__git_count_arguments "mv") -gt 0 ]; then
+		# We need to show both cached and untracked files (including
+		# empty directories) since this may not be the last argument.
+		__git_complete_index_file "--cached --others --directory"
+	else
+		__git_complete_index_file "--cached"
+	fi
 }
 
 _git_name_rev ()
@@ -2075,15 +2277,14 @@ _git_revert ()
 
 _git_rm ()
 {
-	__git_has_doubledash && return
-
 	case "$cur" in
 	--*)
 		__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
 		return
 		;;
 	esac
-	COMPREPLY=()
+
+	__git_complete_index_file "--cached"
 }
 
 _git_shortlog ()
@@ -2448,6 +2649,15 @@ if [[ -n ${ZSH_VERSION-} ]]; then
 		compadd -Q -S "${4- }" -p "${2-}" -- ${=1} && _ret=0
 	}
 
+	__gitcomp_file ()
+	{
+		emulate -L zsh
+
+		local IFS=$'\n'
+		compset -P '*[=:]'
+		compadd -Q -p "${2-}" -f -- ${=1} && _ret=0
+	}
+
 	__git_zsh_helper ()
 	{
 		emulate -L ksh
@@ -2469,6 +2679,14 @@ 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.1.rc1.31.ga3c84da

Re: [PATCH v5] git-completion.bash: add support for path completion

[Junio C Hamano]

Manlio Perillo <manlio.perillo@gmail.com> writes:

> +# 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, returning paths relative to the directory
> +# specified in the first argument, and using the options specified in
> +# the second argument.
> +__git_ls_files_helper ()
> +{
> +	# NOTE: $2 is not quoted in order to support multiple options
> +	cd "$1" && git ls-files --exclude-standard $2
> +} 2>/dev/null

I think this redirection is correct but a bit tricky; it is in
effect during the execution of the { block } (in other words, it is
not about squelching errors during the function definition).

-- >8 --
#!/bin/sh
cat >t.sh <<\EOF &&
echo I am "$1"
t () { echo "Goes to stdout"; echo >&2 "Goes to stderr"; } 2>/dev/null
t
for sh in bash dash ksh zsh
do
	$sh t.sh $sh
done
-- 8< --

Bash does (so do dash and real AT&T ksh) grok this correctly, but
zsh does not seem to (I tried zsh 4.3.10 and 4.3.17; also zsh
pretending to be ksh gets this wrong as well).  Not that what ksh
does matters, as it won't be dot-sourcing bash completion script.

It however may affect zsh, which does seem to dot-source this file.
Perhaps zsh completion may have to be rewritten in a similar way as
tcsh completion is done (i.e. does not dot-source this file but ask
bash to do the heavy-lifting).

This function seems to be always called in an subshell (e.g. as an
upstream of a pipeline), so the "cd" may be harmless, but don't you
need to disable CDPATH while doing this?

> +# 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 ()
> +{
> +	cd "$1" && git diff-index --name-only --relative "$2"
> +} 2>/dev/null

Ditto.

> @@ -722,6 +875,43 @@ __git_has_doubledash ()
>  	return 1
>  }
>  
> +# Try to count non option arguments passed on the command line for the
> +# specified git command.
> +# When options are used, it is necessary to use the special -- option to
> +# tell the implementation were non option arguments begin.
> +# XXX this can not be improved, since options can appear everywhere, as
> +# an example:
> +#	git mv x -n y

If that is the case, it is a bug in the command line parser, I
think.  We should reject it, and the command line completer
certainly should not encourage it.

Re: [PATCH v5] git-completion.bash: add support for path completion

[Manlio Perillo]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 11/01/2013 23:02, Junio C Hamano ha scritto:
> Manlio Perillo <manlio.perillo@gmail.com> writes:
> 
>> +# 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, returning paths relative to the directory
>> +# specified in the first argument, and using the options specified in
>> +# the second argument.
>> +__git_ls_files_helper ()
>> +{
>> +	# NOTE: $2 is not quoted in order to support multiple options
>> +	cd "$1" && git ls-files --exclude-standard $2
>> +} 2>/dev/null
> 
> I think this redirection is correct but a bit tricky;

It's not tricky: it is POSIX:
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_10


> it is in
> effect during the execution of the { block } (in other words, it is
> not about squelching errors during the function definition).
> 

What do you mean by "squelching"?

Note that I originally wrote the code as

__git_ls_files_helper ()
{
	# NOTE: $2 is not quoted in order to support multiple options
	 { cd "$1" && git ls-files --exclude-standard $2 } 2>/dev/null
}

but then I checked the POSIX standard, noting that it is redundant.

> -- >8 --
> #!/bin/sh
> cat >t.sh <<\EOF &&
> echo I am "$1"
> t () { echo "Goes to stdout"; echo >&2 "Goes to stderr"; } 2>/dev/null
> t
> for sh in bash dash ksh zsh
> do
> 	$sh t.sh $sh
> done
> -- 8< --
> 

There is a missing EOF delimiter.
And I'm not sure to understand the meaning of && after EOF.

> Bash does (so do dash and real AT&T ksh) grok this correctly, but
> zsh does not seem to (I tried zsh 4.3.10 and 4.3.17; also zsh
> pretending to be ksh gets this wrong as well).  Not that what ksh
> does matters, as it won't be dot-sourcing bash completion script.
>

I have added tcsh to the sh list, but it fails with:
Badly placed ()'s.


> It however may affect zsh, which does seem to dot-source this file.
> Perhaps zsh completion may have to be rewritten in a similar way as
> tcsh completion is done (i.e. does not dot-source this file but ask
> bash to do the heavy-lifting).
> 

Ok, I was wrong on assuming all modern shells were POSIX compliant.
I will change the code to use a nested {} group.

> This function seems to be always called in an subshell (e.g. as an
> upstream of a pipeline), so the "cd" may be harmless, but don't you
> need to disable CDPATH while doing this?
> 

I don't know.

> [..]


>> +# Try to count non option arguments passed on the command line for the
>> +# specified git command.
>> +# When options are used, it is necessary to use the special -- option to
>> +# tell the implementation were non option arguments begin.
>> +# XXX this can not be improved, since options can appear everywhere, as
>> +# an example:
>> +#	git mv x -n y
> 
> If that is the case, it is a bug in the command line parser, I
> think.  We should reject it, and the command line completer
> certainly should not encourage it.
> 

$ mkdir y
$ git mv x -n y
Checking rename of 'x' to 'y/x'
Renaming x to y/x
$ git status
# On branch master
nothing to commit, working directory clean

I was assuming it to be "normal", given how complex Git command line
parsing is (IMHO).


Thanks  Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDxeMgACgkQscQJ24LbaUTmaQCeMbZ0lRJxZIx3U31gMPmcqTLp
54sAmwYrjJVuvRYcsbGaMa3rb9/EKrBU
=ky30
-----END PGP SIGNATURE-----

Re: [PATCH v5] git-completion.bash: add support for path completion

[Junio C Hamano]

Manlio Perillo <manlio.perillo@gmail.com> writes:

> Il 11/01/2013 23:02, Junio C Hamano ha scritto:
>> Manlio Perillo <manlio.perillo@gmail.com> writes:
>> 
>>> +# 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, returning paths relative to the directory
>>> +# specified in the first argument, and using the options specified in
>>> +# the second argument.
>>> +__git_ls_files_helper ()
>>> +{
>>> +	# NOTE: $2 is not quoted in order to support multiple options
>>> +	cd "$1" && git ls-files --exclude-standard $2
>>> +} 2>/dev/null
>> 
>> I think this redirection is correct but a bit tricky;
>
> It's not tricky: it is POSIX:

I know that.  It is an instance of "Even it is in POSIX, we may want
to refrain using it, because some shells get it wrong, and it is
easy to work it around".

>> effect during the execution of the { block } (in other words, it is
>> not about squelching errors during the function definition).
>
> What do you mean by "squelching"?

Silencing, not showing the end user.  Sending to /dev/null.

> I have added tcsh to the sh list, but it fails with:
> Badly placed ()'s.

tcsh (and csh) are not even in the Bourne shell family and is not
expected to be able to run any non trivial POSIX shell scripts.  The
completion script for it does not dot-source this but instead lets
bash read it, so it is fine.

>> It however may affect zsh, which does seem to dot-source this file.
>> Perhaps zsh completion may have to be rewritten in a similar way as
>> tcsh completion is done (i.e. does not dot-source this file but ask
>> bash to do the heavy-lifting).
>
> Ok, I was wrong on assuming all modern shells were POSIX compliant.

Shells in csh family will never be, and being non-POSIX is not a
crime.  It only matters when such a shell is allowed to dot-source
this script, and this script uses constructs that such a shell does
not understand.

> I will change the code to use a nested {} group.
>
>> This function seems to be always called in an subshell (e.g. as an
>> upstream of a pipeline), so the "cd" may be harmless, but don't you
>> need to disable CDPATH while doing this?
>
> I don't know.

The caller of this function figures out the value of $subdirname,
and calls you; your "cd $subdirname" may not go to ./$subdirname as
you expect, but to the $subdirname directory under one of the
directories listed in CDPATH, before running ls-tree or ls-files.

Re: [PATCH v5] git-completion.bash: add support for path completion

[Manlio Perillo]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 11/01/2013 19:48, Manlio Perillo ha scritto:
> The git-completion.bash script did not implemented full, git aware,
> support to complete paths, for git commands that operate on files within
> the current working directory or the index.
> [...]
>  
> +# Try to count non option arguments passed on the command line for the
> +# specified git command.
> +# When options are used, it is necessary to use the special -- option to
> +# tell the implementation were non option arguments begin.
> +# XXX this can not be improved, since options can appear everywhere, as
> +# an example:
> +#	git mv x -n y
> +#
> +# __git_count_arguments requires 1 argument: the git command executed.
> +__git_count_arguments ()
> +{
> +	local word i c=0
> +
> +	# Skip "git" (first argument)
> +	for ((i=1; i < ${#words[@]}; i++)); do
> +		word="${words[i]}"
> +
> +		case "$word" in
> +			--)

Sorry, I have incorrectly (again) indented the case labels.
I have now configured my editor to correctly indent this.

> +				# Good; we can assume that the following are only non
> +				# option arguments.
> +				((c = 0))
> +				;;

Here I was thinking to do something like this (not tested):

		-*)
			if [ -n ${2-} ]; then
				# Assume specified git command only
                                # accepts simple options
				# (without arguments)
				((c = 0))

Since git mv only accepts simple options, this will make the use of '--'
not required.

Note that I'm assuming the single '-' character is used as a non-option
argument; not sure this is the case of Git.

> [...]


Regards  Manlio
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDxXLkACgkQscQJ24LbaUR+QQCaA4WZP5h5lktXJqSB7c494fAY
B6IAoIRWyIzBq29S7+l+TfRjbyp19HNL
=JRpR
-----END PGP SIGNATURE-----

Re: [PATCH v5] git-completion.bash: add support for path completion

[Junio C Hamano]

Manlio Perillo <manlio.perillo@gmail.com> writes:

>> +	# Skip "git" (first argument)
>> +	for ((i=1; i < ${#words[@]}; i++)); do
>> +		word="${words[i]}"
>> +
>> +		case "$word" in
>> +			--)
>
> Sorry, I have incorrectly (again) indented the case labels.
> I have now configured my editor to correctly indent this.

Yeah, thanks for spotting.

I wouldn't worry *too* much about the style in this script at this
point, though.  It uses a style on its own that is totally different
from the rest of the system (e.g. "[" instead of "test", semicolon
in "if ...; then", etc.) and it probably is better to emulate the
surrounding code, and leave the style "fixes" to a separate topic,
if we want to (as a contrib/ material that is not POSIX but bash
specific, I do not know if that is even worth it).

>> +				# Good; we can assume that the following are only non
>> +				# option arguments.
>> +				((c = 0))
>> +				;;
>
> Here I was thinking to do something like this (not tested):
>
> 		-*)
> 			if [ -n ${2-} ]; then
> 				# Assume specified git command only
>                                 # accepts simple options
> 				# (without arguments)
> 				((c = 0))
>
> Since git mv only accepts simple options, this will make the use of '--'
> not required.

Unless you have a file whose name begins with a dash, perhaps?

Re: [PATCH v5] git-completion.bash: add support for path completion

[Felipe Contreras]

On Fri, Jan 11, 2013 at 12:48 PM, Manlio Perillo
<manlio.perillo@gmail.com> wrote:
> The git-completion.bash script did not implemented full, git aware,
> support to complete paths, for git commands that operate on files within
> the current working directory or the index.

> +__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 ()
> +{
> +       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%%/*}" ;;

Which version of bash is that? It works perfectly fine here with or
without the /.

> +# __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
> +#    slash.
> +__git_index_files ()
> +{
> +       local dir="$(__gitdir)" root="${2-.}"
> +
> +       if [ -d "$dir" ]; then
> +               __git_ls_files_helper "$root" "$1" | __git_index_file_list_filter |
> +                       sort | uniq
> +       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
> +}

These two are exactly the same, except one calls
__git_ls_files_helper, and the other one __git_diff_index_helper,
can't we make another argument that is and select one or the other
based on that?

>  __git_heads ()
>  {
>         local dir="$(__gitdir)"
> @@ -430,6 +543,46 @@ __git_complete_revlist_file ()
>  }
>
>
> +# __git_complete_index_file requires 1 argument: the options to pass to
> +# ls-file
> +__git_complete_index_file ()
> +{
> +       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
> +}
> +
> +# __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
> +}

These are also exactly the same, we could pass the argument to the
function above.

-- 
Felipe Contreras

Re: [PATCH v5] git-completion.bash: add support for path completion

[Manlio Perillo]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 21/04/2013 12:14, Felipe Contreras ha scritto:
> On Fri, Jan 11, 2013 at 12:48 PM, Manlio Perillo
> <manlio.perillo@gmail.com> wrote:
>> The git-completion.bash script did not implemented full, git aware,
>> support to complete paths, for git commands that operate on files within
>> the current working directory or the index.
> 
>> +__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 ()
>> +{
>> +       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%%/*}" ;;
> 
> Which version of bash is that? It works perfectly fine here with or
> without the /.
> 

GNU bash, version 4.1.5(1)-release (i486-pc-linux-gnu)
on a GNU Linux Debian 6

>> +# __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
>> +#    slash.
>> +__git_index_files ()
>> +{
>> +       local dir="$(__gitdir)" root="${2-.}"
>> +
>> +       if [ -d "$dir" ]; then
>> +               __git_ls_files_helper "$root" "$1" | __git_index_file_list_filter |
>> +                       sort | uniq
>> +       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
>> +}
> 
> These two are exactly the same, except one calls
> __git_ls_files_helper, and the other one __git_diff_index_helper,
> can't we make another argument that is and select one or the other
> based on that?
> 

They are not exactly the same.

The first function requires, as first parameter, (space separed) options
to pass to ls-files command; the second function, instead, requires the
id of a tree object.

IMHO, using only one function may be confusing.

>>  __git_heads ()
>>  {
>>         local dir="$(__gitdir)"
>> @@ -430,6 +543,46 @@ __git_complete_revlist_file ()
>>  }
>>
>>
>> +# __git_complete_index_file requires 1 argument: the options to pass to
>> +# ls-file
>> +__git_complete_index_file ()
>> +{
>> +       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
>> +}
>> +
>> +# __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
>> +}
> 
> These are also exactly the same, we could pass the argument to the
> function above.
> 

See previous note.



Regards  Manlio Perillo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlF2p+QACgkQscQJ24LbaUQVcACfeYFO8umJDbgTrXWChqqbk69E
CE4AniZFP7PQkOZCbBY+6hZ2gMpNIJTn
=HqAf
-----END PGP SIGNATURE-----

Re: [PATCH v5] git-completion.bash: add support for path completion

[Felipe Contreras]

On Tue, Apr 23, 2013 at 10:25 AM, Manlio Perillo
<manlio.perillo@gmail.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Il 21/04/2013 12:14, Felipe Contreras ha scritto:
>> On Fri, Jan 11, 2013 at 12:48 PM, Manlio Perillo
>> <manlio.perillo@gmail.com> wrote:
>>> The git-completion.bash script did not implemented full, git aware,
>>> support to complete paths, for git commands that operate on files within
>>> the current working directory or the index.
>>
>>> +__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 ()
>>> +{
>>> +       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%%/*}" ;;
>>
>> Which version of bash is that? It works perfectly fine here with or
>> without the /.
>>
>
> GNU bash, version 4.1.5(1)-release (i486-pc-linux-gnu)
> on a GNU Linux Debian 6

I compiled 4.1 and I can't reproduce, and I tried on a debian squeeze
chroot and I also can't reproduce. What am I missing?

GNU bash, version 4.1.5(1)-release (x86_64-pc-linux-gnu)

>>> +# __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
>>> +#    slash.
>>> +__git_index_files ()
>>> +{
>>> +       local dir="$(__gitdir)" root="${2-.}"
>>> +
>>> +       if [ -d "$dir" ]; then
>>> +               __git_ls_files_helper "$root" "$1" | __git_index_file_list_filter |
>>> +                       sort | uniq
>>> +       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
>>> +}
>>
>> These two are exactly the same, except one calls
>> __git_ls_files_helper, and the other one __git_diff_index_helper,
>> can't we make another argument that is and select one or the other
>> based on that?
>>
>
> They are not exactly the same.
>
> The first function requires, as first parameter, (space separed) options
> to pass to ls-files command; the second function, instead, requires the
> id of a tree object.
>
> IMHO, using only one function may be confusing.

The functions down in the call-stack might be doing something
different but these functions themselves are not. At the end of the
day they simply pass arguments to ls-files or diff-index.

I'm certain these can be simplified greatly.

Cheers.

-- 
Felipe Contreras