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

[PATCH v4] 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 the follow:

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

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

Changes from version 3:

	* Fixed quoting issues
	* Fixed default parameters handling
	* Fixed a typo in the commit message: the affected command was ls-files,
	  not ls-tree.
	* Fixed incorrect behavior when expanding a path in "git commit"
	  command, for a newly created repository (when HEAD does not
	  exists).
	* Make sure to always execute git commands with stderr redirected to
	  /dev/null.
	* Improved path completion for the git mv command.
	  This required a small refactorization of the __git_index_files
	  function, in order to support multiple options for ls-files.

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

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 0b77eb1..c8c6464 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,62 @@ __gitcomp_nl ()
 	COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- "${3-$cur}"))
 }
 
+# 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.
+# It accepts 1 optional argument: a directory path.  The path must have
+# a trailing slash.
+__git_index_file_list_filter ()
+{
+	local pfx="${1-}" offset=${#pfx} path
+
+	while read -r path; do
+		path="${path:$offset}"
+
+		case "$path" in
+		?*/*) echo "${path%%/*}/" ;;
+		*) echo $path ;;
+		esac
+	done
+}
+
+# __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)"
+
+	if [ -d "$dir" ]; then
+		# NOTE: $1 is not quoted in order to support multiple options
+		git --git-dir="$dir" ls-files --exclude-standard $1 ${2+"$2"} 2>/dev/null |
+			__git_index_file_list_filter ${2+"$2"} |
+			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)"
+
+	if [ -d "$dir" ]; then
+		git --git-dir="$dir" diff-index --name-only "$1" 2>/dev/null |
+			__git_index_file_list_filter ${2+"$2"} |
+			uniq
+	fi
+}
+
 __git_heads ()
 {
 	local dir="$(__gitdir)"
@@ -430,6 +487,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_nl "$(__git_index_files "$1" "$pfx")" "$pfx" "$cur_" ""
+		;;
+	*)
+		__gitcomp_nl "$(__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_nl "$(__git_diff_index_files "$1" "$pfx")" "$pfx" "$cur_" ""
+		;;
+	*)
+		__gitcomp_nl "$(__git_diff_index_files "$1")" "" "$cur_" ""
+		;;
+	esac
+}
+
 __git_complete_file ()
 {
 	__git_complete_revlist_file
@@ -770,8 +867,6 @@ _git_apply ()
 
 _git_add ()
 {
-	__git_has_doubledash && return
-
 	case "$cur" in
 	--*)
 		__gitcomp "
@@ -780,7 +875,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 +1027,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 +1066,6 @@ _git_clone ()
 
 _git_commit ()
 {
-	__git_has_doubledash && return
-
 	case "$cur" in
 	--cleanup=*)
 		__gitcomp "default strip verbatim whitespace
@@ -998,7 +1093,13 @@ _git_commit ()
 			"
 		return
 	esac
-	COMPREPLY=()
+
+	if git rev-parse --verify --quiet HEAD 1>/dev/null; then
+		__git_complete_diff_index_file "HEAD"
+	else
+		# This is the first commit
+		__git_complete_index_file "--cached"
+	fi
 }
 
 _git_describe ()
@@ -1216,8 +1317,6 @@ _git_init ()
 
 _git_ls_files ()
 {
-	__git_has_doubledash && return
-
 	case "$cur" in
 	--*)
 		__gitcomp "--cached --deleted --modified --others --ignored
@@ -1230,7 +1329,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 ()
@@ -1362,7 +1464,14 @@ _git_mv ()
 		return
 		;;
 	esac
-	COMPREPLY=()
+
+	if [ $cword -gt 2 ]; 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 ()
@@ -2068,15 +2177,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 ()
-- 
1.8.1.rc1.18.g9db0d25

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

[Junio C Hamano]

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

> +		case "$path" in
> +		?*/*) echo "${path%%/*}/" ;;
> +		*) echo $path ;;

$path unquoted???

> +# __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)"
> +
> +	if [ -d "$dir" ]; then
> +		# NOTE: $1 is not quoted in order to support multiple options

Good thinking to document this.  Thanks.

I take it that $1 never comes from the end user and it is known that
it is correct to split them at $IFS?  That is the way I read callers
of this function in this patch, but I am just double-checking.

> @@ -998,7 +1093,13 @@ _git_commit ()
>  			"
>  		return
>  	esac
> -	COMPREPLY=()
> +
> +	if git rev-parse --verify --quiet HEAD 1>/dev/null; then

