error(?) in "man git-stash" regarding "--keep-index"

error(?) in "man git-stash" regarding "--keep-index"

[Robert P. J. Day]

  toward the bottom of "man git-stash", one reads part of an example:

  # ... hack hack hack ...
  $ git add --patch foo            # add just first part to the index
  $ git stash push --keep-index    # save all other changes to the stash
                                              ^^^^^ ???

i thought that, even if "--keep-index" left staged changes in the
index, it still included those staged changes in the stash. that's not
the impression one gets from the above.

rday

Re: error(?) in "man git-stash" regarding "--keep-index"

[Martin Ågren]

On 18 May 2018 at 11:37, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
>
>   toward the bottom of "man git-stash", one reads part of an example:
>
>   # ... hack hack hack ...
>   $ git add --patch foo            # add just first part to the index
>   $ git stash push --keep-index    # save all other changes to the stash
>                                               ^^^^^ ???
>
> i thought that, even if "--keep-index" left staged changes in the
> index, it still included those staged changes in the stash. that's not
> the impression one gets from the above.

So would the error be in the part of the man-page quoted below?

  If the --keep-index option is used, all changes already added to
  the index are left intact.

That is, this doesn't say *where* things are left intact (in the index?
in the working tree?). The man-page does start with

  git-stash - Stash the changes in a dirty working directory away

which to me suggests that "leaving something intact" refers to precisely
this -- the working directory.

Or is it the name of the option that trips you up? That is, you read the
name as `--keep-the-index-as-is-but-stash-as-usual`, as opposed to
`--keep-what-is-already-in-the-index-around`?

While I'm sure that some clarification could be provided, I'm tempted to
argue that is exactly what the example provides that you quoted from.

Martin

Re: error(?) in "man git-stash" regarding "--keep-index"

[Robert P. J. Day]

[-- Attachment #1: Type: text/plain, Size: 1959 bytes --]

On Fri, 18 May 2018, Martin Ågren wrote:

> On 18 May 2018 at 11:37, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> >
> >   toward the bottom of "man git-stash", one reads part of an example:
> >
> >   # ... hack hack hack ...
> >   $ git add --patch foo            # add just first part to the index
> >   $ git stash push --keep-index    # save all other changes to the stash
> >                                               ^^^^^ ???
> >
> > i thought that, even if "--keep-index" left staged changes in the
> > index, it still included those staged changes in the stash. that's
> > not the impression one gets from the above.
>
> So would the error be in the part of the man-page quoted below?
>
>   If the --keep-index option is used, all changes already added to
>   the index are left intact.

  no, that part is correct, it clearly(?) states that staged changes
are left where they are, in the index. i submit that the misleading
part is in the example i quoted, which suggests that only the "other"
changes are saved to the stash -- that is, the changes other than what
is already in the index.

> That is, this doesn't say *where* things are left intact (in the
> index? in the working tree?).

  in that case, that's something that could obviously be clarified.

> The man-page does start with
>
>   git-stash - Stash the changes in a dirty working directory away
>
> which to me suggests that "leaving something intact" refers to
> precisely this -- the working directory.
>
> Or is it the name of the option that trips you up? That is, you read
> the name as `--keep-the-index-as-is-but-stash-as-usual`, as opposed
> to `--keep-what-is-already-in-the-index-around`?
>
> While I'm sure that some clarification could be provided, I'm tempted to
> argue that is exactly what the example provides that you quoted from.

  i guess we can agree to disagree, since i think the snippet of the
example i provided gives a misleading explanation.

rday

Re: error(?) in "man git-stash" regarding "--keep-index"

[Martin Ågren]

On 18 May 2018 at 12:51, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> On Fri, 18 May 2018, Martin Ågren wrote:
>
>> On 18 May 2018 at 11:37, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
>> >
>> >   toward the bottom of "man git-stash", one reads part of an example:
>> >
>> >   # ... hack hack hack ...
>> >   $ git add --patch foo            # add just first part to the index
>> >   $ git stash push --keep-index    # save all other changes to the stash
>> >                                               ^^^^^ ???
>> >
>> > i thought that, even if "--keep-index" left staged changes in the
>> > index, it still included those staged changes in the stash. that's
>> > not the impression one gets from the above.

If I try to follow the example, it does exactly what it purports to do
and I understand why I would want to use this exact technique with
`--keep-index` in the situation outlined: "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"

So I claim that the example is correct in the sense that it describes
what happens when one uses git in the real world. Another question is
whether the example and the real-world behavior match the impression
you have from elsewhere, e.g., from earlier in the man-page.

That's why I asked this:

>> So would the error be in the part of the man-page quoted below?
>>
>>   If the --keep-index option is used, all changes already added to
>>   the index are left intact.
>
>   no, that part is correct, it clearly(?) states that staged changes
> are left where they are, in the index. i submit that the misleading
> part is in the example i quoted, which suggests that only the "other"
> changes are saved to the stash -- that is, the changes other than what
> is already in the index.

Could you try the example? I think it is important that we find whether
the source of confusion is the example or the earlier explanation
quoted just here.

>> That is, this doesn't say *where* things are left intact (in the
>> index? in the working tree?).
>
>   in that case, that's something that could obviously be clarified.

Would you suggest adding something like "both in the index and in the
working tree" to what I quoted above?

>> The man-page does start with
>>
>>   git-stash - Stash the changes in a dirty working directory away
>>
>> which to me suggests that "leaving something intact" refers to
>> precisely this -- the working directory.
>>
>> Or is it the name of the option that trips you up? That is, you read
>> the name as `--keep-the-index-as-is-but-stash-as-usual`, as opposed
>> to `--keep-what-is-already-in-the-index-around`?
>>
>> While I'm sure that some clarification could be provided, I'm tempted to
>> argue that is exactly what the example provides that you quoted from.
>
>   i guess we can agree to disagree, since i think the snippet of the
> example i provided gives a misleading explanation.

We can disagree on many things, but I would very much like us to agree
on whether the example correctly describes what the command does, or
not.

Then, if the error or source of confusion is not there, then let's
identify it and see if we can find out how to address it.

I suggest that the source of confusion is here:

  If the --keep-index option is used, all changes already added to
  the index are left intact.

Martin

Re: error(?) in "man git-stash" regarding "--keep-index"

[Sybille Peters]

My 2c on this:

1) "If the --keep-index option is used, all changes already added to the 
index are left intact" (manpage git stash)

That appears to be correct and clear


2) "$ git stash push --keep-index    # save *all other* changes to the 
stash" (manpage git stash)

That is either not correct or misleading. "All other" implies in my 
opinion all changes

except the ones that were already added. *"All changes including already 
staged changes"*

might be a better choice.


Please also see open question on StackOverflow:

https://stackoverflow.com/questions/50242489/how-to-ignore-added-hunks-in-git-stash-p/

Re: error(?) in "man git-stash" regarding "--keep-index"

[Robert P. J. Day]

On Fri, 18 May 2018, Sybille Peters wrote:

> My 2c on this:
>
> 1) "If the --keep-index option is used, all changes already added to
>    the index are left intact" (manpage git stash)
>
> That appears to be correct and clear

  yup, that's not the issue.

> 2) "$ git stash push --keep-index # save *all other* changes to the
>    stash"  (manpage git stash)
>
> That is either not correct or misleading. "All other" implies in my
> opinion all changes except the ones that were already added. *"All
> changes including already staged changes"* might be a better choice.

  yup, that is *exactly* the point i was trying to make.

rday

Re: error(?) in "man git-stash" regarding "--keep-index"

[Martin Ågren]

On 18 May 2018 at 17:43, Robert P. J. Day <rpjday@crashcourse.ca> wrote:
> On Fri, 18 May 2018, Sybille Peters wrote:
>
>> My 2c on this:
>>
>> 1) "If the --keep-index option is used, all changes already added to
>>    the index are left intact" (manpage git stash)
>>
>> That appears to be correct and clear
>
>   yup, that's not the issue.
>
>> 2) "$ git stash push --keep-index # save *all other* changes to the
>>    stash"  (manpage git stash)
>>
>> That is either not correct or misleading. "All other" implies in my
>> opinion all changes except the ones that were already added. *"All
>> changes including already staged changes"* might be a better choice.

Thank you Sybille for formulating these two points.

>   yup, that is *exactly* the point i was trying to make.

Ah, this is about saving to the stash vs stashing away. The latter is
what `git stash` is all about -- stashing changes *away*. At least
according to my mental model and the top of the man-page.

But testing this -- not only from the point of view of the example, by
pushing and popping, but by actually investigating what is in the stash
-- finally makes me see what you mean. Yes, the whole lot gets saved to
the stash.

So there is a difference between what gets saved to the stash and what
gets removed from the working directory. The comment in the example
places itself somewhere in the middle by using the word "save" but
really talking about what gets dropped from the working tree.

The proposed wording does not really address that. It could be taken to
mean "all changes are saved to the stash; none are removed from the
working tree".

The work flow in the example is about temporarily stashing a few changes
(changes B) to test a couple of others (changes A). Whether the stash
entry contains changes A or not is practically irrelevant to the use
case. At pop-time, auto-merging will do the correct thing.

