threads / patch / 54316

patchgit-completion.bash: stash-show: add --patch-with-stat

Subject: [PATCH] git-completion.bash: stash-show: add --patch-with-stat

## tl;dr

6 messages between Sep 28, 2020 and Sep 30, 2020. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Robert Karszniewicz· Sep 28, 2020, 11:05 UTC · lore

Signed-off-by: Robert Karszniewicz <avoidr@posteo.de>
---
 contrib/completion/git-completion.bash | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Show changes to contrib/completion/git-completion.bash +4 −1

diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 8be4a0316e..d98c731667 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -3016,7 +3016,10 @@ _git_stash ()
 		list,--*)
 			__gitcomp "--name-status --oneline --patch-with-stat"
 			;;
-		show,--*|branch,--*)
+		show,--*)
+			__gitcomp "--patch-with-stat"
+			;;
+		branch,--*)
 			;;
 		branch,*)
 			if [ $cword -eq 3 ]; then

-- 
2.28.0

Junio C Hamano· Sep 28, 2020, 18:43 UTC · re: Robert Karszniewicz · lore

Re: [PATCH] git-completion.bash: stash-show: add --patch-with-stat

Robert Karszniewicz <avoidr@posteo.de> writes:

Show 17 quoted lines

> Signed-off-by: Robert Karszniewicz <avoidr@posteo.de>
> ---
>  contrib/completion/git-completion.bash | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> index 8be4a0316e..d98c731667 100644
> --- a/contrib/completion/git-completion.bash
> +++ b/contrib/completion/git-completion.bash
> @@ -3016,7 +3016,10 @@ _git_stash ()
>  		list,--*)
>  			__gitcomp "--name-status --oneline --patch-with-stat"
>  			;;
> -		show,--*|branch,--*)
> +		show,--*)
> +			__gitcomp "--patch-with-stat"
> +			;;

Why is --patch-with-stat so special?

Without completion support for "--patch" and "--stat", typing "--<TAB>" after "git stash show" and seeing only "--patch-with-stat" (which has been made obsolete-but-still-kept synonym immediately after the other two were invented in 2005) would make a rather surprising experience to the end users. For "show" alone, it may make a lot of sense to complete "git stash show -<TAB>" and offer "-p".

In any case, it might make more sense to do this instead, and then rethink what options make sense to these subcommands of "git stash". I do not think patch-with-stat should be among them.

-  		list,--*)
+  		list,--* | show,--*)
  			__gitcomp "--name-status --oneline --patch-with-stat"

Thanks.

Denton Liu· Sep 29, 2020, 06:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-completion.bash: stash-show: add --patch-with-stat

Hi Junio,

On Mon, Sep 28, 2020 at 11:43:11AM -0700, Junio C Hamano wrote:

> In any case, it might make more sense to do this instead, and then
> rethink what options make sense to these subcommands of "git stash".
> I do not think patch-with-stat should be among them.

Perhaps it would make sense to add --patch and --no-patch to $__git_diff_common_options and then use that list for `git stash show` since it's documented that all of the diff options are valid.

Thanks, Denton

Robert Karszniewicz· Sep 29, 2020, 21:31 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-completion.bash: stash-show: add --patch-with-stat

On Mon, Sep 28, 2020 at 11:43:11AM -0700, Junio C Hamano wrote:

Show 26 quoted lines

> Robert Karszniewicz <avoidr@posteo.de> writes:
> 
> > Signed-off-by: Robert Karszniewicz <avoidr@posteo.de>
> > ---
> >  contrib/completion/git-completion.bash | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
> > index 8be4a0316e..d98c731667 100644
> > --- a/contrib/completion/git-completion.bash
> > +++ b/contrib/completion/git-completion.bash
> > @@ -3016,7 +3016,10 @@ _git_stash ()
> >  		list,--*)
> >  			__gitcomp "--name-status --oneline --patch-with-stat"
> >  			;;
> > -		show,--*|branch,--*)
> > +		show,--*)
> > +			__gitcomp "--patch-with-stat"
> > +			;;
> 
> Why is --patch-with-stat so special?  
> 
> Without completion support for "--patch" and "--stat", typing
> "--<TAB>" after "git stash show" and seeing only "--patch-with-stat"
> (which has been made obsolete-but-still-kept synonym immediately
> after the other two were invented in 2005) would make a rather

Oh, I didn't know --patch-with-stat was obsoleted. It was recently added to `stash-list`, too. I can as well use "--patch --stat".

> surprising experience to the end users.  For "show" alone, it may
> make a lot of sense to complete "git stash show -<TAB>" and offer
> "-p".

Does git complete short options at all? I only see long options completed. (I'm also very new to bash-completion)

> 
> In any case, it might make more sense to do this instead, and then
> rethink what options make sense to these subcommands of "git stash".
> I do not think patch-with-stat should be among them.

So shall I do a v2 as per your suggestion and replace "--patch-with-stat" with "--patch --stat"?

Show 6 quoted lines

> 
> -  		list,--*)
> +  		list,--* | show,--*)
>   			__gitcomp "--name-status --oneline --patch-with-stat"
> 
> Thanks.

Thank you.

Junio C Hamano· Sep 30, 2020, 19:09 UTC · re: Robert Karszniewicz · lore

Re: [PATCH] git-completion.bash: stash-show: add --patch-with-stat

Robert Karszniewicz <avoidr@posteo.de> writes:

Show 6 quoted lines

>> surprising experience to the end users.  For "show" alone, it may
>> make a lot of sense to complete "git stash show -<TAB>" and offer
>> "-p".
>
> Does git complete short options at all? I only see long options
> completed. (I'm also very new to bash-completion)

I wouldn't personally recommend it, but I did see a patch that added support for one short option completion quite recently. As long as "--<TAB>" gets completed to often-used options, among which "--patch" and "--stat" are included, it would be OK not to react to "-<TAB>".

Show 6 quoted lines

>> In any case, it might make more sense to do this instead, and then
>> rethink what options make sense to these subcommands of "git stash".
>> I do not think patch-with-stat should be among them.
>
> So shall I do a v2 as per your suggestion and replace
> "--patch-with-stat" with "--patch --stat"?

I think Denton Liu offered a different suggestion; I didn't look at and compare which direction is the better one myself, but an approach that keeps the number of manually-maintained list of options low is almost always a good approach.

Thanks.

Robert Karszniewicz· Sep 30, 2020, 21:56 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-completion.bash: stash-show: add --patch-with-stat

On Wed, Sep 30, 2020 at 12:09:09PM -0700, Junio C Hamano wrote:

Show 8 quoted lines

> Robert Karszniewicz <avoidr@posteo.de> writes:
> > So shall I do a v2 as per your suggestion and replace
> > "--patch-with-stat" with "--patch --stat"?
> 
> I think Denton Liu offered a different suggestion; I didn't look at
> and compare which direction is the better one myself, but an
> approach that keeps the number of manually-maintained list of
> options low is almost always a good approach.

Ok, I understand, then. Will take a look at it.

Thank you.