s/1>/>/;

> +		__git_complete_diff_index_file "HEAD"

As this runs "git diff-index" without --cached, 

The completion will give only for paths that have difference between
the working tree and the HEAD.  If the user has a bogus contents
that was "git add"ed earlier, (i.e. the index is different from
HEAD), then realizes the mistake and fixes it in the working tree
with his editor to match "HEAD" (i.e. the working tree is the same
as HEAD):

	git commit the-prefix-to-that-file<TAB>

to complete the filename will not give that file.  I do not think it
is a show-stopper, but it may puzzle the users when they encounter
the situation.

I am wondering if reading from "git status --porcelain" might be a
better alternative, or if it is too much trouble and slow things
down to cover such a corner case.

> @@ -1362,7 +1464,14 @@ _git_mv ()
>  		return
>  		;;
>  	esac
> -	COMPREPLY=()
> +
> +	if [ $cword -gt 2 ]; 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

Is $cword affected by the presense of "-f" in "git mv [-f] foo bar"?
Just being curious.

Other than that, I do not see anything majorly wrong from the coding
and semantics point of view in the patch.  As to the interaction
with the rest of the completion machinery, I'll leave the review to
the area experts CC'ed and wait for their comments.

Thanks.

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

[Manlio Perillo]

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

Il 21/12/2012 18:59, Junio C Hamano ha scritto:
> Manlio Perillo <manlio.perillo@gmail.com> writes:
> 
>> +		case "$path" in
>> +		?*/*) echo "${path%%/*}/" ;;
>> +		*) echo $path ;;
> 
> $path unquoted???
> 

Missed again, thanks.
I hope this is really the last one!

> [...]
>> +__git_index_files ()
>> +{
>> +	local dir="$(__gitdir)"
>> +
>> +	if [ -d "$dir" ]; then
>> +		# NOTE: $1 is not quoted in order to support multiple options
> 
> Good thinking to document this.  Thanks.
> 
> I take it that $1 never comes from the end user and it is known that
> it is correct to split them at $IFS?  That is the way I read callers
> of this function in this patch, but I am just double-checking.
> 

Yes, $1 is always set internally (but I will check again)
Probably there are better solutions.


>> @@ -998,7 +1093,13 @@ _git_commit ()
>>  			"
>>  		return
>>  	esac
>> -	COMPREPLY=()
>> +
>> +	if git rev-parse --verify --quiet HEAD 1>/dev/null; then
> 
> s/1>/>/;
> 

What is the reason?
Coding style?

>> +		__git_complete_diff_index_file "HEAD"
> 
> As this runs "git diff-index" without --cached, 
> 
> The completion will give only for paths that have difference between
> the working tree and the HEAD.  If the user has a bogus contents
> that was "git add"ed earlier, (i.e. the index is different from
> HEAD), then realizes the mistake and fixes it in the working tree
> with his editor to match "HEAD" (i.e. the working tree is the same
> as HEAD):
> 
> 	git commit the-prefix-to-that-file<TAB>
> 
> to complete the filename will not give that file.  I do not think it
> is a show-stopper, but it may puzzle the users when they encounter
> the situation.
> 

Umh, I just checked this case.

  0) git init test
  1) Added a README file with "Hello World.", and committed.
  2) Modified the README file with "Hello World!" and executed
     git add README
  3) Modified the README file with "Hello World." (the original content)
  4) $ git diff HEAD:README README
     $ git diff-index --name-only HEAD
     README

     git commit <TAB> shows the README file.

If I understand correctly the documentation of diff-index, it will
always compare the content of the index with HEAD.
If --cached is specified, it will ignore the stat state of the file on disk.


> I am wondering if reading from "git status --porcelain" might be a
> better alternative, or if it is too much trouble and slow things
> down to cover such a corner case.
> 

It have considered it.

The problem is that the output of "git status --porcelain" is not
trivial to parse.


>> @@ -1362,7 +1464,14 @@ _git_mv ()
>>  		return
>>  		;;
>>  	esac
>> -	COMPREPLY=()
>> +
>> +	if [ $cword -gt 2 ]; 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
> 
> Is $cword affected by the presense of "-f" in "git mv [-f] foo bar"?
> Just being curious.
> 

Yes, it is affected; I missed it, thanks.
It should count only non-option arguments.


> Other than that, I do not see anything majorly wrong from the coding
> and semantics point of view in the patch.  As to the interaction
> with the rest of the completion machinery, I'll leave the review to
> the area experts CC'ed and wait for their comments.
> 
> Thanks.
> 

Thanks for your review.


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

iEYEARECAAYFAlDUsjwACgkQscQJ24LbaUSGuwCffon7/VGFo98CCBsZlmOdNYYE
91oAn3X8fbr5jtzMUOZkMp9CuGWCa7Cf
=Qzsv
-----END PGP SIGNATURE-----

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

[Marc Khouzam]

> -----Original Message-----
> From: git-owner@vger.kernel.org 
> [mailto:git-owner@vger.kernel.org] On Behalf Of Manlio Perillo
> Sent: Friday, December 21, 2012 11:55 AM
> To: git@vger.kernel.org
> Cc: szeder@ira.uka.de; felipe.contreras@gmail.com; Manlio Perillo
> Subject: [PATCH v4] git-completion.bash: add support for path 
> completion
> 
> 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.

I think this is a great improvement!  Very nice.

I've been playing with it but I'm not getting the expected 
behavior when I cd to a sub-directory.  Instead I get all files
in the repo as proposals.  I'm trying it in the git git-repository
itself.  Here is a sample:

> git status
# On branch pu
nothing to commit, working directory clean
> source contrib/completion/git-completion.bash
> touch contrib/test1
> touch contrib/test2
> git status
# On branch pu
# Untracked files:
#   (use "git add <file>..." to include in what will be committed)
#
#       contrib/test1
#       contrib/test2
nothing added to commit but untracked files present (use "git add" to track)
> git add <TAB> # this works as expected and I get: contrib/test1  contrib/test2  
> cd contrib/
> git add <TAB>
Display all 387 possibilities? (y or n)  # That is not right. Shouldn't I get
                                         # the same two files only?

Maybe I mis-understood what should happen?

Besides that, without looking at the patch in detail, I put just a couple
of minor points below.

> 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 the follow:

s/as the follow/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.
> 
> Signed-off-by: Manlio Perillo <manlio.perillo@gmail.com>
> ---
> 
> Changes from version 3:
> 
> 	* Fixed quoting issues
> 	* Fixed default parameters handling
> 	* Fixed a typo in the commit message: the affected 
> command was ls-files,
> 	  not ls-tree.
> 	* Fixed incorrect behavior when expanding a path in "git commit"
> 	  command, for a newly created repository (when HEAD does not
> 	  exists).
> 	* Make sure to always execute git commands with stderr 
> redirected to
> 	  /dev/null.
> 	* Improved path completion for the git mv command.
> 	  This required a small refactorization of the __git_index_files
> 	  function, in order to support multiple options for ls-files.
> 
>  contrib/completion/git-completion.bash | 140 
> +++++++++++++++++++++++++++++----
>  1 file changed, 124 insertions(+), 16 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index 0b77eb1..c8c6464 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,62 @@ __gitcomp_nl ()
>  	COMPREPLY=($(compgen -P "${2-}" -S "${4- }" -W "$1" -- 
> "${3-$cur}"))
>  }
>  
> +# 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.
> +# It accepts 1 optional argument: a directory path.  The 
> path must have
> +# a trailing slash.
> +__git_index_file_list_filter ()
> +{
> +	local pfx="${1-}" offset=${#pfx} path
> +
> +	while read -r path; do
> +		path="${path:$offset}"
> +
> +		case "$path" in
> +		?*/*) echo "${path%%/*}/" ;;
> +		*) echo $path ;;
> +		esac
> +	done
> +}
> +
> +# __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)"
> +
> +	if [ -d "$dir" ]; then
> +		# NOTE: $1 is not quoted in order to support 
> multiple options
> +		git --git-dir="$dir" ls-files 
> --exclude-standard $1 ${2+"$2"} 2>/dev/null |
> +			__git_index_file_list_filter ${2+"$2"} |
> +			uniq

