patch, 3 partscompletion: improvements for git stash

In this series we stop completing the 'git stash save' subcommand in git-completion.bash. The command has been deprecated for a while, so I think we're doing the users a disservice by still completing it, making them think it's still supported while we already deprecated it.

The first commit is a preparatory step for doing that, while the third commit is an improvement that I thought would be good to do "while there". The most interesting part of the series should be the second commit, where we actually remove the completion of 'git stash save'.

Thomas Gummerer (3):
  completion: rename save_opts to default_opts for stash
  completion: stop completing 'save' as stash subcommand
  completion: make stash -p and alias for stash push -p

 contrib/completion/git-completion.bash | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

'git stash save' is deprecated, but we still call the options for completion 'save_opts'. Simply renaming them to 'push_opts' is not ideal because for example '--message foo' can't be passed directly to 'git stash', as non-option arguments are not allowed, and foo would be treated as one.

Completing '--message' in 'git stash --<tab>' would end up being quite confusing for the user. Therefore name them 'default_opts', and keep treating --message specially for 'git stash push'.

The ulterior motive for renaming 'save_opts' is that in the next commit we're going to stop completing 'git stash save', so 'save_opts' would be the only remaining thing referring to 'git stash save', which would probably be confusing for future readers of this code.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 contrib/completion/git-completion.bash | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

The 'save' subcommand in git stash has been deprecated in fd2ebf14db ("stash: mark "git stash save" deprecated in the man page", 2017-10-22). It is however still completed by the git bash completion.

Stop completing the 'save' subcommand as a further step in the deprecation process. As the only use of the bash completion is interactive, this wouldn't break any scripts, but may give users a hint that the command is deprecated.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 contrib/completion/git-completion.bash | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

We define 'git stash -p' as an alias for 'git stash push -p' in the manpage. Do the same in the completion script, so all options that can be given to 'git stash push' are being completed when the user is using 'git stash -p --<tab>'. Currently the only additional option the user will get is '--message', but there may be more in the future.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 contrib/completion/git-completion.bash | 3 +++
 1 file changed, 3 insertions(+)

Thomas Gummerer <t.gummerer@gmail.com> writes:

Anything deprecated in Oct 2017 is still too young in Git's timescale, but this is the right thing to do in the longer term.

Regarding [2/3], I think 

 - It is perfectly fine to stop offering "save" among the choices
   when "git stash <TAB>" is given, so that we AVOID actively
   suggesting "save" to those who do not know (or do not want to
   learn) about it.  Instead we would knudge them towards "push".
   After all, that is what "deprecation" is all about.

 - It also is OK to limit the offering to "show" against "git stash
   s<TAB>", even though the user _could_ have meant "save" than the
   above case with totally empty subcommand name.

 - It is questionable to stop offering "save" to "git stash
   sav<TAB>" it is clear that the user wants to say "save" in this
   case, as there is no other subcommand the user could have meant.

 - It is outright hostile to the end user if we stop completing "git
   stash save --q<TAB>" with "--quiet".  At that point, we _know_
   that the user wants "save", and getting in the way of those who
   know what they are using does not feel quite right.

Of course, being more accomodating to existing users by taking the last two points above seriously would raise a separate issue of when we stop doing so, and if we should start doing something else. If there is a way to implement a "reluctant completion" that gives "save" as a completion choice while giving a deprecation warning to let the user know of the plan for removal of "save", that would be good, for example.

Thanks.

On 04/18, Junio C Hamano wrote:

Fair enough. I vaguely remembered this thread [1], that tried to add 'rm' back to the autocompletion after it was removed originally to try and start the deprecation process. Reading the thread again now though it seems what you outline below to just not show "save" when "git stash <TAB>" or "git stash s<TAB>" is typed makes more sense as a step now.

I also notice that we never seemed to have taken any of the suggestions made there (either adding 'rm' back to the completion options, or going further in the deprecation process), though that's a different topic :)

Thanks for the suggestions, I'll take a closer look at what could be done, and will send a re-roll.

[1]: 01020160a0004473-277c3d7c-4e3b-4c50-9d44-4a106f37f1d9-000000@eu-west-1.amazonses.com

Previous round was at <20180417212945.24002-1-t.gummerer@gmail.com>.

Thanks Junio for your input on the previous round.

This round drops what was 1/3 in the previous round. We keep completing the options for 'git stash save', so calling the variable 'save_opts' and defining what would be 'push_opts' as 'save_opts' + '--message' makes sense.

2/3 (now 1/2) was mostly rewritten. We now no longer suggest 'save' in 'git stash <tab>', complete 'git stash s<tab>' will complete to 'git stash show', but 'git stash sa<tab>' (or longer) will keep completing to 'git stash save', as the user most likely already knows about 'git stash save', and wanted to type that. We also keep completing the options for 'git stash save' on 'git stash save --<tab>'.

3/3 (now 2/2) stays the same as in the previous round.

I didn't find a good way to implement "reluctant completion" (I'm also by no means an expert in bash completion, so there may well be a way I couldn't find by googl'ing around), so I left that out of this series.

I don't think it's strictly necessary for the deprecation either, as we can just print a warning message when the user actually uses 'git stash save' at some point, which would make a message printed when using the completion redundant anyway. I feel like that warning message is not something we're quite ready for yet and I'd rather wait a few more releases before doing that though.

Thomas Gummerer (2):
  completion: stop showing 'save' for stash by default
  completion: make stash -p and alias for stash push -p

 contrib/completion/git-completion.bash | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

The 'save' subcommand in git stash has been deprecated in fd2ebf14db ("stash: mark "git stash save" deprecated in the man page", 2017-10-22).

Stop showing it when the users enters 'git stash <tab>' or 'git stash s<tab>'. Keep showing it however when the user enters 'git stash sa<tab>' or any more characters of the 'save' subcommand. This is designed to not encourage users to use 'git stash save', but still leaving the completion option once it's clear that's what the user means.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 contrib/completion/git-completion.bash | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

On Fri, Apr 20, 2018 at 1:25 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:

I don't think this is worth it. You only save two keystrokes for 've' and already waste one on <tab>.

On 04/20, Duy Nguyen wrote:

I think the main reason for keeping the completion is not actually saving keystrokes, but rather not giving the users the false impression that we removed the command, or that something is wrong with it, without properly warning them before. That was the main reason given in [1] why the completion is useful even for short commands such as 'rm'.

So while I had the same impression before re-reading that thread, I think keeping the completion for 'git stash sa<tab>' is the right thing to do, and we can remove it some time after we started warning about 'git stash save' being deprecated.

[1]: <01020160a0004473-277c3d7c-4e3b-4c50-9d44-4a106f37f1d9-000000@eu-west-1.amazonses.com>

We define 'git stash -p' as an alias for 'git stash push -p' in the manpage. Do the same in the completion script, so all options that can be given to 'git stash push' are being completed when the user is using 'git stash -p --<tab>'. Currently the only additional option the user will get is '--message', but there may be more in the future.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 contrib/completion/git-completion.bash | 3 +++
 1 file changed, 3 insertions(+)

Thomas Gummerer <t.gummerer@gmail.com> writes:

Yeah, I agree that what you outined above is quite sensible.

Thanks, will queue.