git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* [RFC] deprecate git stash save?  (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path)
@ 2017-10-05 20:00 Thomas Gummerer
  2017-10-05 20:10 ` [PATCH 2/1] mention git stash push first in the man page Thomas Gummerer
                   ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Thomas Gummerer @ 2017-10-05 20:00 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Jonathan Nieder, git

On 10/04, Junio C Hamano wrote:
> I do not object with such a well designed deprecation plan for any
> other features, if there are other "favourite" ones people would
> want to deprecate and eventually remove, by the way.

Since you were asking :)  With the introduction of 'git stash push',
my hope was always that we could eventually get rid of 'git stash
save' and only keep one interface around.

As there still many references to it around on the internet, it
probably requires a bit of a longer deprecation plan.  What do you
think about the following:

- Change docs/man pages to use 'git stash push' everywhere instead of
  'git stash save'.
- Mention the deprecation in the release notes and in the man page.
- Print a warning when 'git stash save' is used.
- Eventually get rid of it (maybe something for 3.0?)

The steps above would all occur sequentially with a few releases
between each of them.

A patch for the first step below.  I think that even makes sense if we
don't want to follow through with the deprecation.

--- >8 ---
Subject: [PATCH] replace git stash save with git stash push in the documentation

git stash push is the newer interface for creating a stash.  While we
are still keeping git stash save around for the time being, it's better
to point new users of git stash to the more modern (and more feature
rich) interface, instead of teaching them the older version that we
might want to phase out in the future.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-stash.txt    | 12 ++++++------
 Documentation/gitworkflows.txt |  2 +-
 Documentation/user-manual.txt  |  2 +-
 git-stash.sh                   | 10 +++++-----
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 00f95fee1f..53b2e60aeb 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -33,7 +33,7 @@ and reverts the working directory to match the `HEAD` commit.
 The modifications stashed away by this command can be listed with
 `git stash list`, inspected with `git stash show`, and restored
 (potentially on top of a different commit) with `git stash apply`.
-Calling `git stash` without any arguments is equivalent to `git stash save`.
+Calling `git stash` without any arguments is equivalent to `git stash push`.
 A stash is by default listed as "WIP on 'branchname' ...", but
 you can give a more descriptive message on the command line when
 you create one.
@@ -118,7 +118,7 @@ pop [--index] [-q|--quiet] [<stash>]::
 
 	Remove a single stashed state from the stash list and apply it
 	on top of the current working tree state, i.e., do the inverse
-	operation of `git stash save`. The working directory must
+	operation of `git stash push`. The working directory must
 	match the index.
 +
 Applying the state can fail with conflicts; in this case, it is not
@@ -137,7 +137,7 @@ apply [--index] [-q|--quiet] [<stash>]::
 
 	Like `pop`, but do not remove the state from the stash list. Unlike `pop`,
 	`<stash>` may be any commit that looks like a commit created by
-	`stash save` or `stash create`.
+	`stash push` or `stash create`.
 
 branch <branchname> [<stash>]::
 
@@ -148,7 +148,7 @@ branch <branchname> [<stash>]::
 	`stash@{<revision>}`, it then drops the `<stash>`. When no `<stash>`
 	is given, applies the latest one.
 +
-This is useful if the branch on which you ran `git stash save` has
+This is useful if the branch on which you ran `git stash push` has
 changed enough that `git stash apply` fails due to conflicts. Since
 the stash entry is applied on top of the commit that was HEAD at the
 time `git stash` was run, it restores the originally stashed state
@@ -255,14 +255,14 @@ $ git stash pop
 
 Testing partial commits::
 
-You can use `git stash save --keep-index` when you want to make two or
+You can use `git stash push --keep-index` when you want to make two or
 more commits out of the changes in the work tree, and you want to test
 each change before committing:
 +
 ----------------------------------------------------------------
 # ... hack hack hack ...
 $ git add --patch foo            # add just first part to the index
-$ git stash save --keep-index    # save all other changes to the stash
+$ git stash push --keep-index    # save all other changes to the stash
 $ edit/build/test first part
 $ git commit -m 'First part'     # commit fully tested change
 $ git stash pop                  # prepare to work on all other changes
diff --git a/Documentation/gitworkflows.txt b/Documentation/gitworkflows.txt
index 177610e44e..02569d0614 100644
--- a/Documentation/gitworkflows.txt
+++ b/Documentation/gitworkflows.txt
@@ -40,7 +40,7 @@ beginning. It is always easier to squash a few commits together than
 to split one big commit into several.  Don't be afraid of making too
 small or imperfect steps along the way. You can always go back later
 and edit the commits with `git rebase --interactive` before you
-publish them.  You can use `git stash save --keep-index` to run the
+publish them.  You can use `git stash push --keep-index` to run the
 test suite independent of other uncommitted changes; see the EXAMPLES
 section of linkgit:git-stash[1].
 
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index b4d88af133..1c4e44892d 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1556,7 +1556,7 @@ so on a different branch and then coming back), unstash the
 work-in-progress changes.
 
 ------------------------------------------------
-$ git stash save "work in progress for foo feature"
+$ git stash push "work in progress for foo feature"
 ------------------------------------------------
 
 This command will save your changes away to the `stash`, and
diff --git a/git-stash.sh b/git-stash.sh
index 8b2ce9afda..8ce6929d7f 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -267,11 +267,11 @@ push_stash () {
 			# translation of "error: " takes in your language. E.g. in
 			# English this is:
 			#
-			#    $ git stash save --blah-blah 2>&1 | head -n 2
-			#    error: unknown option for 'stash save': --blah-blah
-			#           To provide a message, use git stash save -- '--blah-blah'
-			eval_gettextln "error: unknown option for 'stash save': \$option
-       To provide a message, use git stash save -- '\$option'"
+			#    $ git stash push --blah-blah 2>&1 | head -n 2
+			#    error: unknown option for 'stash push': --blah-blah
+			#           To provide a message, use git stash push -- '--blah-blah'
+			eval_gettextln "error: unknown option for 'stash push': \$option
+       To provide a message, use git stash push -- '\$option'"
 			usage
 			;;
 		*)
-- 
2.14.1.480.gb18f417b89


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

* [PATCH 2/1] mention git stash push first in the man page
  2017-10-05 20:00 [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path) Thomas Gummerer
@ 2017-10-05 20:10 ` Thomas Gummerer
  2017-10-17  4:14   ` Jeff King
  2017-10-12  0:56 ` [RFC] deprecate git stash save? Junio C Hamano
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Thomas Gummerer @ 2017-10-05 20:10 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Jonathan Nieder, git, Thomas Gummerer