Will the output piped to uniq always be sorted?  It has to be.
If it is not, you must use
  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)"
> +
> +	if [ -d "$dir" ]; then
> +		git --git-dir="$dir" diff-index --name-only 
> "$1" 2>/dev/null |
> +			__git_index_file_list_filter ${2+"$2"} |
> +			uniq

Will the output piped to uniq always be sorted?  It has to be.
If it is not, you must use
  sort | uniq

> +	fi
> +}
> +
>  __git_heads ()
>  {
>  	local dir="$(__gitdir)"
> @@ -430,6 +487,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_nl "$(__git_index_files "$1" "$pfx")" 
> "$pfx" "$cur_" ""
> +		;;
> +	*)
> +		__gitcomp_nl "$(__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_nl "$(__git_diff_index_files "$1" 
> "$pfx")" "$pfx" "$cur_" ""
> +		;;
> +	*)
> +		__gitcomp_nl "$(__git_diff_index_files "$1")" 
> "" "$cur_" ""
> +		;;
> +	esac
> +}
> +
>  __git_complete_file ()
>  {
>  	__git_complete_revlist_file
> @@ -770,8 +867,6 @@ _git_apply ()
>  
>  _git_add ()
>  {
> -	__git_has_doubledash && return
> -
>  	case "$cur" in
>  	--*)
>  		__gitcomp "
> @@ -780,7 +875,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 +1027,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 +1066,6 @@ _git_clone ()
>  
>  _git_commit ()
>  {
> -	__git_has_doubledash && return
> -
>  	case "$cur" in
>  	--cleanup=*)
>  		__gitcomp "default strip verbatim whitespace
> @@ -998,7 +1093,13 @@ _git_commit ()
>  			"
>  		return
>  	esac
> -	COMPREPLY=()
> +
> +	if git rev-parse --verify --quiet HEAD 1>/dev/null; then
> +		__git_complete_diff_index_file "HEAD"
> +	else
> +		# This is the first commit
> +		__git_complete_index_file "--cached"
> +	fi
>  }
>  
>  _git_describe ()
> @@ -1216,8 +1317,6 @@ _git_init ()
>  
>  _git_ls_files ()
>  {
> -	__git_has_doubledash && return
> -
>  	case "$cur" in
>  	--*)
>  		__gitcomp "--cached --deleted --modified 
> --others --ignored
> @@ -1230,7 +1329,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 ()
> @@ -1362,7 +1464,14 @@ _git_mv ()
>  		return
>  		;;
>  	esac
> -	COMPREPLY=()
> +
> +	if [ $cword -gt 2 ]; 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 ()
> @@ -2068,15 +2177,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 ()
> -- 
> 1.8.1.rc1.18.g9db0d25
> 
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

