Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Jeff King]

On Sat, Jan 28, 2017 at 07:30:28PM +, Thomas Gummerer wrote:

> Thanks all who chimed in here.  My new description is definitely not
> right.  The reason I wanted to change it is part because it's an
> implementation detail, and part because it's going to be not quite
> right when the filename argument is introduced.
> 
> How about the following:
> 
>   Save your local modifications to a new 'stash' and roll them back
>   both in the working tree and in the index.
> 
> As an added bonus this also matches what git stash save -p does.

IMHO that is both informative and accurate. You could add:
 
  (unless --keep-index was used)

at the end of the sentence, though I am not sure it is necessary.

-Peff

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Thomas Gummerer]

On 01/25, Junio C Hamano wrote:
> Jeff King  writes:
> 
> > On Sat, Jan 21, 2017 at 08:08:02PM +, Thomas Gummerer wrote:
> >
> >> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> >> index 2e9cef06e6..0ad5335a3e 100644
> >> --- a/Documentation/git-stash.txt
> >> +++ b/Documentation/git-stash.txt
> >> @@ -47,8 +47,9 @@ OPTIONS
> >>  
> >>  save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] 
> >> [-a|--all] [-q|--quiet] []::
> >>  
> >> -  Save your local modifications to a new 'stash', and run `git reset
> >> -  --hard` to revert them.  The  part is optional and gives
> >> +  Save your local modifications to a new 'stash', and revert the
> >> +  the changes in the working tree to match the index.
> >> +  The  part is optional and gives
> >
> > Hrm. "git reset --hard" doesn't just make the working tree match the
> > index. It also resets the index to HEAD.  So either the original or your
> > new description is wrong.
> >
> > I think it's the latter. We really do reset the index unless
> > --keep-index is specified.
> 
> Correct.  So the patch is a net loss.  Perhaps not requiring readers
> to know "reset --hard" might be an improvement (I personally do not
> think so), but this loses crucial information from the description.
> 
>   Save your local modifications (both in the working tree and
>   to the index) to a new 'stash', and resets the index to HEAD
>   and makes the working tree match the index (i.e. runs "git
>   reset --hard").
> 
> That's one-and-a-half lines longer than the original, though.

Thanks all who chimed in here.  My new description is definitely not
right.  The reason I wanted to change it is part because it's an
implementation detail, and part because it's going to be not quite
right when the filename argument is introduced.

How about the following:

Save your local modifications to a new 'stash' and roll them back
both in the working tree and in the index.

As an added bonus this also matches what git stash save -p does.

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Junio C Hamano]

Jakub Narębski  writes:

>>> -   Save your local modifications to a new 'stash', and run `git reset
>>> -   --hard` to revert them.  The  part is optional and gives
>>> +   Save your local modifications to a new 'stash', and revert the
>>> +   the changes in the working tree to match the index.
>>> +   The  part is optional and gives
>> 
>> Hrm. "git reset --hard" doesn't just make the working tree match the
>> index. It also resets the index to HEAD.  So either the original or your
>> new description is wrong.
>> 
>> I think it's the latter. We really do reset the index unless
>> --keep-index is specified.
>
> I wonder if it is worth mentioning that "saving local modifications"
> in 'git stash' means storing both the state of the worktree (of tracked
> files in it) _and_ the state of the index, before both get set to
> state of HEAD.

Yes, it is, I would say.

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Junio C Hamano]

Jeff King  writes:

> On Sat, Jan 21, 2017 at 08:08:02PM +, Thomas Gummerer wrote:
>
>> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
>> index 2e9cef06e6..0ad5335a3e 100644
>> --- a/Documentation/git-stash.txt
>> +++ b/Documentation/git-stash.txt
>> @@ -47,8 +47,9 @@ OPTIONS
>>  
>>  save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] 
>> [-a|--all] [-q|--quiet] []::
>>  
>> -Save your local modifications to a new 'stash', and run `git reset
>> ---hard` to revert them.  The  part is optional and gives
>> +Save your local modifications to a new 'stash', and revert the
>> +the changes in the working tree to match the index.
>> +The  part is optional and gives
>
> Hrm. "git reset --hard" doesn't just make the working tree match the
> index. It also resets the index to HEAD.  So either the original or your
> new description is wrong.
>
> I think it's the latter. We really do reset the index unless
> --keep-index is specified.

Correct.  So the patch is a net loss.  Perhaps not requiring readers
to know "reset --hard" might be an improvement (I personally do not
think so), but this loses crucial information from the description.

Save your local modifications (both in the working tree and
to the index) to a new 'stash', and resets the index to HEAD
and makes the working tree match the index (i.e. runs "git
reset --hard").

That's one-and-a-half lines longer than the original, though.

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Jakub Narębski]