Because 'stash push' and 'stash save' are so closely related they share one
section in the man page.  Currently 'stash save' comes first, as that
was the command that people were historically using.  However this makes
the newer, more feature rich git stash push very easy to overlook.
Change the order to give the newer interface for creating a stash the
more prominent position.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-stash.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 53b2e60aeb..a1ddfb8eae 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -48,8 +48,8 @@ stash index (e.g. the integer `n` is equivalent to `stash@{n}`).
 OPTIONS
 -------
 
-save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
 push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<pathspec>...]::
+save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
 
 	Save your local modifications to a new 'stash entry' and roll them
 	back to HEAD (in the working tree and in the index).
-- 
2.14.1.480.gb18f417b89


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

* Re: [RFC] deprecate git stash save?
  2017-10-05 20:00 [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path) Thomas Gummerer
  2017-10-05 20:10 ` [PATCH 2/1] mention git stash push first in the man page Thomas Gummerer
@ 2017-10-12  0:56 ` Junio C Hamano
  2017-10-12 11:14   ` Junio C Hamano
  2017-10-17  4:13 ` [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path) Jeff King
  2017-10-19 18:33 ` [PATCH v2 1/2] replace git stash save with git stash push in the documentation Thomas Gummerer
  3 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2017-10-12  0:56 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Jeff King, Jonathan Nieder, git

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

> git stash push is the newer interface for creating a stash.  While we
> are still keeping git stash save around for the time being, it's better
> to point new users of git stash to the more modern (and more feature
> rich) interface, instead of teaching them the older version that we
> might want to phase out in the future.

With devil's advocate hat on, because the primary point of "stash"
being "clear the desk quickly", I do not necessarily agree that
"more feature rich" is a plus and something we should nudge newbies
towards.



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

* Re: [RFC] deprecate git stash save?
  2017-10-12  0:56 ` [RFC] deprecate git stash save? Junio C Hamano
@ 2017-10-12 11:14   ` Junio C Hamano
  2017-10-16 18:19     ` Thomas Gummerer
  0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2017-10-12 11:14 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Jeff King, Jonathan Nieder, git

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
>> git stash push is the newer interface for creating a stash.  While we
>> are still keeping git stash save around for the time being, it's better
>> to point new users of git stash to the more modern (and more feature
>> rich) interface, instead of teaching them the older version that we
>> might want to phase out in the future.
>
> With devil's advocate hat on, because the primary point of "stash"
> being "clear the desk quickly", I do not necessarily agree that
> "more feature rich" is a plus and something we should nudge newbies
> towards.

I do not particulary view "feature richness" is the primary benefit
of 'push' that makes it shine.  'save' has a quirk in the command
line option syntax, but 'push' corrects it, and that is why we want
to move users towards the latter.

IOW, I do not object to the agenda; it is just I found the
justification used to push the agenda forward was flawed.

Thanks.

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

* Re: [RFC] deprecate git stash save?
  2017-10-12 11:14   ` Junio C Hamano
@ 2017-10-16 18:19     ` Thomas Gummerer
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Gummerer @ 2017-10-16 18:19 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Jonathan Nieder, git