[Junio C Hamano]

Marc Khouzam <marc.khouzam@ericsson.com> writes:

> I've been playing with it but I'm not getting the expected 
> behavior when I cd to a sub-directory.

Thanks for testing.  Manlio?

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Marc Khouzam <marc.khouzam@ericsson.com> writes:
>
>> I've been playing with it but I'm not getting the expected 
>> behavior when I cd to a sub-directory.
>
> Thanks for testing.  Manlio?

Can you try the attached patch?

As I am not familiar with the completion machinery, take this with a
large grain of salt.  Here is my explanation of what is going on in
this "how about this" fixup:

 * Giving --git-dir from the command line (or GIT_DIR environment)
   without specifying GIT_WORK_TREE is to signal Git that you are at
   the top of the working tree.  "git ls-files" will then show the
   full tree even outside the real $cwd because you are lying to
   Git.

 * "git diff-index" could be told to show only the $cwd and its
   subdirectory with the "--relative" option, but it alone is not
   sufficient if you throw --git-dir at it; again, you end up lying
   that you are at the top.

 * As far as I can tell, there is no reason you would want to pass
   "--git-dir" to these invocations of ls-files and diff-index.  If
   the previous call to "__gitdir" could figure out where it is, the
   command should be able to figure it out the same way.
   
There seem to be millions of other existing "git --git-dir=$there"
in this script.  As I already said, I am not familiar with the
completion machinery, and I do not know what they are for in the
first place.  Perhaps people put them there for a reason, but I do
not know what that reason is.

I think the ones for "for-each-ref", "config" and "stash" should be
harmless.  They are commands that do not care about the working
tree.

There is one given to "ls-tree" used in __git_complete_revlist_file;
I do not know if this was intended, what it is for, and if that is
working as intended, though.

I've been CC'ing two people who touched this script rather heavily,
are expected to know the completion machinery, and should be able to
help polishing this topic and moving it forward.  Perhaps one of
them can shed light on this.

-- >8 --
Subject: completion: do not pass harmful "--git-dir=$there"

