[PATCH v2] Make use of git status when autocompleting git add, rm, checkout --, and reset HEAD

[PATCH v2] Make use of git status when autocompleting git add, rm, checkout --, and reset HEAD

[Ron Panduwana]

Signed-off-by: Ron Panduwana <panduwana@gmail.com>
---

On Fri, Aug 19, 2011 at 5:10 PM, Thomas Rast <trast@student.ethz.ch> wrote:
> Some thoughts:
>
> * running git-status for . has some issues: it doesn't work in the
>  case of
>
>    cd subdir
>    git add ../some/file[TAB]
>
>  It's also inefficient if you are at the top level and
>
>    git add path/to/file/a/few/levels/down[TAB]
>
>  since it wouldn't actually have to look for untracked files in the
>  entire repo.

Fixed by running git-status for $cur if $cur is a directory. Otherwise run on .


> * -uall is not required unless you are looking for untracked files.
>   For the other commands you could speed up completion by passing
>   -uno instead.

Fixed by adding second parameter to __git_files_having_status


 contrib/completion/git-completion.bash |   84 ++++++++++++++++++++-----------
 1 files changed, 54 insertions(+), 30 deletions(-)

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8648a36..9d44501 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1010,6 +1010,16 @@ __git_has_doubledash ()
 	return 1
 }

+# __git_files_having_status requires 2 arguments
+__git_files_having_status ()
+{
+    local dir="."
+    if [ -d "$cur" ]; then
+        dir="$cur"
+    fi
+	echo "$(git status $2 -s "$dir" 2>/dev/null | egrep "^$1" | cut -c4-)"
+}
+
 __git_whitespacelist="nowarn warn error error-all fix"

 _git_am ()
@@ -1058,17 +1068,17 @@ _git_apply ()

 _git_add ()
 {
-	__git_has_doubledash && return
-
-	case "$cur" in
-	--*)
-		__gitcomp "
-			--interactive --refresh --patch --update --dry-run
-			--ignore-errors --intent-to-add
-			"
-		return
-	esac
-	COMPREPLY=()
+	if ! __git_has_doubledash; then
+		case "$cur" in
+		--*)
+			__gitcomp "
+				--interactive --refresh --patch --update --dry-run
+				--ignore-errors --intent-to-add
+				"
+			return
+		esac
+	fi
+	__gitcomp "$(__git_files_having_status "(.[MAU]|UD|\?\?)" -uall)"
 }

 _git_archive ()
@@ -1171,7 +1181,12 @@ _git_bundle ()

 _git_checkout ()
 {
-	__git_has_doubledash && return
+	if __git_has_doubledash; then
+		if [[ ${words[2]} = "--" ]]; then
+			__gitcomp "$(__git_files_having_status ".[MD]" -uno)"
+		fi
+		return
+	fi

 	case "$cur" in
 	--conflict=*)
@@ -1469,7 +1484,7 @@ _git_help ()
 	__gitcomp "$__git_all_commands $(__git_aliases)
 		attributes cli core-tutorial cvs-migration
 		diffcore gitk glossary hooks ignore modules
-		namespaces repository-layout tutorial tutorial-2
+		repository-layout tutorial tutorial-2
 		workflows
 		"
 }
@@ -2313,14 +2328,18 @@ _git_replace ()

 _git_reset ()
 {
-	__git_has_doubledash && return
-
-	case "$cur" in
-	--*)
-		__gitcomp "--merge --mixed --hard --soft --patch"
+	if ! __git_has_doubledash; then
+		case "$cur" in
+		--*)
+			__gitcomp "--merge --mixed --hard --soft --patch"
+			return
+			;;
+		esac
+	fi
+	if [[ ${words[2]} = "HEAD" ]]; then
+		__gitcomp "$(__git_files_having_status "[ADM]." -uno)"
 		return
-		;;
-	esac
+	fi
 	__gitcomp "$(__git_refs)"
 }