On 10/12, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Thomas Gummerer <t.gummerer@gmail.com> writes:
> >
> >> git stash push is the newer interface for creating a stash.  While we
> >> are still keeping git stash save around for the time being, it's better
> >> to point new users of git stash to the more modern (and more feature
> >> rich) interface, instead of teaching them the older version that we
> >> might want to phase out in the future.
> >
> > With devil's advocate hat on, because the primary point of "stash"
> > being "clear the desk quickly", I do not necessarily agree that
> > "more feature rich" is a plus and something we should nudge newbies
> > towards.
> 
> I do not particulary view "feature richness" is the primary benefit
> of 'push' that makes it shine.  'save' has a quirk in the command
> line option syntax, but 'push' corrects it, and that is why we want
> to move users towards the latter.

I agree it's not the primary benefit (hence why it's in braces :)),
but I at least some users will eventually want the features 'git stash
push' has to offer, and it's better if they don't have to re-train
their fingers at that point.

But yeah, fixing the quirky command line interface is definitely the
stronger reason for deprecating it.

> IOW, I do not object to the agenda; it is just I found the
> justification used to push the agenda forward was flawed.

I'm happy as long as we agree on the agenda here.  Any opinions about
the patches themselves?  Would you prefer me to resend with an updated
description?

> Thanks.

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

* Re: [RFC] deprecate git stash save?  (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path)
  2017-10-05 20:00 [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path) Thomas Gummerer
  2017-10-05 20:10 ` [PATCH 2/1] mention git stash push first in the man page Thomas Gummerer
  2017-10-12  0:56 ` [RFC] deprecate git stash save? Junio C Hamano
@ 2017-10-17  4:13 ` Jeff King
  2017-10-17 21:47   ` Thomas Gummerer
  2017-10-19 18:33 ` [PATCH v2 1/2] replace git stash save with git stash push in the documentation Thomas Gummerer
  3 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2017-10-17  4:13 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Junio C Hamano, Jonathan Nieder, git

On Thu, Oct 05, 2017 at 09:00:49PM +0100, Thomas Gummerer wrote:

> Since you were asking :)  With the introduction of 'git stash push',
> my hope was always that we could eventually get rid of 'git stash
> save' and only keep one interface around.
> 
> As there still many references to it around on the internet, it
> probably requires a bit of a longer deprecation plan.  What do you
> think about the following:
> 
> - Change docs/man pages to use 'git stash push' everywhere instead of
>   'git stash save'.
> - Mention the deprecation in the release notes and in the man page.
> - Print a warning when 'git stash save' is used.
> - Eventually get rid of it (maybe something for 3.0?)
> 
> The steps above would all occur sequentially with a few releases
> between each of them.

That sounds like a pretty good plan.

> A patch for the first step below.  I think that even makes sense if we
> don't want to follow through with the deprecation.

Agreed. The patch mostly looks good, except:

> diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> index b4d88af133..1c4e44892d 100644
> --- a/Documentation/user-manual.txt
> +++ b/Documentation/user-manual.txt
> @@ -1556,7 +1556,7 @@ so on a different branch and then coming back), unstash the
>  work-in-progress changes.
>  
>  ------------------------------------------------
> -$ git stash save "work in progress for foo feature"
> +$ git stash push "work in progress for foo feature"
>  ------------------------------------------------

This needs "-m", doesn't it?

>  
>  This command will save your changes away to the `stash`, and
> diff --git a/git-stash.sh b/git-stash.sh
> index 8b2ce9afda..8ce6929d7f 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -267,11 +267,11 @@ push_stash () {
>  			# translation of "error: " takes in your language. E.g. in
>  			# English this is:
>  			#
> -			#    $ git stash save --blah-blah 2>&1 | head -n 2
> -			#    error: unknown option for 'stash save': --blah-blah
> -			#           To provide a message, use git stash save -- '--blah-blah'
> -			eval_gettextln "error: unknown option for 'stash save': \$option
> -       To provide a message, use git stash save -- '\$option'"
> +			#    $ git stash push --blah-blah 2>&1 | head -n 2
> +			#    error: unknown option for 'stash push': --blah-blah
> +			#           To provide a message, use git stash push -- '--blah-blah'
> +			eval_gettextln "error: unknown option for 'stash push': \$option
> +       To provide a message, use git stash push -- '\$option'"

And here, too?

-Peff

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

* Re: [PATCH 2/1] mention git stash push first in the man page
  2017-10-05 20:10 ` [PATCH 2/1] mention git stash push first in the man page Thomas Gummerer
@ 2017-10-17  4:14   ` Jeff King
  2017-10-17 21:45     ` Thomas Gummerer
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2017-10-17  4:14 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Junio C Hamano, Jonathan Nieder, git

On Thu, Oct 05, 2017 at 09:10:29PM +0100, Thomas Gummerer wrote:

> Because 'stash push' and 'stash save' are so closely related they share one
> section in the man page.  Currently 'stash save' comes first, as that
> was the command that people were historically using.  However this makes
> the newer, more feature rich git stash push very easy to overlook.
> Change the order to give the newer interface for creating a stash the
> more prominent position.

Seems reasonable, though if we are deprecating "save" should we demote
it from being in the synopsis entirely?

