threads / patch / 17085

patchbash completion: refactor diff options

Subject: [PATCH] bash completion: refactor diff options

## tl;dr

8 messages between Jan 11, 2009 and Jan 20, 2009. Diffs are folded; open one to read it.

replies: 7people: 3as markdown or json

Thomas Rast· Jan 11, 2009, 13:14 UTC · lore

diff, log and show all take the same diff options. Refactor them from __git_diff and __git_log into a variable, and complete them in __git_show too.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---

This conflicts with the --patience series; I can send a version based on that if it goes in first.

 contrib/completion/git-completion.bash |   38 ++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 16 deletions(-)
Show changes to contrib/completion/git-completion.bash +22 −16
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 7b074d7..5017369 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -759,24 +759,29 @@ _git_describe ()
 	__gitcomp "$(__git_refs)"
 }
 
-_git_diff ()
-{
-	__git_has_doubledash && return
-
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-	case "$cur" in
-	--*)
-		__gitcomp "--cached --stat --numstat --shortstat --summary
+__git_diff_common_options="--stat --numstat --shortstat --summary
 			--patch-with-stat --name-only --name-status --color
 			--no-color --color-words --no-renames --check
 			--full-index --binary --abbrev --diff-filter=
-			--find-copies-harder --pickaxe-all --pickaxe-regex
+			--find-copies-harder
 			--text --ignore-space-at-eol --ignore-space-change
 			--ignore-all-space --exit-code --quiet --ext-diff
 			--no-ext-diff
 			--no-prefix --src-prefix= --dst-prefix=
-			--base --ours --theirs
 			--inter-hunk-context=
+			--raw
+"
+
+_git_diff ()
+{
+	__git_has_doubledash && return
+
+	local cur="${COMP_WORDS[COMP_CWORD]}"
+	case "$cur" in
+	--*)
+		__gitcomp "--cached --pickaxe-all --pickaxe-regex
+			--base --ours --theirs
+			$__git_diff_common_options
 			"
 		return
 		;;
@@ -959,16 +964,15 @@ _git_log ()
 			--relative-date --date=
 			--author= --committer= --grep=
 			--all-match
-			--pretty= --name-status --name-only --raw
+			--pretty=
 			--not --all
 			--left-right --cherry-pick
 			--graph
-			--stat --numstat --shortstat
-			--decorate --diff-filter=
-			--color-words --walk-reflogs
+			--decorate
+			--walk-reflogs
 			--parents --children --full-history
 			--merge
-			--inter-hunk-context=
+			$__git_diff_common_options
 			"
 		return
 		;;
@@ -1473,7 +1477,9 @@ _git_show ()
 		return
 		;;
 	--*)
-		__gitcomp "--pretty="
+		__gitcomp "--pretty=
+			$__git_diff_common_options
+			"
 		return
 		;;
 	esac

-- 
tg: (7eb5bbd..) t/bash-complete-show (depends on: origin/master)
Thomas Rast· Jan 12, 2009, 09:16 UTC · re: Thomas Rast · lore

[PATCH next] bash completion: refactor diff options

diff, log and show all take the same diff options. Refactor them from __git_diff and __git_log into a variable, and complete them in __git_show too.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
js/patience-diff is now in next, so here's a version on top of that.
 contrib/completion/git-completion.bash |   38 ++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 16 deletions(-)
Show changes to contrib/completion/git-completion.bash +22 −16
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index dee75b5..39ae7a3 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -759,25 +759,30 @@ _git_describe ()
 	__gitcomp "$(__git_refs)"
 }
 
-_git_diff ()
-{
-	__git_has_doubledash && return
-
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-	case "$cur" in
-	--*)
-		__gitcomp "--cached --stat --numstat --shortstat --summary
+__git_diff_common_options="--stat --numstat --shortstat --summary
 			--patch-with-stat --name-only --name-status --color
 			--no-color --color-words --no-renames --check
 			--full-index --binary --abbrev --diff-filter=
-			--find-copies-harder --pickaxe-all --pickaxe-regex
+			--find-copies-harder
 			--text --ignore-space-at-eol --ignore-space-change
 			--ignore-all-space --exit-code --quiet --ext-diff
 			--no-ext-diff
 			--no-prefix --src-prefix= --dst-prefix=
-			--base --ours --theirs
 			--inter-hunk-context=
 			--patience
+			--raw
+"
+
+_git_diff ()
+{
+	__git_has_doubledash && return
+
+	local cur="${COMP_WORDS[COMP_CWORD]}"
+	case "$cur" in
+	--*)
+		__gitcomp "--cached --pickaxe-all --pickaxe-regex
+			--base --ours --theirs
+			$__git_diff_common_options
 			"
 		return
 		;;