So how about "All changes are saved to the stash. Those that have been
added to the index are left intact in the working tree, all others are
removed from the working tree."? That's quite a lot of text. Maybe
"save all changes to the stash, make the working tree match the index"?

Or more to the point: "make the working directory match the index" or
"keep only what is in the index"?

Martin

Re: error(?) in "man git-stash" regarding "--keep-index"

[Robert P. J. Day]

On Fri, 18 May 2018, Sybille Peters wrote:

> My 2c on this:
>
> 1) "If the --keep-index option is used, all changes already added to
>    the index are left intact" (manpage git stash)
>
> That appears to be correct and clear
>
>
> 2) "$ git stash push --keep-index # save *all other* changes to the
>    stash"  (manpage git stash)
>
> That is either not correct or misleading. "All other" implies in my
> opinion all changes except the ones that were already added. *"All
> changes including already staged changes"* might be a better choice.
>
> Please also see open question on StackOverflow:
>
> https://stackoverflow.com/questions/50242489/how-to-ignore-added-hunks-in-git-stash-p/

  hilariously, that SO piece refers to an issue posted to github here:

https://github.com/progit/progit2/issues/822

which was, in fact, posted by me. :-) in any event, let me summarize a
couple things here.

  when i first read up on git stash, it was just that section in the
man page that threw me, and once i figured out how it worked, i
thought of how *i* would have explained it, and it would have gone
like this (assuming i do, in fact, now understand it correctly, which
is by no means guaranteed).

  first, when you do "git stash push", what *always* happens is that
what is stashed is, in two parts, changes in the working directory,
and what is staged in the index. clearly, the staged changes are a
subset of the overall working directory changes, but what's important
is that the stash contains *all* of those changes and, more
importantly, distinguishes between the two categories. that's the
crucial part -- what is stashed (regardless of "--keep-index" or not)
is:

  1) staged changes
  2) unstaged changes

can we agree on that? the only difference made by "--keep-index" is
that the staged changes, in addition to being stashed, are left in the
index. but that makes no difference to what is stashed, do i have that
right?

  now, when popping (or applying), what is popped are all of the
changes in the stash, back into the working directory. period. that
always happens, correct? the only difference introduced by "--index"
is that the pop/apply *also* tries to restage what was staged before.

  is all of the above correct? if it had been explained that way, i
wouldn't have spent several confused hours trying to figure out why i
wasn't getting the results i was expecting.

rday

Re: error(?) in "man git-stash" regarding "--keep-index"

[Sybille Peters]

On 18.05.2018 19:14, Martin Ågren wrote:
> On 18 May 2018 at 17:43, Robert P. J. Day <rpjday@crashcourse.ca> wrote:

...

> Ah, this is about saving to the stash vs stashing away. The latter is
> what `git stash` is all about -- stashing changes *away*. At least
> according to my mental model and the top of the man-page.

Stashing changes *away* would mean a move in my mental model (as in
command line mv): Whatever gets copied to stash gets removed in working 
area. And what doesn't get removed, doesn't get copied to stash.

Thus, --keep-index *would* ignore the index and only move working-tree
to the stash minus the changes already staged.

BUT, that might lead to other problems, I suppose.

The current behaviour can cause problems as in the Stackoverflow
question where someone already staged some hunks, and on
stashing with -p gets asked about these *again*. Which does make
the workflow more tedious.


> 
> ...
> 
> The work flow in the example is about temporarily stashing a few changes
> (changes B) to test a couple of others (changes A). Whether the stash
> entry contains changes A or not is practically irrelevant to the use
> case. At pop-time, auto-merging will do the correct thing.

Aha, now it gets interesting. So you are saying, it doesn't matter if
the stash entry also contains the already staged changes A? Well in the 
combination with -p it does (see Stackoverflow question and above).

Even if auto-merging will take care of it (?), it would be good to
have the manpage clarify things that might confuse people.

Disclaimer: I am not a git expert, so I may be getting things wrong. I
just noticed the Stackoverflow question, answered it (only because
an adequate answer did not exist yet) and then saw a similar topic
being raised here.

> 
> So how about "All changes are saved to the stash. Those that have been
> added to the index are left intact in the working tree, all others are
> removed from the working tree."? That's quite a lot of text. Maybe
> "save all changes to the stash, make the working tree match the index"?
> 
> Or more to the point: "make the working directory match the index" or
> "keep only what is in the index"?


I am not sure yet about the text. Still in the clarifying phase.

Sybille

> 
> Martin
>