Re: [PATCH v2 18/18] completion: support branch-diff

2018-05-06 Thread Johannes Schindelin
Hi Duy,

On Sun, 6 May 2018, Duy Nguyen wrote:

> On Fri, May 04, 2018 at 05:35:11PM +0200, Johannes Schindelin wrote:
> > Tab completion of `branch-diff` is very convenient, especially given
> > that the revision arguments that need to be passed to `git branch-diff`
> > are typically more complex than, say, your grandfather's `git log`
> > arguments.
> > 
> > Without this patch, we would only complete the `branch-diff` part but
> > not the options and other arguments.
> > 
> > This of itself may already be slightly disruptive for well-trained
> > fingers that assume that `git braorimas` would expand to
> > `git branch origin/master`, as we now no longer automatically append a
> > space after completing `git branch`: this is now ambiguous.
> > 
> > Signed-off-by: Johannes Schindelin 
> > ---
> >  contrib/completion/git-completion.bash | 18 ++
> >  1 file changed, 18 insertions(+)
> > 
> > diff --git a/contrib/completion/git-completion.bash 
> > b/contrib/completion/git-completion.bash
> > index 01dd9ff07a2..45addd525ac 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -1496,6 +1496,24 @@ _git_format_patch ()
> > __git_complete_revlist
> >  }
> >  
> > +__git_branch_diff_options="
> > +   --no-patches --creation-weight= --dual-color
> > +"
> > +
> > +_git_branch_diff ()
> > +{
> > +   case "$cur" in
> > +   --*)
> > +   __gitcomp "
> 
> You should use __gitcomp_builtin so you don't have to maintain
> $__git_branch_diff_options here. Something like this
> 
> -- 8< --
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index 45addd525a..4745631daf 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1496,18 +1496,11 @@ _git_format_patch ()
>   __git_complete_revlist
>  }
>  
> -__git_branch_diff_options="
> - --no-patches --creation-weight= --dual-color
> -"
> -
>  _git_branch_diff ()
>  {
>   case "$cur" in
>   --*)
> - __gitcomp "
> - $__git_branch_diff_options
> - $__git_diff_common_options
> - "
> + __gitcomp_builtin branch-diff "$__git_diff_common_options"
>   return
>   ;;
>   esac
> -- 8< --

Does this really work? I have this instead, for now, and verified that it
works:

-- snipsnap --
diff --git a/contrib/completion/git-completion.bash
b/contrib/completion/git-completion.bash
index 01dd9ff07a2..c498c053881 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1205,13 +1205,14 @@ _git_bisect ()

 _git_branch ()
 {
-   local i c=1 only_local_ref="n" has_r="n"
+   local i c=1 only_local_ref="n" has_r="n" diff_mode="n"

while [ $c -lt $cword ]; do
i="${words[c]}"
case "$i" in
-d|--delete|-m|--move)  only_local_ref="y" ;;
-r|--remotes)   has_r="y" ;;
+   --diff) diff_mode="y" ;;
esac
((c++))
done
@@ -1221,11 +1222,22 @@ _git_branch ()
__git_complete_refs --cur="${cur##--set-upstream-to=}"
;;
--*)
+   if [ $diff_mode = "y" ]; then
+   __gitcomp "
+   --creation-factor= --dual-color
+   $__git_diff_common_options
+   "
+   return
+   fi
__gitcomp_builtin branch "--no-color --no-abbrev
--no-track --no-column
"
;;
*)
+   if [ $diff_mode = "y" ]; then
+   __git_complete_revlist
+   return
+   fi
if [ $only_local_ref = "y" -a $has_r = "n" ]; then
__gitcomp_direct "$(__git_heads "" "$cur" " ")"
else



Re: [PATCH v2 18/18] completion: support branch-diff

2018-05-06 Thread Duy Nguyen
On Fri, May 04, 2018 at 05:35:11PM +0200, Johannes Schindelin wrote:
> Tab completion of `branch-diff` is very convenient, especially given
> that the revision arguments that need to be passed to `git branch-diff`
> are typically more complex than, say, your grandfather's `git log`
> arguments.
> 
> Without this patch, we would only complete the `branch-diff` part but
> not the options and other arguments.
> 
> This of itself may already be slightly disruptive for well-trained
> fingers that assume that `git braorimas` would expand to
> `git branch origin/master`, as we now no longer automatically append a
> space after completing `git branch`: this is now ambiguous.
> 
> Signed-off-by: Johannes Schindelin 
> ---
>  contrib/completion/git-completion.bash | 18 ++
>  1 file changed, 18 insertions(+)
> 
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index 01dd9ff07a2..45addd525ac 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1496,6 +1496,24 @@ _git_format_patch ()
>   __git_complete_revlist
>  }
>  
> +__git_branch_diff_options="
> + --no-patches --creation-weight= --dual-color
> +"
> +
> +_git_branch_diff ()
> +{
> + case "$cur" in
> + --*)
> + __gitcomp "

You should use __gitcomp_builtin so you don't have to maintain
$__git_branch_diff_options here. Something like this

-- 8< --
diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 45addd525a..4745631daf 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1496,18 +1496,11 @@ _git_format_patch ()
__git_complete_revlist
 }
 
-__git_branch_diff_options="
-   --no-patches --creation-weight= --dual-color
-"
-
 _git_branch_diff ()
 {
case "$cur" in
--*)
-   __gitcomp "
-   $__git_branch_diff_options
-   $__git_diff_common_options
-   "
+   __gitcomp_builtin branch-diff "$__git_diff_common_options"
return
;;
esac
-- 8< --


> + $__git_branch_diff_options
> + $__git_diff_common_options
> + "
> + return
> + ;;
> + esac
> + __git_complete_revlist
> +}
> +
>  _git_fsck ()
>  {
>   case "$cur" in
> -- 
> 2.17.0.409.g71698f11835


[PATCH v2 18/18] completion: support branch-diff

2018-05-04 Thread Johannes Schindelin
Tab completion of `branch-diff` is very convenient, especially given
that the revision arguments that need to be passed to `git branch-diff`
are typically more complex than, say, your grandfather's `git log`
arguments.

Without this patch, we would only complete the `branch-diff` part but
not the options and other arguments.

This of itself may already be slightly disruptive for well-trained
fingers that assume that `git braorimas` would expand to
`git branch origin/master`, as we now no longer automatically append a
space after completing `git branch`: this is now ambiguous.

Signed-off-by: Johannes Schindelin 
---
 contrib/completion/git-completion.bash | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index 01dd9ff07a2..45addd525ac 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1496,6 +1496,24 @@ _git_format_patch ()
__git_complete_revlist
 }
 
+__git_branch_diff_options="
+   --no-patches --creation-weight= --dual-color
+"
+
+_git_branch_diff ()
+{
+   case "$cur" in
+   --*)
+   __gitcomp "
+   $__git_branch_diff_options
+   $__git_diff_common_options
+   "
+   return
+   ;;
+   esac
+   __git_complete_revlist
+}
+
 _git_fsck ()
 {
case "$cur" in
-- 
2.17.0.409.g71698f11835