W dniu 24.01.2017 o 21:14, Jeff King pisze:
> On Sat, Jan 21, 2017 at 08:08:02PM +, Thomas Gummerer wrote:
> 
>> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
>> index 2e9cef06e6..0ad5335a3e 100644
>> --- a/Documentation/git-stash.txt
>> +++ b/Documentation/git-stash.txt
>> @@ -47,8 +47,9 @@ OPTIONS
>>  
>>  save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] 
>> [-a|--all] [-q|--quiet] []::
>>  
>> -Save your local modifications to a new 'stash', and run `git reset
>> ---hard` to revert them.  The  part is optional and gives
>> +Save your local modifications to a new 'stash', and revert the
>> +the changes in the working tree to match the index.
>> +The  part is optional and gives
> 
> Hrm. "git reset --hard" doesn't just make the working tree match the
> index. It also resets the index to HEAD.  So either the original or your
> new description is wrong.
> 
> I think it's the latter. We really do reset the index unless
> --keep-index is specified.

I wonder if it is worth mentioning that "saving local modifications"
in 'git stash' means storing both the state of the worktree (of tracked
files in it) _and_ the state of the index, before both get set to
state of HEAD.

Best,
-- 
Jakub Narębski

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Jeff King]

On Sat, Jan 21, 2017 at 08:08:02PM +, Thomas Gummerer wrote:

> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 2e9cef06e6..0ad5335a3e 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -47,8 +47,9 @@ OPTIONS
>  
>  save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
> [-q|--quiet] []::
>  
> - Save your local modifications to a new 'stash', and run `git reset
> - --hard` to revert them.  The  part is optional and gives
> + Save your local modifications to a new 'stash', and revert the
> + the changes in the working tree to match the index.
> + The  part is optional and gives

Hrm. "git reset --hard" doesn't just make the working tree match the
index. It also resets the index to HEAD.  So either the original or your
new description is wrong.

I think it's the latter. We really do reset the index unless
--keep-index is specified.

I also wondered if it was worth avoiding the word "revert", as "git
revert" has a much different meaning (as opposed to "svn revert", which
does what you're talking about here). But I see that "git add -i"
already uses the word revert in this way (and there are probably
others).

So it may not be worth worrying about, but "set" or "reset" probably
serves the same purpose.

-Peff

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Jakub Narębski]

W dniu 21.01.2017 o 21:08, Thomas Gummerer pisze:
> Don't mention git reset --hard in the documentation for git stash save.
> It's an implementation detail that doesn't matter to the end user and
> thus shouldn't be exposed to them.
> 
> Signed-off-by: Thomas Gummerer 
> ---
>  Documentation/git-stash.txt | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
> index 2e9cef06e6..0ad5335a3e 100644
> --- a/Documentation/git-stash.txt
> +++ b/Documentation/git-stash.txt
> @@ -47,8 +47,9 @@ OPTIONS
>  
>  save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
> [-q|--quiet] []::
>  
> - Save your local modifications to a new 'stash', and run `git reset
> - --hard` to revert them.  The  part is optional and gives
> + Save your local modifications to a new 'stash', and revert the
> + the changes in the working tree to match the index.

I think the following might be better:

..., and set the working tree to match the index.

Or not, as it ignores problem of untracked files.

Anyway, removing internal implementation detail looks like a good idea.
OTOH the reader should be familiar with what `git reset --hard` does,
and if not, he knows where to find the information.

> + The  part is optional and gives
>   the description along with the stashed state.  For quickly making
>   a snapshot, you can omit _both_ "save" and , but giving
>   only  does not trigger this action to prevent a misspelled
> 

-- 
Jakub Narębski

Re: [PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Øyvind A . Holm]

On 2017-01-21 20:08:02, Thomas Gummerer wrote:
> Don't mention git reset --hard in the documentation for git stash save.
> It's an implementation detail that doesn't matter to the end user and
> thus shouldn't be exposed to them.
> [...]
> + Save your local modifications to a new 'stash', and revert the
> + the changes in the working tree to match the index.

The patch contains a duplicated "the".

Regards,
Øyvind

+-| Øyvind A. Holm  - N 60.37604° E 5.9° |-+
| OpenPGP: 0xFB0CBEE894A506E5 - http://www.sunbase.org/pubkey.asc |
| Fingerprint: A006 05D6 E676 B319 55E2  E77E FB0C BEE8 94A5 06E5 |
+| 42073b1c-e041-11e6-bae1-db5caa6d21d3 |-+


signature.asc
Description: PGP signature

[PATCH 1/3] Documentation/stash: remove mention of git reset --hard

[Thomas Gummerer]

Don't mention git reset --hard in the documentation for git stash save.
It's an implementation detail that doesn't matter to the end user and
thus shouldn't be exposed to them.

Signed-off-by: Thomas Gummerer 
---
 Documentation/git-stash.txt | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-stash.txt b/Documentation/git-stash.txt
index 2e9cef06e6..0ad5335a3e 100644
--- a/Documentation/git-stash.txt
+++ b/Documentation/git-stash.txt
@@ -47,8 +47,9 @@ OPTIONS
 
 save [-p|--patch] [-k|--[no-]keep-index] [-u|--include-untracked] [-a|--all] 
[-q|--quiet] []::
 
-   Save your local modifications to a new 'stash', and run `git reset
-   --hard` to revert them.  The  part is optional and gives
+   Save your local modifications to a new 'stash', and revert the
+   the changes in the working tree to match the index.
+   The  part is optional and gives
the description along with the stashed state.  For quickly making
a snapshot, you can omit _both_ "save" and , but giving
only  does not trigger this action to prevent a misspelled
-- 
2.11.0.483.g087da7b7c