-Peff

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

* Re: [PATCH 2/1] mention git stash push first in the man page
  2017-10-17  4:14   ` Jeff King
@ 2017-10-17 21:45     ` Thomas Gummerer
  2017-10-17 21:47       ` Jeff King
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Gummerer @ 2017-10-17 21:45 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Jonathan Nieder, git

On 10/17, Jeff King wrote:
> On Thu, Oct 05, 2017 at 09:10:29PM +0100, Thomas Gummerer wrote:
> 
> > Because 'stash push' and 'stash save' are so closely related they share one
> > section in the man page.  Currently 'stash save' comes first, as that
> > was the command that people were historically using.  However this makes
> > the newer, more feature rich git stash push very easy to overlook.
> > Change the order to give the newer interface for creating a stash the
> > more prominent position.
> 
> Seems reasonable, though if we are deprecating "save" should we demote
> it from being in the synopsis entirely?

I saw that as a next step, with the "official" deprecation of "save".
I thought we might want to advertise "push" a bit more before actually
officially deprecating "save" and sending the deprecation notice out
in the release notes.

OTOH, dropping it from the synopsis in the man page probably wouldn't
cause much issue, as "push" directly replaces it, and is easily
visible.  Not sure how slow we want to take the deprecation?

> -Peff

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

* Re: [PATCH 2/1] mention git stash push first in the man page
  2017-10-17 21:45     ` Thomas Gummerer
@ 2017-10-17 21:47       ` Jeff King
  2017-10-19 18:08         ` Thomas Gummerer
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2017-10-17 21:47 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Junio C Hamano, Jonathan Nieder, git

On Tue, Oct 17, 2017 at 10:45:15PM +0100, Thomas Gummerer wrote:

> > Seems reasonable, though if we are deprecating "save" should we demote
> > it from being in the synopsis entirely?
> 
> I saw that as a next step, with the "official" deprecation of "save".
> I thought we might want to advertise "push" a bit more before actually
> officially deprecating "save" and sending the deprecation notice out
> in the release notes.

Right, my thinking was that it would still be documented in the manpage,
just lower down. And that would probably say something like "save is a
historical synonym for push, except that it differs in these ways...".

> OTOH, dropping it from the synopsis in the man page probably wouldn't
> cause much issue, as "push" directly replaces it, and is easily
> visible.  Not sure how slow we want to take the deprecation?

I don't think there's any reason to go slow in marking something as
deprecated. It's the part where we follow up and remove or change the
feature that must take a while.

-Peff

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

* Re: [RFC] deprecate git stash save?  (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path)
  2017-10-17  4:13 ` [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path) Jeff King
@ 2017-10-17 21:47   ` Thomas Gummerer
  0 siblings, 0 replies; 18+ messages in thread
From: Thomas Gummerer @ 2017-10-17 21:47 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Jonathan Nieder, git

On 10/17, Jeff King wrote:
> On Thu, Oct 05, 2017 at 09:00:49PM +0100, Thomas Gummerer wrote:
> 
> > Since you were asking :)  With the introduction of 'git stash push',
> > my hope was always that we could eventually get rid of 'git stash
> > save' and only keep one interface around.
> > 
> > As there still many references to it around on the internet, it
> > probably requires a bit of a longer deprecation plan.  What do you
> > think about the following:
> > 
> > - Change docs/man pages to use 'git stash push' everywhere instead of
> >   'git stash save'.
> > - Mention the deprecation in the release notes and in the man page.
> > - Print a warning when 'git stash save' is used.
> > - Eventually get rid of it (maybe something for 3.0?)
> > 
> > The steps above would all occur sequentially with a few releases
> > between each of them.
> 
> That sounds like a pretty good plan.
> 
> > A patch for the first step below.  I think that even makes sense if we
> > don't want to follow through with the deprecation.
> 
> Agreed. The patch mostly looks good, except:

Thanks for the review.

> > diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
> > index b4d88af133..1c4e44892d 100644
> > --- a/Documentation/user-manual.txt
> > +++ b/Documentation/user-manual.txt
> > @@ -1556,7 +1556,7 @@ so on a different branch and then coming back), unstash the
> >  work-in-progress changes.
> >  
> >  ------------------------------------------------
> > -$ git stash save "work in progress for foo feature"
> > +$ git stash push "work in progress for foo feature"
> >  ------------------------------------------------
> 
> This needs "-m", doesn't it?

Yeah, this definitely needs "-m".  Will change.