The recently added __git_index_files and __git_diff_index_files
helper functions invoke "ls-files" and "diff_index" while explicitly
passing "--git-dir=$there", to tell them that the invocation is done
at the top of the working tree, which may not be the case when the
user is in a subdirectory.  Remove the harmful use of this option,

Tell "diff-index" to show only the paths in the $cwd and show them
relative to the $cwd by passing "--relative". The "ls-files" does
not need this, as that is already its default mode of operation.

---
 contrib/completion/git-completion.bash | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index c8c6464..f4bd548 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -267,9 +267,9 @@ __git_index_files ()
 
 	if [ -d "$dir" ]; then
 		# NOTE: $1 is not quoted in order to support multiple options
-		git --git-dir="$dir" ls-files --exclude-standard $1 ${2+"$2"} 2>/dev/null |
-			__git_index_file_list_filter ${2+"$2"} |
-			uniq
+		git ls-files --exclude-standard $1 ${2+"$2"} 2>/dev/null |
+		__git_index_file_list_filter ${2+"$2"} |
+		uniq
 	fi
 }
 
@@ -284,9 +284,9 @@ __git_diff_index_files ()
 	local dir="$(__gitdir)"
 
 	if [ -d "$dir" ]; then
-		git --git-dir="$dir" diff-index --name-only "$1" 2>/dev/null |
-			__git_index_file_list_filter ${2+"$2"} |
-			uniq
+		git diff-index --name-only --relative "$1" 2>/dev/null |
+		__git_index_file_list_filter ${2+"$2"} |
+		uniq
 	fi
 }
 

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

[Marc Khouzam]

> Junio C Hamano <gitster@pobox.com> writes:
> 
>>  Marc Khouzam <marc.khouzam@ericsson.com> writes:
>>
>>> I've been playing with it but I'm not getting the expected
>>> behavior when I cd to a sub-directory.
>>
>> Thanks for testing.  Manlio?
> 
> Can you try the attached patch?

Thanks for this, it improves the situation dramatically.
I did further testing with your patch and found some less obvious
issues.  I didn't debug the script myself as I'm not that familiar with
it either, but I think the testcases below should help Manlio or
someone else look into some regressions.

1- Using .. or . breaks completion when after the '/':
> cd git/contrib
> git rm ../contrib/completion/<tab>
../contrib/completion/ion.bash  ../contrib/completion/ion.tcsh  ../contrib/completion/ion.zsh   ../contrib/completion/sh
It looks like the space taken by the path where we are located, eats up the same number of characters from the file name, e.g.,
   ../contrib/completion/ion.bash
   ../contrib/git-completion.bash

2- Maybe related to problem 1.  Using .. breaks completion in other ways:
> cd git/contrib
> git rm ../con<tab>
../config.c       ../config.mak.in  ../configure.ac   ../connect.c      ../connected.c    ../connected.h    ../convert.c      ../convert.h
Notice that 'contrib' is not proposed.
Don't be fooled by the fact that
> git rm ../cont<tab>
will complete to 
> git rm ../contrib
as it is only because the completion script returned nothing, and bash file-completion 
kicked-in instead (it fooled me :)).

3- Also probably related to problems 1 and 2.  Using absolute paths behaves wierdly and 
worse than before:
git rm /home/marc/git/git/con<tab>
will give
git rm /home/marc/git/git/config-
although there is the 'contrib' dir that should be shown, amongst others.
Seems like the same problem as above with 'git rm ../con<tab>'

In my opinion, the above three cases are regressions.

4- Completion choices include their entire path, which is not what bash does by default.  For example:
> cd git/contrib
> ls completion/git-<tab>
git-completion.bash  git-completion.tcsh  git-completion.zsh   git-prompt.sh
but
> git rm completion/git-<tab>
completion/git-completion.bash  completion/git-completion.tcsh  completion/git-completion.zsh   completion/git-prompt.sh
notice the extra 'completion/' before each completion.  This can get pretty large when completing with 
many directory prefixes.  The current tcsh completion has the same problem which I couldn't fix.  However, I am 
not sure if it can be fixed for bash.

I personally don't think this is regression, just an slight annoyance.

5- With this feature git-completion.bash will return directories as completions.  This is something
git-completion.tcsh is not handling very well.  I will post a patch to fix that.

Below are two suggestions that are in line with this effort but that are not regressions.

A) It would be nice if 
git commit -a <TAB>
also completed with untracked files