@@ -2337,15 +2356,20 @@ _git_revert ()

 _git_rm ()
 {
-	__git_has_doubledash && return
-
-	case "$cur" in
-	--*)
-		__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
-		return
-		;;
-	esac
-	COMPREPLY=()
+	if ! __git_has_doubledash; then
+		case "$cur" in
+		--*)
+			__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
+			return
+			;;
+		esac
+	fi
+	# check if --cached was specified
+	if [ "$(__git_find_on_cmdline "--cached")" ]; then
+		COMPREPLY=()
+	else
+		__gitcomp "$(__git_files_having_status "(.D|DU|UA)" -uno)"
+	fi
 }

 _git_shortlog ()
@@ -2640,7 +2664,6 @@ _git ()
 			--exec-path
 			--html-path
 			--work-tree=
-			--namespace=
 			--help
 			"
 			;;
@@ -2737,3 +2760,4 @@ else
 		shopt "$@"
 	}
 fi
+
--
1.7.1

Re: [PATCH v2] Make use of git status when autocompleting git add, rm, checkout --, and reset HEAD

[Junio C Hamano]

Ron Panduwana <panduwana@gmail.com> writes:

> Signed-off-by: Ron Panduwana <panduwana@gmail.com>

"Make use of" is something anybody can read from the patch. What we need
from the proposed commit log message above S-o-b: line is to justify why
it is a good change. Does it make the code simpler to follow by making it
shorter? Does it make it faster to complete, and if so by how much faster?
Does it make it easier to use by not including paths that it used to show,
and if so what are the differences the end users would see, and why is it
justified to omit these paths from the candidate set?

Re: [PATCH v2] Make use of git status when autocompleting git add, rm, checkout --, and reset HEAD

[SZEDER Gábor]

Hi,


On Wed, Aug 31, 2011 at 04:43:03AM +0700, Ron Panduwana wrote:
> Signed-off-by: Ron Panduwana <panduwana@gmail.com>

I agree with Junio that some explanation would be necessary here.

I'm not sure this change is in general a good idea.  I can imagine
that it would be useful for e.g. java projects, where you are forced
to have deep directory structures, but it will surely cause surprises
and confusion for users who trained their fingers to quickly enter
long paths with a few keystrokes and completion.  Furthermore, it
seems to cause considerable performance penalty, because it will fork
three processes and a subshell to connect them with a pipe for such a
"simple" thing as file name completion.  And one of those processes is
git status, which alone can take a while, especially for large
repositories, and as a side effect it refreshes the index (but that is
probably harmless).

> ---
> 
> On Fri, Aug 19, 2011 at 5:10 PM, Thomas Rast <trast@student.ethz.ch> wrote:
> > Some thoughts:
> >
> > * running git-status for . has some issues: it doesn't work in the
> >  case of
> >
> >    cd subdir
> >    git add ../some/file[TAB]
> >
> >  It's also inefficient if you are at the top level and
> >
> >    git add path/to/file/a/few/levels/down[TAB]
> >
> >  since it wouldn't actually have to look for untracked files in the
> >  entire repo.
> 
> Fixed by running git-status for $cur if $cur is a directory. Otherwise run on .

That won't do.  Imaginge you want to add ../some/file, and therefore
you do 'git add ../so<TAB>'.  In this case $cur will hold '../so',
which is not a directory (or at least not the directory you are
looking for).

> > * -uall is not required unless you are looking for untracked files.
> >   For the other commands you could speed up completion by passing
> >   -uno instead.
> 
> Fixed by adding second parameter to __git_files_having_status
> 
> 
>  contrib/completion/git-completion.bash |   84 ++++++++++++++++++++-----------
>  1 files changed, 54 insertions(+), 30 deletions(-)
> 
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 8648a36..9d44501 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1010,6 +1010,16 @@ __git_has_doubledash ()
>  	return 1
>  }
> 
> +# __git_files_having_status requires 2 arguments
> +__git_files_having_status ()
> +{
> +    local dir="."
> +    if [ -d "$cur" ]; then
> +        dir="$cur"
> +    fi

Please use tabs for indentation.

> +	echo "$(git status $2 -s "$dir" 2>/dev/null | egrep "^$1" | cut -c4-)"

Echoing a command substitution is unnecessary.

> +}
> +
>  __git_whitespacelist="nowarn warn error error-all fix"
> 
>  _git_am ()
> @@ -1058,17 +1068,17 @@ _git_apply ()
> 
>  _git_add ()
>  {
> -	__git_has_doubledash && return
> -
> -	case "$cur" in
> -	--*)
> -		__gitcomp "
> -			--interactive --refresh --patch --update --dry-run
> -			--ignore-errors --intent-to-add
> -			"
> -		return
> -	esac
> -	COMPREPLY=()
> +	if ! __git_has_doubledash; then
> +		case "$cur" in
> +		--*)
> +			__gitcomp "
> +				--interactive --refresh --patch --update --dry-run
> +				--ignore-errors --intent-to-add
> +				"
> +			return
> +		esac
> +	fi
> +	__gitcomp "$(__git_files_having_status "(.[MAU]|UD|\?\?)" -uall)"

Please do this and the others the other way around, as you did in
_git_checkout(), i.e.

	if __git_has_doubledash ; then
		__gitcomp "$(__git_files_having_status ...)"
		return
	fi

	case "$cur" in
	[...]

This will make the patch shorter and easier to review, it won't
increase the indentation unnecesarily, and it will be friendlier to
the users running 'git blame' in the future.

>  }
> 
>  _git_archive ()
> @@ -1171,7 +1181,12 @@ _git_bundle ()
> 
>  _git_checkout ()
>  {
> -	__git_has_doubledash && return
> +	if __git_has_doubledash; then
> +		if [[ ${words[2]} = "--" ]]; then
> +			__gitcomp "$(__git_files_having_status ".[MD]" -uno)"
> +		fi
> +		return
> +	fi
> 
>  	case "$cur" in
>  	--conflict=*)
> @@ -1469,7 +1484,7 @@ _git_help ()
>  	__gitcomp "$__git_all_commands $(__git_aliases)
>  		attributes cli core-tutorial cvs-migration
>  		diffcore gitk glossary hooks ignore modules
> -		namespaces repository-layout tutorial tutorial-2
> +		repository-layout tutorial tutorial-2

That's an independent change; I don't think you want to remove
namespaces here.

>  		workflows
>  		"
>  }
> @@ -2313,14 +2328,18 @@ _git_replace ()
> 
>  _git_reset ()
>  {
> -	__git_has_doubledash && return
> -
> -	case "$cur" in
> -	--*)
> -		__gitcomp "--merge --mixed --hard --soft --patch"
> +	if ! __git_has_doubledash; then
> +		case "$cur" in
> +		--*)
> +			__gitcomp "--merge --mixed --hard --soft --patch"
> +			return
> +			;;
> +		esac
> +	fi
> +	if [[ ${words[2]} = "HEAD" ]]; then
> +		__gitcomp "$(__git_files_having_status "[ADM]." -uno)"
>  		return
> -		;;
> -	esac
> +	fi
>  	__gitcomp "$(__git_refs)"
>  }
> 
> @@ -2337,15 +2356,20 @@ _git_revert ()
> 
>  _git_rm ()
>  {
> -	__git_has_doubledash && return
> -
> -	case "$cur" in
> -	--*)
> -		__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
> -		return
> -		;;
> -	esac
> -	COMPREPLY=()
> +	if ! __git_has_doubledash; then
> +		case "$cur" in
> +		--*)
> +			__gitcomp "--cached --dry-run --ignore-unmatch --quiet"
> +			return
> +			;;
> +		esac
> +	fi
> +	# check if --cached was specified
> +	if [ "$(__git_find_on_cmdline "--cached")" ]; then
> +		COMPREPLY=()
> +	else
> +		__gitcomp "$(__git_files_having_status "(.D|DU|UA)" -uno)"

'git rm' can be used to delete any tracked files, but this limits
completion to only those files that are already deleted from the work
tree or are unmerged.

> +	fi
>  }
> 
>  _git_shortlog ()
> @@ -2640,7 +2664,6 @@ _git ()
>  			--exec-path
>  			--html-path
>  			--work-tree=
> -			--namespace=

Independent change.

>  			--help
>  			"
>  			;;
> @@ -2737,3 +2760,4 @@ else
>  		shopt "$@"
>  	}
>  fi
> +

Superfluous new line.



Best,
Gábor