Re: [PATCH 2/2] completion: simplify _git_notes

[Junio C Hamano]

SZEDER Gábor  writes:

> That works fine, but this would work just as well and has one less
> case arm:

OK, I missed that "obvious optimization" opportunity.  I agree that
would work.

>
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index ab80f4e6e8..038af63c1a 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1839,6 +1839,8 @@ _git_notes ()
>   *,--*)
>   __gitcomp_builtin notes_$subcommand
>   ;;
> + prune,*)
> + ;;
>   *)
>   case "$prev" in
>   -m|-F)

Re: [PATCH 2/2] completion: simplify _git_notes

[SZEDER Gábor]

> SZEDER Gábor  writes:
> 
> > There is a minor behaviour change here, though.  This
> >
> >   prune,*)
> > ;;
> >
> > case arm ensured that we don't list refs for 'git notes prune ',
> > because it doesn't accept them (and then we take our usual fallback and
> > let Bash complete filenames;  yeah, 'git notes prune' doesn't accept
> > filenames either, but, as I said, that's our usual fallback when we
> > can't offer anything for completion).
> >
> > This patch removes that case arm, and refs will be offered for 'git
> > notes prune '.
> >
> >> +   *,--*)
> >> +   __gitcomp_builtin notes_$subcommand
> >> ;;
> >> *)
> >> case "$prev" in
> 
> I have this tentatively queued on the topic.  Can we wrap the topic
> up and move it forward, instead of leaving it (and other topics)
> hanging around and causing conflicts with other topics in flight,
> please?
> 
> Thanks.
> 
> 
> Subject: [PATCH] SQUASH???
> 
> By Szeder 

Re: [PATCH 2/2] completion: simplify _git_notes

[Junio C Hamano]

SZEDER Gábor  writes:

> There is a minor behaviour change here, though.  This
>
>   prune,*)
> ;;
>
> case arm ensured that we don't list refs for 'git notes prune ',
> because it doesn't accept them (and then we take our usual fallback and
> let Bash complete filenames;  yeah, 'git notes prune' doesn't accept
> filenames either, but, as I said, that's our usual fallback when we
> can't offer anything for completion).
>
> This patch removes that case arm, and refs will be offered for 'git
> notes prune '.
>
>> +   *,--*)
>> +   __gitcomp_builtin notes_$subcommand
>> ;;
>> *)
>> case "$prev" in

I have this tentatively queued on the topic.  Can we wrap the topic
up and move it forward, instead of leaving it (and other topics)
hanging around and causing conflicts with other topics in flight,
please?

Thanks.


Subject: [PATCH] SQUASH???

By Szeder 

Re: [PATCH 2/2] completion: simplify _git_notes

[SZEDER Gábor]

On Sat, Mar 3, 2018 at 10:23 AM, Nguyễn Thái Ngọc Duy  wrote:
> This also adds completion for 'git notes remove' with two options:
> --ignore-missing and --stdin.
>
> For some strange reason, 'git notes undefined --' completes --ref
> without even running --git-completion-helper. But since this is an error
> case (and we're not doing anything destructive, it's probably ok for now)
>
> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  contrib/completion/git-completion.bash | 15 ++-
>  1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index c310b241d3..ab80f4e6e8 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1836,19 +1836,8 @@ _git_notes ()
> add,--reedit-message=*|append,--reedit-message=*)
> __git_complete_refs --cur="${cur#*=}"
> ;;
> -   add,--*)
> -   __gitcomp_builtin notes_add
> -   ;;
> -   append,--*)
> -   __gitcomp_builtin notes_append
> -   ;;
> -   copy,--*)
> -   __gitcomp_builtin notes_copy
> -   ;;
> -   prune,--*)
> -   __gitcomp_builtin notes_prune
> -   ;;
> -   prune,*)

There is a minor behaviour change here, though.  This

  prune,*)
;;

case arm ensured that we don't list refs for 'git notes prune ',
because it doesn't accept them (and then we take our usual fallback and
let Bash complete filenames;  yeah, 'git notes prune' doesn't accept
filenames either, but, as I said, that's our usual fallback when we
can't offer anything for completion).

This patch removes that case arm, and refs will be offered for 'git
notes prune '.

> +   *,--*)
> +   __gitcomp_builtin notes_$subcommand
> ;;
> *)
> case "$prev" in
> --
> 2.16.1.435.g8f24da2e1a
>

Re: [PATCH 2/2] completion: simplify _git_notes

[SZEDER Gábor]

On Sat, Mar 3, 2018 at 10:23 AM, Nguyễn Thái Ngọc Duy  wrote:
> This also adds completion for 'git notes remove' with two options:
> --ignore-missing and --stdin.
>
> For some strange reason, 'git notes undefined --' completes --ref
> without even running --git-completion-helper.

There is nothing strange about it.  _git_notes() first looks for the
presence of any subcommands on the command line, and if it doesn't find
any, it will list 'git notes's subcommands and options for completion.
And it does so by running '__gitcomp "$subcommands --ref"'

> But since this is an error
> case (and we're not doing anything destructive, it's probably ok for now)

I agree; and it matches the behaviour before the patch.

> Signed-off-by: Nguyễn Thái Ngọc Duy 
> ---
>  contrib/completion/git-completion.bash | 15 ++-
>  1 file changed, 2 insertions(+), 13 deletions(-)
>
> diff --git a/contrib/completion/git-completion.bash 
> b/contrib/completion/git-completion.bash
> index c310b241d3..ab80f4e6e8 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -1836,19 +1836,8 @@ _git_notes ()
> add,--reedit-message=*|append,--reedit-message=*)
> __git_complete_refs --cur="${cur#*=}"
> ;;
> -   add,--*)
> -   __gitcomp_builtin notes_add
> -   ;;
> -   append,--*)
> -   __gitcomp_builtin notes_append
> -   ;;
> -   copy,--*)
> -   __gitcomp_builtin notes_copy
> -   ;;
> -   prune,--*)
> -   __gitcomp_builtin notes_prune
> -   ;;
> -   prune,*)
> +   *,--*)
> +   __gitcomp_builtin notes_$subcommand
> ;;
> *)
> case "$prev" in
> --
> 2.16.1.435.g8f24da2e1a
>

[PATCH 2/2] completion: simplify _git_notes

[Nguyễn Thái Ngọc Duy]

This also adds completion for 'git notes remove' with two options:
--ignore-missing and --stdin.

For some strange reason, 'git notes undefined --' completes --ref
without even running --git-completion-helper. But since this is an error
case (and we're not doing anything destructive, it's probably ok for now)

Signed-off-by: Nguyễn Thái Ngọc Duy 
---
 contrib/completion/git-completion.bash | 15 ++-
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/contrib/completion/git-completion.bash 
b/contrib/completion/git-completion.bash
index c310b241d3..ab80f4e6e8 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1836,19 +1836,8 @@ _git_notes ()
add,--reedit-message=*|append,--reedit-message=*)
__git_complete_refs --cur="${cur#*=}"
;;
-   add,--*)
-   __gitcomp_builtin notes_add
-   ;;
-   append,--*)
-   __gitcomp_builtin notes_append
-   ;;
-   copy,--*)
-   __gitcomp_builtin notes_copy
-   ;;
-   prune,--*)
-   __gitcomp_builtin notes_prune
-   ;;
-   prune,*)
+   *,--*)
+   __gitcomp_builtin notes_$subcommand
;;
*)
case "$prev" in
-- 
2.16.1.435.g8f24da2e1a