B) Now that, for example, 'git rm' completion is smart about path completion 
it would be nice to somehow not trigger bash default file completion
when 'git rm' does not report any completions.  

For example, if I have a file called zz.tar.gz (which is an ignored file) 
and I do 'git rm <tab>', I will get the proper list of files that can be
removed by git, excluding zz.tar.gz.  But if I complete
'git rm zz.tar.<tab>' then the completion script will return nothing,
since git cannot remove that ignored file, but we will then fall-back
to the bash default completion, which will complete the file to zz.tar.gz.

Although there are some issues, I think this feature will greatly benefit the user
and is worth the time needed to fix.

Thanks!

Marc

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

[Manlio Perillo]

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

Il 05/01/2013 21:23, Marc Khouzam ha scritto:
> [...]
> I did further testing with your patch and found some less obvious
> issues.  I didn't debug the script myself as I'm not that familiar with
> it either, but I think the testcases below should help Manlio or
> someone else look into some regressions.
> 
> 1- Using .. or . breaks completion when after the '/':
> [...] 
> 2- Maybe related to problem 1.  Using .. breaks completion in other ways:
> [...]
> 3- Also probably related to problems 1 and 2.  Using absolute paths behaves wierdly and 
> worse than before:

Thanks for this.

I begin to suspect that this is the reason why path completion has not
been implemented by the original author of the bash completion script.

These issues seems hard to fix.

Tomorrow I will take some time to try to fix all the reported issues.

> [...]


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

iEYEARECAAYFAlDomskACgkQscQJ24LbaUQJdwCfX0bMq3V88soqtf+xlypZ5D4f
qwAAn3bK7UlcOK+hm+u06jmT05l1aJVf
=IWap
-----END PGP SIGNATURE-----

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

[Manlio Perillo]

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

Il 05/01/2013 07:27, Junio C Hamano ha scritto:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Marc Khouzam <marc.khouzam@ericsson.com> writes:
>>
>>> I've been playing with it but I'm not getting the expected 
>>> behavior when I cd to a sub-directory.
>>
>> Thanks for testing.  Manlio?
> 
> Can you try the attached patch?
> 

Thanks, it seems to fix the problem.

> As I am not familiar with the completion machinery, take this with a
> large grain of salt.  Here is my explanation of what is going on in
> this "how about this" fixup:
> 
>  * Giving --git-dir from the command line (or GIT_DIR environment)
>    without specifying GIT_WORK_TREE is to signal Git that you are at
>    the top of the working tree.  "git ls-files" will then show the
>    full tree even outside the real $cwd because you are lying to
>    Git.
> 

I was not aware of this, and blindly copied the code from the other
existing functions.
However the other completion functions never have to deal with paths in
the working directory.


I have applied the patch to my local branch.


> [...]


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

iEYEARECAAYFAlDpu7UACgkQscQJ24LbaUSmUACgl+OKUyvpp183kFZGmBpOfqm1
yqEAnjxcqmZYvWSeIpOo6cNFl/dnMH76
=oE/+
-----END PGP SIGNATURE-----

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

[Manlio Perillo]

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

Il 05/01/2013 21:23, Marc Khouzam ha scritto:
> [...]
> Thanks for this, it improves the situation dramatically.
> I did further testing with your patch and found some less obvious
> issues.  I didn't debug the script myself as I'm not that familiar with
> it either, but I think the testcases below should help Manlio or
> someone else look into some regressions.
> 
> 1- Using .. or . breaks completion when after the '/':
> [...]
> 2- Maybe related to problem 1.  Using .. breaks completion in other ways:
> [...]
> 3- Also probably related to problems 1 and 2.  Using absolute paths behaves wierdly and 
> worse than before:

> In my opinion, the above three cases are regressions.
>

Yes.
I did not considered this use case, thanks!
I have never done something like this when working with Mercurial.

The problem is caused by the __git_index_file_list_filter function.

The job of this function is to stop path completion at directory
boundary (in order to avoid to suggest files in child
directories [1]), and to make paths relative to current directory.

Unfortunately, what it does is to simply remove the prefix string from
the path name; of course this will not work when the prefix is a non
canonical path name.

The solution is quite simple: canonicalize both the prefix path and each
of the path name returned by git.

This can be done using `readlink -f "$path"` or `realpath $path`, but
the problem is that it is inefficient to execute an external command for
each of the path returned by git; moreover readlink and realpath are not
POSIX and may not be supported on all platforms where git works (but I
found a portable implementation using pushd, popd, `pwd`,  `dirname`,
`basename` -- not very efficient).

IMHO, the best solution is to recode __git_index_file_list_filter in Perl.

Another possible solution (as suggested by Junio) is to use the
- --relative option; unfortunately this is only supported by
`git diff-index` and not by `git ls-files`.
And it will not solve the problem when using absolute path names (but
this case can be handled by leaving path completion to bash).

> 4- Completion choices include their entire path, which is not what bash does by default.  For example:
>> cd git/contrib
>> ls completion/git-<tab>
> git-completion.bash  git-completion.tcsh  git-completion.zsh   git-prompt.sh
> but
>> git rm completion/git-<tab>
> completion/git-completion.bash  completion/git-completion.tcsh  completion/git-completion.zsh   completion/git-prompt.sh
> notice the extra 'completion/' before each completion.

This is another thing I missed.
The problem is that only the current directory is removed from the path
names returned by git.

>  This can get pretty large when completing with 
> many directory prefixes.  The current tcsh completion has the same problem which I couldn't fix.  However, I am 
> not sure if it can be fixed for bash.
> 

The fix was very easy, and it seems to work.
The problem is in the __git_complete_index_files and
__git_complete_diff_index_files function.

When calling the __git_index_files and git_index_files, the "$cur"
variable should be used, instead of the computed "$pfx".

Not sure if this is correct.
I will post the patch, so you can test it.

> I personally don't think this is regression, just an slight annoyance.
> 
> 5- With this feature git-completion.bash will return directories as completions.  This is something
> git-completion.tcsh is not handling very well.  I will post a patch to fix that.
> 

I'll pass on this, thanks.

> Below are two suggestions that are in line with this effort but that are not regressions.
> 
> A) It would be nice if 
> git commit -a <TAB>
> also completed with untracked files
> 

I agree.
And there are other places when it may be useful to check the passed
options (see the comments).

But I think it is better to leave these issues for the future.
I will just add a comment to take note of this use case.

> B) Now that, for example, 'git rm' completion is smart about path completion 
> it would be nice to somehow not trigger bash default file completion
> when 'git rm' does not report any completions.  
> 

Not sure how this can be done, but it is possible and should be easy.

> For example, if I have a file called zz.tar.gz (which is an ignored file) 
> and I do 'git rm <tab>', I will get the proper list of files that can be
> removed by git, excluding zz.tar.gz.  But if I complete
> 'git rm zz.tar.<tab>' then the completion script will return nothing,
> since git cannot remove that ignored file, but we will then fall-back
> to the bash default completion, which will complete the file to zz.tar.gz.
> 
> Although there are some issues, I think this feature will greatly benefit the user
> and is worth the time needed to fix.
> 
> Thanks!
> 
> Marc


Thanks to you for the review!

Regards   Manlio


[1] this is what the Mercurial bash completion script does
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAlDpxNEACgkQscQJ24LbaURZEgCcD2Uc+7/W+RCrMk3j+vrd5w36
6ogAn1ou4pOarBSMywaQ3zQKdZmofyKA
=iU13
-----END PGP SIGNATURE-----

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

[Manlio Perillo]

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

Il 05/01/2013 21:23, Marc Khouzam ha scritto:
> [...]
> Below are two suggestions that are in line with this effort but that are not regressions.
> 
> A) It would be nice if 
> git commit -a <TAB>
> also completed with untracked files
> 

$ git commit -a foo
fatal: Paths with -a does not make sense.

So
  git commit -a <TAB>

should not suggest untracked files; instead it should suggest nothing.

> [...]


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

iEYEARECAAYFAlDq0PoACgkQscQJ24LbaUSTNwCfWt7a1Tdg9u5sd6B3FXCEFj1/
sBwAoIv4B2y4MUQgLNafY2PTWx4giSfD
=tb3O
-----END PGP SIGNATURE-----

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

[Marc Khouzam]