@@ -960,17 +965,16 @@ _git_log ()
 			--relative-date --date=
 			--author= --committer= --grep=
 			--all-match
-			--pretty= --name-status --name-only --raw
+			--pretty=
 			--not --all
 			--left-right --cherry-pick
 			--graph
-			--stat --numstat --shortstat
-			--decorate --diff-filter=
-			--color-words --walk-reflogs
+			--decorate
+			--walk-reflogs
 			--parents --children --full-history
 			--merge
 			--inter-hunk-context=
-			--patience
+			$__git_diff_common_options
 			"
 		return
 		;;
@@ -1475,7 +1479,9 @@ _git_show ()
 		return
 		;;
 	--*)
-		__gitcomp "--pretty="
+		__gitcomp "--pretty=
+			$__git_diff_common_options
+			"
 		return
 		;;
 	esac

-- 
tg: (f5cbdbb..) t/bash-complete-show (depends on: origin/next)
Thomas Rast· Jan 18, 2009, 01:03 UTC · re: Thomas Rast · lore

[PATCH next resend] bash completion: refactor diff options

diff, log and show all take the same diff options. Refactor them from __git_diff and __git_log into a variable, and complete them in __git_show too.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Any news on this?
 contrib/completion/git-completion.bash |   38 ++++++++++++++++++-------------
 1 files changed, 22 insertions(+), 16 deletions(-)
Show changes to contrib/completion/git-completion.bash +22 −16
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 70a6b44..40e3a14 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -774,25 +774,30 @@ _git_describe ()
 	__gitcomp "$(__git_refs)"
 }
 
-_git_diff ()
-{
-	__git_has_doubledash && return
-
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-	case "$cur" in
-	--*)
-		__gitcomp "--cached --stat --numstat --shortstat --summary
+__git_diff_common_options="--stat --numstat --shortstat --summary
 			--patch-with-stat --name-only --name-status --color
 			--no-color --color-words --no-renames --check
 			--full-index --binary --abbrev --diff-filter=
-			--find-copies-harder --pickaxe-all --pickaxe-regex
+			--find-copies-harder
 			--text --ignore-space-at-eol --ignore-space-change
 			--ignore-all-space --exit-code --quiet --ext-diff
 			--no-ext-diff
 			--no-prefix --src-prefix= --dst-prefix=
-			--base --ours --theirs
 			--inter-hunk-context=
 			--patience
+			--raw
+"
+
+_git_diff ()
+{
+	__git_has_doubledash && return
+
+	local cur="${COMP_WORDS[COMP_CWORD]}"
+	case "$cur" in
+	--*)
+		__gitcomp "--cached --pickaxe-all --pickaxe-regex
+			--base --ours --theirs
+			$__git_diff_common_options
 			"
 		return
 		;;
@@ -975,17 +980,16 @@ _git_log ()
 			--relative-date --date=
 			--author= --committer= --grep=
 			--all-match
-			--pretty= --name-status --name-only --raw
+			--pretty=
 			--not --all
 			--left-right --cherry-pick
 			--graph
-			--stat --numstat --shortstat
-			--decorate --diff-filter=
-			--color-words --walk-reflogs
+			--decorate
+			--walk-reflogs
 			--parents --children --full-history
 			--merge
 			--inter-hunk-context=
-			--patience
+			$__git_diff_common_options
 			"
 		return
 		;;
@@ -1490,7 +1494,9 @@ _git_show ()
 		return
 		;;
 	--*)
-		__gitcomp "--pretty="
+		__gitcomp "--pretty=
+			$__git_diff_common_options
+			"
 		return
 		;;
 	esac

-- 
tg: (7ff1443..) t/bash-complete-show (depends on: origin/next)
Junio C Hamano· Jan 18, 2009, 07:18 UTC · re: Thomas Rast · lore

Re: [PATCH next resend] bash completion: refactor diff options

Thomas Rast <trast@student.ethz.ch> writes:
Show 9 quoted lines
> diff, log and show all take the same diff options.  Refactor them from
> __git_diff and __git_log into a variable, and complete them in
> __git_show too.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
>
> ---
>
> Any news on this?
Show 6 quoted lines
> +__git_diff_common_options="--stat --numstat --shortstat --summary
>  			--patch-with-stat --name-only --name-status --color
>  			--no-color --color-words --no-renames --check
>  			--full-index --binary --abbrev --diff-filter=
> -			--find-copies-harder --pickaxe-all --pickaxe-regex
> +			--find-copies-harder