> >  
> >  This command will save your changes away to the `stash`, and
> > diff --git a/git-stash.sh b/git-stash.sh
> > index 8b2ce9afda..8ce6929d7f 100755
> > --- a/git-stash.sh
> > +++ b/git-stash.sh
> > @@ -267,11 +267,11 @@ push_stash () {
> >  			# translation of "error: " takes in your language. E.g. in
> >  			# English this is:
> >  			#
> > -			#    $ git stash save --blah-blah 2>&1 | head -n 2
> > -			#    error: unknown option for 'stash save': --blah-blah
> > -			#           To provide a message, use git stash save -- '--blah-blah'
> > -			eval_gettextln "error: unknown option for 'stash save': \$option
> > -       To provide a message, use git stash save -- '\$option'"
> > +			#    $ git stash push --blah-blah 2>&1 | head -n 2
> > +			#    error: unknown option for 'stash push': --blah-blah
> > +			#           To provide a message, use git stash push -- '--blah-blah'
> > +			eval_gettextln "error: unknown option for 'stash push': \$option
> > +       To provide a message, use git stash push -- '\$option'"
> 
> And here, too?

Indeed, thanks for catching these.  Will change and re-roll the
patches.

> -Peff

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

* Re: [PATCH 2/1] mention git stash push first in the man page
  2017-10-17 21:47       ` Jeff King
@ 2017-10-19 18:08         ` Thomas Gummerer
  2017-10-20  8:04           ` Robert P. J. Day
  0 siblings, 1 reply; 18+ messages in thread
From: Thomas Gummerer @ 2017-10-19 18:08 UTC (permalink / raw)
  To: Jeff King; +Cc: Junio C Hamano, Jonathan Nieder, git

On 10/17, Jeff King wrote:
> On Tue, Oct 17, 2017 at 10:45:15PM +0100, Thomas Gummerer wrote:
> 
> > > Seems reasonable, though if we are deprecating "save" should we demote
> > > it from being in the synopsis entirely?
> > 
> > I saw that as a next step, with the "official" deprecation of "save".
> > I thought we might want to advertise "push" a bit more before actually
> > officially deprecating "save" and sending the deprecation notice out
> > in the release notes.
> 
> Right, my thinking was that it would still be documented in the manpage,
> just lower down. And that would probably say something like "save is a
> historical synonym for push, except that it differs in these ways...".
> 
> > OTOH, dropping it from the synopsis in the man page probably wouldn't
> > cause much issue, as "push" directly replaces it, and is easily
> > visible.  Not sure how slow we want to take the deprecation?
> 
> I don't think there's any reason to go slow in marking something as
> deprecated. It's the part where we follow up and remove or change the
> feature that must take a while.

Makes sense, let me drop it from the synopsis then.

Thanks!

> -Peff

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

* [PATCH v2 1/2] replace git stash save with git stash push in the documentation
  2017-10-05 20:00 [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path) Thomas Gummerer
                   ` (2 preceding siblings ...)
  2017-10-17  4:13 ` [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path) Jeff King
@ 2017-10-19 18:33 ` Thomas Gummerer
  2017-10-19 18:33   ` [PATCH v2 2/2] mark git stash push deprecated in the man page Thomas Gummerer
  2017-10-21  6:26   ` [PATCH v2 1/2] replace git stash save with git stash push in the documentation Jeff King
  3 siblings, 2 replies; 18+ messages in thread
From: Thomas Gummerer @ 2017-10-19 18:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Jonathan Nieder, git, Robert P . J . Day, Thomas Gummerer

git stash push is the newer interface for creating a stash.  While we
are still keeping git stash save around for the time being, it's better
to point new users of git stash to the more modern (and more feature
rich) interface, instead of teaching them the older version that we
might want to phase out in the future.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-stash.txt    | 12 ++++++------
 Documentation/gitworkflows.txt |  2 +-
 Documentation/user-manual.txt  |  2 +-
 git-stash.sh                   | 10 +++++-----
 4 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 00f95fee1f..53b2e60aeb 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -33,7 +33,7 @@ and reverts the working directory to match the `HEAD` commit.
 The modifications stashed away by this command can be listed with
 `git stash list`, inspected with `git stash show`, and restored
 (potentially on top of a different commit) with `git stash apply`.
-Calling `git stash` without any arguments is equivalent to `git stash save`.
+Calling `git stash` without any arguments is equivalent to `git stash push`.
 A stash is by default listed as "WIP on 'branchname' ...", but
 you can give a more descriptive message on the command line when
 you create one.
@@ -118,7 +118,7 @@ pop [--index] [-q|--quiet] [<stash>]::
 
 	Remove a single stashed state from the stash list and apply it
 	on top of the current working tree state, i.e., do the inverse
-	operation of `git stash save`. The working directory must
+	operation of `git stash push`. The working directory must
 	match the index.
 +
 Applying the state can fail with conflicts; in this case, it is not
@@ -137,7 +137,7 @@ apply [--index] [-q|--quiet] [<stash>]::
 
 	Like `pop`, but do not remove the state from the stash list. Unlike `pop`,
 	`<stash>` may be any commit that looks like a commit created by
-	`stash save` or `stash create`.
+	`stash push` or `stash create`.
 
 branch <branchname> [<stash>]::
 
@@ -148,7 +148,7 @@ branch <branchname> [<stash>]::
 	`stash@{<revision>}`, it then drops the `<stash>`. When no `<stash>`
 	is given, applies the latest one.
 +
-This is useful if the branch on which you ran `git stash save` has
+This is useful if the branch on which you ran `git stash push` has
 changed enough that `git stash apply` fails due to conflicts. Since
 the stash entry is applied on top of the commit that was HEAD at the
 time `git stash` was run, it restores the originally stashed state
@@ -255,14 +255,14 @@ $ git stash pop
 
 Testing partial commits::
 
-You can use `git stash save --keep-index` when you want to make two or
+You can use `git stash push --keep-index` when you want to make two or
 more commits out of the changes in the work tree, and you want to test
 each change before committing:
 +
 ----------------------------------------------------------------
 # ... hack hack hack ...
 $ git add --patch foo            # add just first part to the index
-$ git stash save --keep-index    # save all other changes to the stash
+$ git stash push --keep-index    # save all other changes to the stash
 $ edit/build/test first part
 $ git commit -m 'First part'     # commit fully tested change
 $ git stash pop                  # prepare to work on all other changes
diff --git a/Documentation/gitworkflows.txt b/Documentation/gitworkflows.txt
index 177610e44e..02569d0614 100644
--- a/Documentation/gitworkflows.txt
+++ b/Documentation/gitworkflows.txt
@@ -40,7 +40,7 @@ beginning. It is always easier to squash a few commits together than
 to split one big commit into several.  Don't be afraid of making too
 small or imperfect steps along the way. You can always go back later
 and edit the commits with `git rebase --interactive` before you
-publish them.  You can use `git stash save --keep-index` to run the
+publish them.  You can use `git stash push --keep-index` to run the
 test suite independent of other uncommitted changes; see the EXAMPLES
 section of linkgit:git-stash[1].
 
diff --git a/Documentation/user-manual.txt b/Documentation/user-manual.txt
index b4d88af133..3a03e63eb0 100644
--- a/Documentation/user-manual.txt
+++ b/Documentation/user-manual.txt
@@ -1556,7 +1556,7 @@ so on a different branch and then coming back), unstash the
 work-in-progress changes.
 
 ------------------------------------------------
