git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* [PATCH] git-completion.bash: stash-show: add --patch-with-stat
@ 2020-09-28 11:05 Robert Karszniewicz
  2020-09-28 18:43 ` Junio C Hamano
  0 siblings, 1 reply; 6+ messages in thread
From: Robert Karszniewicz @ 2020-09-28 11:05 UTC (permalink / raw)
  To: git

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"
+			;;
+		branch,--*)
 			;;
 		branch,*)
 			if [ $cword -eq 3 ]; then
-- 
2.28.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-completion.bash: stash-show: add --patch-with-stat
  2020-09-28 11:05 [PATCH] git-completion.bash: stash-show: add --patch-with-stat Robert Karszniewicz
@ 2020-09-28 18:43 ` Junio C Hamano
  2020-09-29  6:25   ` Denton Liu
  2020-09-29 21:31   ` Robert Karszniewicz
  0 siblings, 2 replies; 6+ messages in thread
From: Junio C Hamano @ 2020-09-28 18:43 UTC (permalink / raw)
  To: Robert Karszniewicz; +Cc: git

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
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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-completion.bash: stash-show: add --patch-with-stat
  2020-09-28 18:43 ` Junio C Hamano
@ 2020-09-29  6:25   ` Denton Liu
  2020-09-29 21:31   ` Robert Karszniewicz
  1 sibling, 0 replies; 6+ messages in thread
From: Denton Liu @ 2020-09-29  6:25 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Robert Karszniewicz, git

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-completion.bash: stash-show: add --patch-with-stat
  2020-09-28 18:43 ` Junio C Hamano
  2020-09-29  6:25   ` Denton Liu
@ 2020-09-29 21:31   ` Robert Karszniewicz
  2020-09-30 19:09     ` Junio C Hamano
  1 sibling, 1 reply; 6+ messages in thread
From: Robert Karszniewicz @ 2020-09-29 21:31 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Mon, Sep 28, 2020 at 11:43:11AM -0700, Junio C Hamano wrote:
> 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"?

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

Thank you.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-completion.bash: stash-show: add --patch-with-stat
  2020-09-29 21:31   ` Robert Karszniewicz
@ 2020-09-30 19:09     ` Junio C Hamano
  2020-09-30 21:56       ` Robert Karszniewicz
  0 siblings, 1 reply; 6+ messages in thread
From: Junio C Hamano @ 2020-09-30 19:09 UTC (permalink / raw)
  To: Robert Karszniewicz; +Cc: git

Robert Karszniewicz <avoidr@posteo.de> writes:

>> 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>".

>> 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] git-completion.bash: stash-show: add --patch-with-stat
  2020-09-30 19:09     ` Junio C Hamano
@ 2020-09-30 21:56       ` Robert Karszniewicz
  0 siblings, 0 replies; 6+ messages in thread
From: Robert Karszniewicz @ 2020-09-30 21:56 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Wed, Sep 30, 2020 at 12:09:09PM -0700, Junio C Hamano wrote:
> 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-09-30 21:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-28 11:05 [PATCH] git-completion.bash: stash-show: add --patch-with-stat Robert Karszniewicz
2020-09-28 18:43 ` Junio C Hamano
2020-09-29  6:25   ` Denton Liu
2020-09-29 21:31   ` Robert Karszniewicz
2020-09-30 19:09     ` Junio C Hamano
2020-09-30 21:56       ` Robert Karszniewicz

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).