Re: [PATCH 3/3] stash: require a clean index to apply

3 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH 3/3] stash: require a clean index to apply

From: Junio C Hamano <hidden>
Date: 2016-06-15 23:05:15

Jeff King [off-list ref] writes:
So I am trying to figure out what the use case here is. Clearly the
above is a toy case, but why is "stash -k" followed by a quick pop
useful in general? Certainly I use "stash" (without "-k") and a quick
pop all the time, and I think that is what stash was designed for.

The best use case I can think of is Jonathan's original: to see only the
staged content in the working tree, and then restore the original state.
But stash does not currently work very well for that, as shown above.
The canonical use case for "stash -k" is to see only the content to
be committed (for testing), commit it after testing and then pop on
top of the committed result, which is the same as what you saw in
the working tree and the index when you did "stash -k".  I do not
think "stash -k && stash pop" was in the design parameter when "-k"
was added (as you demonstrated, it would not fundamentally work
reliably depending on the differences between HEAD-Index-Worktree).

Re: [PATCH 3/3] stash: require a clean index to apply

From: Jeff King <hidden>
Date: 2016-06-15 23:05:15

On Wed, Jun 10, 2015 at 12:16:25PM -0700, Junio C Hamano wrote:
Jeff King [off-list ref] writes:
quoted
So I am trying to figure out what the use case here is. Clearly the
above is a toy case, but why is "stash -k" followed by a quick pop
useful in general? Certainly I use "stash" (without "-k") and a quick
pop all the time, and I think that is what stash was designed for.

The best use case I can think of is Jonathan's original: to see only the
staged content in the working tree, and then restore the original state.
But stash does not currently work very well for that, as shown above.
The canonical use case for "stash -k" is to see only the content to
be committed (for testing), commit it after testing and then pop on
top of the committed result, which is the same as what you saw in
the working tree and the index when you did "stash -k".  I do not
think "stash -k && stash pop" was in the design parameter when "-k"
was added (as you demonstrated, it would not fundamentally work
reliably depending on the differences between HEAD-Index-Worktree).
It seems like applying a stash made with "-k" is fundamentally
misdesigned in the current code. We would want to apply to the working
tree the difference between the index and the working tree, but instead
we try to apply the difference between the HEAD and the working tree.
Which is nonsensical for this use case (i.e., to apply the diff between
$stash and $stash^2, not $stash^1).

I don't think there is any way to tell that "-k" was used, though. But
even if the user knew that, I do not think there is any option to tell
"stash apply" to do it this way.

I dunno. With respect to the original patch, I am OK if we just want to
revert it. This area of stash seems a bit under-designed IMHO, but if
people were happy enough with it before, I do not think the safety
benefit from ed178ef is that great (it is not saving you from destroying
working tree content, only the index state; the individual content blobs
are still available from git-fsck).

-Peff

Re: [PATCH 3/3] stash: require a clean index to apply

From: bär <hidden>
Date: 2016-06-15 23:05:15

On Wed, Jun 10, 2015 at 4:27 PM, Jeff King [off-list ref] wrote:
I dunno. With respect to the original patch, I am OK if we just want to
revert it. This area of stash seems a bit under-designed IMHO, but if
people were happy enough with it before, I do not think the safety
benefit from ed178ef is that great (it is not saving you from destroying
working tree content, only the index state; the individual content blobs
are still available from git-fsck).
I feel the same way, in fact I'm +1 to revert it until we figure out a
better way to deal with this properly.

-- 
Ber Clausen
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help