-$ git stash save "work in progress for foo feature"
+$ git stash push -m "work in progress for foo feature"
 ------------------------------------------------
 
 This command will save your changes away to the `stash`, and
diff --git a/git-stash.sh b/git-stash.sh
index 8b2ce9afda..16919277ba 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -267,11 +267,11 @@ push_stash () {
 			# translation of "error: " takes in your language. E.g. in
 			# English this is:
 			#
-			#    $ git stash save --blah-blah 2>&1 | head -n 2
-			#    error: unknown option for 'stash save': --blah-blah
-			#           To provide a message, use git stash save -- '--blah-blah'
-			eval_gettextln "error: unknown option for 'stash save': \$option
-       To provide a message, use git stash save -- '\$option'"
+			#    $ git stash push --blah-blah 2>&1 | head -n 2
+			#    error: unknown option for 'stash push': --blah-blah
+			#           To provide a message, use git stash push -- '--blah-blah'
+			eval_gettextln "error: unknown option for 'stash push': \$option
+       To provide a message, use git stash push -m '\$option'"
 			usage
 			;;
 		*)
-- 
2.15.0.rc1.65.g660fb3dfa8


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

* [PATCH v2 2/2] mark git stash push deprecated in the man page
  2017-10-19 18:33 ` [PATCH v2 1/2] replace git stash save with git stash push in the documentation Thomas Gummerer
@ 2017-10-19 18:33   ` Thomas Gummerer
  2017-10-21  6:28     ` Jeff King
  2017-10-21  6:26   ` [PATCH v2 1/2] replace git stash save with git stash push in the documentation Jeff King
  1 sibling, 1 reply; 18+ messages in thread
From: Thomas Gummerer @ 2017-10-19 18:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Jeff King, Jonathan Nieder, git, Robert P . J . Day, Thomas Gummerer

'git stash push' fixes a historical wart in the interface of 'git stash
save'.  As 'git stash push' has all functionality of 'git stash save',
with a nicer, more consistent user interface deprecate 'git stash
save'.  To do this, remove it from the synopsis of the man page, and
move it to a separate section, stating that it is deprecated.

Helped-by: Robert P. J. Day <rpjday@crashcourse.ca>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git-stash.txt | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 53b2e60aeb..89b6a0e672 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -13,8 +13,6 @@ SYNOPSIS
 'git stash' drop [-q|--quiet] [<stash>]
 'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
 'git stash' branch <branchname> [<stash>]
-'git stash' save [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
-	     [-u|--include-untracked] [-a|--all] [<message>]
 'git stash' [push [-p|--patch] [-k|--[no-]keep-index] [-q|--quiet]
 	     [-u|--include-untracked] [-a|--all] [-m|--message <message>]]
 	     [--] [<pathspec>...]]
@@ -48,7 +46,6 @@ stash index (e.g. the integer `n` is equivalent to `stash@{n}`).
 OPTIONS
 -------
 