The changes around pickaxe made me "Huh?". For log pickaxe makes very good sense but for a single diff it doesn't, yet the original seems to have had them only on "git diff" and not on "git log", which feels wrong.

Other than that, I think the patch tries to achieve a great thing in the longer term---we do not have to worry about common parts going out of sync between diff and log family of commands.

Shawn O. Pearce· Jan 19, 2009, 17:31 UTC · re: Junio C Hamano · lore

Re: [PATCH next resend] bash completion: refactor diff options

Junio C Hamano <gitster@pobox.com> wrote:
Show 16 quoted lines
> Thomas Rast <trast@student.ethz.ch> writes:
> 
> > +__git_diff_common_options="--stat --numstat --shortstat --summary
> >  			--patch-with-stat --name-only --name-status --color
> >  			--no-color --color-words --no-renames --check
> >  			--full-index --binary --abbrev --diff-filter=
> > -			--find-copies-harder --pickaxe-all --pickaxe-regex
> > +			--find-copies-harder
> 
> The changes around pickaxe made me "Huh?".  For log pickaxe makes very
> good sense but for a single diff it doesn't, yet the original seems to
> have had them only on "git diff" and not on "git log", which feels wrong.
> 
> Other than that, I think the patch tries to achieve a great thing in the
> longer term---we do not have to worry about common parts going out of sync
> between diff and log family of commands.

I agree completely. The pickaxe stuff in current completion is just plain wrong. I'd like to see it fixed with this cleanup. Maybe do it in two parts; fix the pickaxe options to be on log and not diff, and then do the cleanup for the common options.

-- 
Shawn.
Thomas Rast· Jan 19, 2009, 21:17 UTC · re: Shawn O. Pearce · lore

[PATCH v2 1/2] bash completion: move pickaxe options to log

Move the options --pickaxe-all and --pickaxe-regex to git-log, where they make more sense than with git-diff.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
Shawn O. Pearce wrote:
Show 9 quoted lines
> Junio C Hamano <gitster@pobox.com> wrote:
> > The changes around pickaxe made me "Huh?".  For log pickaxe makes very
> > good sense but for a single diff it doesn't, yet the original seems to
> > have had them only on "git diff" and not on "git log", which feels wrong.
> 
> I agree completely.  The pickaxe stuff in current completion is
> just plain wrong.  I'd like to see it fixed with this cleanup.
> Maybe do it in two parts; fix the pickaxe options to be on log
> and not diff, and then do the cleanup for the common options.

Good point. Sorry for not noticing in the first place; I was too focused on just getting git-show completion working ;-)

 contrib/completion/git-completion.bash |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
Show changes to contrib/completion/git-completion.bash +2 −1
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 60497a4..b5d3bbb 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -784,7 +784,7 @@ _git_diff ()
 			--patch-with-stat --name-only --name-status --color
 			--no-color --color-words --no-renames --check
 			--full-index --binary --abbrev --diff-filter=
-			--find-copies-harder --pickaxe-all --pickaxe-regex
+			--find-copies-harder
 			--text --ignore-space-at-eol --ignore-space-change
 			--ignore-all-space --exit-code --quiet --ext-diff
 			--no-ext-diff
@@ -986,6 +986,7 @@ _git_log ()
 			--parents --children --full-history
 			--merge
 			--inter-hunk-context=
+			--pickaxe-all --pickaxe-regex
 			"
 		return
 		;;

-- 
tg: (28da86a..) t/bash-complete-pickaxe (depends on: origin/master)
Thomas Rast· Jan 19, 2009, 21:18 UTC · re: Thomas Rast · lore

[PATCH v2 2/2] bash completion: refactor diff options

diff, log and show all take the same diff options. Refactor them from __git_diff and __git_log into a variable, and complete them in __git_show too.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 contrib/completion/git-completion.bash |   36 ++++++++++++++++++-------------
 1 files changed, 21 insertions(+), 15 deletions(-)
Show changes to contrib/completion/git-completion.bash +21 −15
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 096603b..bfae953 100755
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -773,14 +773,7 @@ _git_describe ()
 	__gitcomp "$(__git_refs)"
 }
 
-_git_diff ()
-{
-	__git_has_doubledash && return
-
-	local cur="${COMP_WORDS[COMP_CWORD]}"
-	case "$cur" in
-	--*)
-		__gitcomp "--cached --stat --numstat --shortstat --summary
+__git_diff_common_options="--stat --numstat --shortstat --summary
 			--patch-with-stat --name-only --name-status --color
 			--no-color --color-words --no-renames --check
 			--full-index --binary --abbrev --diff-filter=
@@ -789,9 +782,21 @@ _git_diff ()
 			--ignore-all-space --exit-code --quiet --ext-diff
 			--no-ext-diff
 			--no-prefix --src-prefix= --dst-prefix=
-			--base --ours --theirs
 			--inter-hunk-context=
 			--patience
+			--raw
+"
+
+_git_diff ()
+{
+	__git_has_doubledash && return
+
+	local cur="${COMP_WORDS[COMP_CWORD]}"
+	case "$cur" in
+	--*)
+		__gitcomp "--cached --pickaxe-all --pickaxe-regex
+			--base --ours --theirs
+			$__git_diff_common_options
 			"
 		return
 		;;
@@ -977,17 +982,16 @@ _git_log ()
 			--relative-date --date=
 			--author= --committer= --grep=
 			--all-match
-			--pretty= --name-status --name-only --raw
+			--pretty=
 			--not --all
 			--left-right --cherry-pick
 			--graph
-			--stat --numstat --shortstat
-			--decorate --diff-filter=
-			--color-words --walk-reflogs
+			--decorate
+			--walk-reflogs
 			--parents --children --full-history
 			--merge
 			--inter-hunk-context=
-			--patience
+			$__git_diff_common_options
 			--pickaxe-all --pickaxe-regex
 			"
 		return
@@ -1492,7 +1496,9 @@ _git_show ()
 		return
 		;;
 	--*)
-		__gitcomp "--pretty="
+		__gitcomp "--pretty=
+			$__git_diff_common_options
+			"
 		return
 		;;
 	esac

-- 
tg: (3f8c856..) t/bash-complete-show (depends on: origin/next t/bash-complete-pickaxe t/bash-complete-pickaxe)
Junio C Hamano· Jan 20, 2009, 08:36 UTC · re: Thomas Rast · lore

Re: [PATCH v2 2/2] bash completion: refactor diff options

Thomas Rast <trast@student.ethz.ch> writes:
Show 74 quoted lines
> diff, log and show all take the same diff options.  Refactor them from
> __git_diff and __git_log into a variable, and complete them in
> __git_show too.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
>
> ---
>  contrib/completion/git-completion.bash |   36 ++++++++++++++++++-------------
>  1 files changed, 21 insertions(+), 15 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 096603b..bfae953 100755
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -773,14 +773,7 @@ _git_describe ()
>  	__gitcomp "$(__git_refs)"
>  }
>  
> -_git_diff ()
> -{
> -	__git_has_doubledash && return
> -
> -	local cur="${COMP_WORDS[COMP_CWORD]}"
> -	case "$cur" in
> -	--*)
> -		__gitcomp "--cached --stat --numstat --shortstat --summary
> +__git_diff_common_options="--stat --numstat --shortstat --summary
>  			--patch-with-stat --name-only --name-status --color
>  			--no-color --color-words --no-renames --check
>  			--full-index --binary --abbrev --diff-filter=
> @@ -789,9 +782,21 @@ _git_diff ()
>  			--ignore-all-space --exit-code --quiet --ext-diff
>  			--no-ext-diff
>  			--no-prefix --src-prefix= --dst-prefix=
> -			--base --ours --theirs
>  			--inter-hunk-context=
>  			--patience
> +			--raw
> +"
> +
> +_git_diff ()
> +{
> +	__git_has_doubledash && return
> +
> +	local cur="${COMP_WORDS[COMP_CWORD]}"
> +	case "$cur" in
> +	--*)
> +		__gitcomp "--cached --pickaxe-all --pickaxe-regex
> +			--base --ours --theirs
> +			$__git_diff_common_options
>  			"
>  		return
>  		;;
> @@ -977,17 +982,16 @@ _git_log ()
>  			--relative-date --date=
>  			--author= --committer= --grep=
>  			--all-match
> -			--pretty= --name-status --name-only --raw
> +			--pretty=
>  			--not --all
>  			--left-right --cherry-pick
>  			--graph
> -			--stat --numstat --shortstat
> -			--decorate --diff-filter=
> -			--color-words --walk-reflogs
> +			--decorate
> +			--walk-reflogs
>  			--parents --children --full-history
>  			--merge
>  			--inter-hunk-context=
> -			--patience
> +			$__git_diff_common_options
>  			--pickaxe-all --pickaxe-regex
>  			"

I'll tweak this part to drop duplicated --ihc; other than that it looked good. Thanks.

← back to recent threads