> -----Original Message-----
> From: git-owner@vger.kernel.org 
> [mailto:git-owner@vger.kernel.org] On Behalf Of Manlio Perillo
> Sent: Monday, January 07, 2013 8:43 AM
> To: Marc Khouzam
> Cc: Junio C Hamano; git@vger.kernel.org; szeder@ira.uka.de; 
> felipe.contreras@gmail.com
> Subject: Re: [PATCH v4] git-completion.bash: add support for 
> path completion
> 
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Il 05/01/2013 21:23, Marc Khouzam ha scritto:
> > [...]
> > Below are two suggestions that are in line with this effort 
> but that are not regressions.
> > 
> > A) It would be nice if 
> > git commit -a <TAB>
> > also completed with untracked files
> > 
> 
> $ git commit -a foo
> fatal: Paths with -a does not make sense.
> 
> So
>   git commit -a <TAB>
> 
> should not suggest untracked files; instead it should suggest nothing.

You are right, I was confused.

git commit --all <TAB>

should also suggest nothing then.

Thanks

Marc

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

[Manlio Perillo]

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

Il 05/01/2013 21:23, Marc Khouzam ha scritto:
> [...]
> 4- Completion choices include their entire path, which is not what bash does by default.  For example:
>> cd git/contrib
>> ls completion/git-<tab>
> git-completion.bash  git-completion.tcsh  git-completion.zsh   git-prompt.sh
> but
>> git rm completion/git-<tab>
> completion/git-completion.bash  completion/git-completion.tcsh  completion/git-completion.zsh   completion/git-prompt.sh
> notice the extra 'completion/' before each completion.  This can get pretty large when completing with 
> many directory prefixes.  The current tcsh completion has the same problem which I couldn't fix.  However, I am 
> not sure if it can be fixed for bash.
> 
> I personally don't think this is regression, just an slight annoyance.
> 

After some searching, I found how this is supposed to be done.
It is possible to use the -o filenames option to tell Bash completion
that "the compspec generates filenames, so it can perform any
filename-specific processing".

Unfortunately this option must be passed to the complete builtin
command, and we can not do this, since the comspec not always contains
filenames.

> [...]


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

iEYEARECAAYFAlDsXUEACgkQscQJ24LbaURMlgCdEyeSRTRktKtGuDxq4HX1meWt
IV4AmwS6wasCip+1u4vS2FwG8AlXXB7r
=pN8F
-----END PGP SIGNATURE-----

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

[John Keeping]

On Tue, Jan 08, 2013 at 06:54:09PM +0100, Manlio Perillo wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Il 05/01/2013 21:23, Marc Khouzam ha scritto:
>> [...]
>> 4- Completion choices include their entire path, which is not what bash does by default.  For example:
>>> cd git/contrib
>>> ls completion/git-<tab>
>> git-completion.bash  git-completion.tcsh  git-completion.zsh   git-prompt.sh
>> but
>>> git rm completion/git-<tab>
>> completion/git-completion.bash  completion/git-completion.tcsh  completion/git-completion.zsh   completion/git-prompt.sh
>> notice the extra 'completion/' before each completion.  This can get pretty large when completing with 
>> many directory prefixes.  The current tcsh completion has the same problem which I couldn't fix.  However, I am 
>> not sure if it can be fixed for bash.
>> 
>> I personally don't think this is regression, just an slight annoyance.
>> 
> 
> After some searching, I found how this is supposed to be done.
> It is possible to use the -o filenames option to tell Bash completion
> that "the compspec generates filenames, so it can perform any
> filename-specific processing".
> 
> Unfortunately this option must be passed to the complete builtin
> command, and we can not do this, since the comspec not always contains
> filenames.

You should also be able to pass it to 'compopt' during completion in
order to change the behaviour for only the current completion.


John

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

[Manlio Perillo]

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

Il 08/01/2013 19:05, John Keeping ha scritto:
> [...]
>>
>> After some searching, I found how this is supposed to be done.
>> It is possible to use the -o filenames option to tell Bash completion
>> that "the compspec generates filenames, so it can perform any
>> filename-specific processing".
>>
>> Unfortunately this option must be passed to the complete builtin
>> command, and we can not do this, since the comspec not always contains
>> filenames.
> 
> You should also be able to pass it to 'compopt' during completion in
> order to change the behaviour for only the current completion.
> 

Thanks, compopt is what I wanted.

I was reading an old Bash manual (for Bash 3.1), and compopt is only
available starting from Bash 4.0.

I will do some test, being careful to not break the code for Bash < 4.0
and the other supported shells.



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

iEYEARECAAYFAlDsZVgACgkQscQJ24LbaUQlAACdGbhOuGICCYFwkRTPJla+3JGT
EcQAoINEGvdwtOz1QFbAA4FqoI3c7VSa
=5Oqw
-----END PGP SIGNATURE-----