-save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
 push [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [-m|--message <message>] [--] [<pathspec>...]::
 
 	Save your local modifications to a new 'stash entry' and roll them
@@ -87,6 +84,10 @@ linkgit:git-add[1] to learn how to operate the `--patch` mode.
 The `--patch` option implies `--keep-index`.  You can use
 `--no-keep-index` to override this.
 
+save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
+
+	This option is deprecated in favour of 'git stash push'.
+
 list [<options>]::
 
 	List the stash entries that you currently have.  Each 'stash entry' is
-- 
2.15.0.rc1.65.g660fb3dfa8


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

* Re: [PATCH 2/1] mention git stash push first in the man page
  2017-10-19 18:08         ` Thomas Gummerer
@ 2017-10-20  8:04           ` Robert P. J. Day
  2017-10-21  6:20             ` Jeff King
  0 siblings, 1 reply; 18+ messages in thread
From: Robert P. J. Day @ 2017-10-20  8:04 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Jeff King, Junio C Hamano, Jonathan Nieder, git

On Thu, 19 Oct 2017, Thomas Gummerer wrote:

> On 10/17, Jeff King wrote:
> > On Tue, Oct 17, 2017 at 10:45:15PM +0100, Thomas Gummerer wrote:
> >
> > > > Seems reasonable, though if we are deprecating "save" should we demote
> > > > it from being in the synopsis entirely?
> > >
> > > I saw that as a next step, with the "official" deprecation of "save".
> > > I thought we might want to advertise "push" a bit more before actually
> > > officially deprecating "save" and sending the deprecation notice out
> > > in the release notes.
> >
> > Right, my thinking was that it would still be documented in the manpage,
> > just lower down. And that would probably say something like "save is a
> > historical synonym for push, except that it differs in these ways...".
> >
> > > OTOH, dropping it from the synopsis in the man page probably wouldn't
> > > cause much issue, as "push" directly replaces it, and is easily
> > > visible.  Not sure how slow we want to take the deprecation?
> >
> > I don't think there's any reason to go slow in marking something as
> > deprecated. It's the part where we follow up and remove or change the
> > feature that must take a while.
>
> Makes sense, let me drop it from the synopsis then.

  what, exactly, is the oft-referred-to issue with how "git stash
save" works that is being addressed with the new syntax of "git stash
push"?

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

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

* Re: [PATCH 2/1] mention git stash push first in the man page
  2017-10-20  8:04           ` Robert P. J. Day
@ 2017-10-21  6:20             ` Jeff King
  2017-10-21  6:43               ` Robert P. J. Day
  0 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2017-10-21  6:20 UTC (permalink / raw)
  To: Robert P. J. Day; +Cc: Thomas Gummerer, Junio C Hamano, Jonathan Nieder, git

On Fri, Oct 20, 2017 at 04:04:10AM -0400, Robert P. J. Day wrote:

> > > I don't think there's any reason to go slow in marking something as
> > > deprecated. It's the part where we follow up and remove or change the
> > > feature that must take a while.
> >
> > Makes sense, let me drop it from the synopsis then.
> 
>   what, exactly, is the oft-referred-to issue with how "git stash
> save" works that is being addressed with the new syntax of "git stash
> push"?

"stash save" soaks up all arguments as the stash message, so it's not
possible to specify pathspecs. "push" uses "-m <msg>" for the stash
message, and can accept pathspecs.

-Peff

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

* Re: [PATCH v2 1/2] replace git stash save with git stash push in the documentation
  2017-10-19 18:33 ` [PATCH v2 1/2] replace git stash save with git stash push in the documentation Thomas Gummerer
  2017-10-19 18:33   ` [PATCH v2 2/2] mark git stash push deprecated in the man page Thomas Gummerer
@ 2017-10-21  6:26   ` Jeff King
  1 sibling, 0 replies; 18+ messages in thread
From: Jeff King @ 2017-10-21  6:26 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Junio C Hamano, Jonathan Nieder, git, Robert P . J . Day

On Thu, Oct 19, 2017 at 07:33:03PM +0100, Thomas Gummerer wrote:

> diff --git a/git-stash.sh b/git-stash.sh
> index 8b2ce9afda..16919277ba 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -267,11 +267,11 @@ push_stash () {
>  			# translation of "error: " takes in your language. E.g. in
>  			# English this is:
>  			#
> -			#    $ git stash save --blah-blah 2>&1 | head -n 2
> -			#    error: unknown option for 'stash save': --blah-blah
> -			#           To provide a message, use git stash save -- '--blah-blah'
> -			eval_gettextln "error: unknown option for 'stash save': \$option
> -       To provide a message, use git stash save -- '\$option'"
> +			#    $ git stash push --blah-blah 2>&1 | head -n 2
> +			#    error: unknown option for 'stash push': --blah-blah
> +			#           To provide a message, use git stash push -- '--blah-blah'
> +			eval_gettextln "error: unknown option for 'stash push': \$option
> +       To provide a message, use git stash push -m '\$option'"

The user message is fixed here, but doesn't the message for translators
need the same treatment?  I guess it's just talking about the spacing,
so it doesn't need to be completely accurate. But it probably makes
sense to update it at the same time.

As an aside, I do kind of wonder if the right advice for "push" is
different than for "save" here. I.e., should it say "to provide a
pathspec that starts with --, use push -- --blah-blah". I'm not sure it
matters that much. Second-guessing what a user meant seems likely to go
wrong (e.g., "--icnlude-untracked" would trigger this message, which is
just silly). But that's largely orthogonal to what you're doing here.

-Peff

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

* Re: [PATCH v2 2/2] mark git stash push deprecated in the man page
  2017-10-19 18:33   ` [PATCH v2 2/2] mark git stash push deprecated in the man page Thomas Gummerer
@ 2017-10-21  6:28     ` Jeff King
  0 siblings, 0 replies; 18+ messages in thread
From: Jeff King @ 2017-10-21  6:28 UTC (permalink / raw)
  To: Thomas Gummerer; +Cc: Junio C Hamano, Jonathan Nieder, git, Robert P . J . Day

On Thu, Oct 19, 2017 at 07:33:04PM +0100, Thomas Gummerer wrote:

> 'git stash push' fixes a historical wart in the interface of 'git stash
> save'.  As 'git stash push' has all functionality of 'git stash save',
> with a nicer, more consistent user interface deprecate 'git stash
> save'.  To do this, remove it from the synopsis of the man page, and
> move it to a separate section, stating that it is deprecated.

This looks fine.

> @@ -87,6 +84,10 @@ linkgit:git-add[1] to learn how to operate the `--patch` mode.
>  The `--patch` option implies `--keep-index`.  You can use
>  `--no-keep-index` to override this.
>  
> +save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] [-q|--quiet] [<message>]::
> +
> +	This option is deprecated in favour of 'git stash push'.
> +

We could possibly go into more detail, like:

  It differs from "stash push" in that it cannot take pathspecs, and any
  non-option arguments form the message.

or something. Since we don't want people to use it, it probably doesn't
matter much. I just wondered if people would peer at the (long) synopsis
line trying to figure out how it's different.

-Peff

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

* Re: [PATCH 2/1] mention git stash push first in the man page
  2017-10-21  6:20             ` Jeff King
@ 2017-10-21  6:43               ` Robert P. J. Day
  0 siblings, 0 replies; 18+ messages in thread
From: Robert P. J. Day @ 2017-10-21  6:43 UTC (permalink / raw)
  To: Jeff King; +Cc: Thomas Gummerer, Junio C Hamano, Jonathan Nieder, git

On Sat, 21 Oct 2017, Jeff King wrote:

> On Fri, Oct 20, 2017 at 04:04:10AM -0400, Robert P. J. Day wrote:
>
> > > > I don't think there's any reason to go slow in marking something as
> > > > deprecated. It's the part where we follow up and remove or change the
> > > > feature that must take a while.
> > >
> > > Makes sense, let me drop it from the synopsis then.
> >
> >   what, exactly, is the oft-referred-to issue with how "git stash
> > save" works that is being addressed with the new syntax of "git stash
> > push"?
>
> "stash save" soaks up all arguments as the stash message, so it's not
> possible to specify pathspecs. "push" uses "-m <msg>" for the stash
> message, and can accept pathspecs.

  ah, i knew that, yeah, that's the ticket. :-)

rday

-- 

========================================================================
Robert P. J. Day                                 Ottawa, Ontario, CANADA
                        http://crashcourse.ca

Twitter:                                       http://twitter.com/rpjday
LinkedIn:                               http://ca.linkedin.com/in/rpjday
========================================================================

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

end of thread, back to index

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-05 20:00 [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path) Thomas Gummerer
2017-10-05 20:10 ` [PATCH 2/1] mention git stash push first in the man page Thomas Gummerer
2017-10-17  4:14   ` Jeff King
2017-10-17 21:45     ` Thomas Gummerer
2017-10-17 21:47       ` Jeff King
2017-10-19 18:08         ` Thomas Gummerer
2017-10-20  8:04           ` Robert P. J. Day
2017-10-21  6:20             ` Jeff King
2017-10-21  6:43               ` Robert P. J. Day
2017-10-12  0:56 ` [RFC] deprecate git stash save? Junio C Hamano
2017-10-12 11:14   ` Junio C Hamano
2017-10-16 18:19     ` Thomas Gummerer
2017-10-17  4:13 ` [RFC] deprecate git stash save? (was: Re: [PATCH 2/3] http-push: fix construction of hex value from path) Jeff King
2017-10-17 21:47   ` Thomas Gummerer
2017-10-19 18:33 ` [PATCH v2 1/2] replace git stash save with git stash push in the documentation Thomas Gummerer
2017-10-19 18:33   ` [PATCH v2 2/2] mark git stash push deprecated in the man page Thomas Gummerer
2017-10-21  6:28     ` Jeff King
2017-10-21  6:26   ` [PATCH v2 1/2] replace git stash save with git stash push in the documentation Jeff King

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox