On Wed, Aug 06, 2014 at 10:12:25AM -0700, Junio C Hamano wrote:
I think we want to drop the stash show patch, based on the discussion
we had. The first three patches are nominally prep for that final
patch, but actually are things I've often wanted over the years. I'd be
glad if they made it in separately, but there were some compatibility
questions.
I am not sure what compatibility you are worried about. The empty
format one looks like a pure bugfix to me, and I agree that they
are good changes regardless of the remainder of the series.
I was mostly worried that somebody is relying on the weird current
behavior with the blank line. I'm inclined to call it a bugfix, too.
As clever as I find the --simplify-combined-diff patch, I think we came
to the conclusion that --first-parent is probably the reasonable
choice. It matches stash show, and it's simple and obvious. Do we just
want a patch to specify --first-parent to stash-log? That would make
-p just work. The only downside is that there isn't a good way to turn
it off.
Perhaps we can add --no-first-parent to countermand it?
I started down that road and then realized that --first-parent is not
enough. It is only interesting combined with -m. But it turns out that
using the two together does exactly what we want, and is overridden
as you would hope with just --cc.
See the patch below, which I think could replace the top three from
jk/stash-list-p (or really, could replace the whole series, and the
bottom three could go into their own topic).
-- 8 --
Subject: stash: default listing to working-tree diff
When you list stashes, you can provide arbitrary git-log
options to change the display. However, adding just -p
does nothing, because each stash is actually a merge commit.
This implementation detail is easy to forget, leading to
confused users who think -p is not working. We can make
this easier by defaulting to --first-parent -m, which will
show the diff against the working tree. This omits the
index portion of the stash entirely, but it's simple and it
matches what git stash show provides.
People who are more clueful about stash's true form can use
--cc to override the -m, and the --first-parent will
then do nothing. For diffs, it only affects non-combined
diffs, so --cc overrides it. And for the traversal, we are
walking the linear reflog anyway, so we do not even care
about the parents.
Signed-off-by: Jeff King p...@peff.net
---
git-stash.sh | 2 +-
t/t3903-stash.sh | 42 ++
2 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/git-stash.sh b/git-stash.sh
index bcc757b..9c1ba8e 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -297,7 +297,7 @@ have_stash () {
list_stash () {
have_stash || return 0
- git log --format=%gd: %gs -g $@ $ref_stash --
+ git log --format=%gd: %gs -g --first-parent -m $@ $ref_stash --
}
show_stash () {
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 5b79b21..1e29962 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -685,4 +685,46 @@ test_expect_success 'handle stash specification with
spaces' '
grep pig file
'
+test_expect_success 'setup stash with index and worktree changes' '
+ git stash clear
+ git reset --hard
+ echo index file
+ git add file
+ echo working file
+ git stash
+'
+
+test_expect_success 'stash list implies --first-parent -m' '
+ cat expect -\EOF
+ stash@{0}: WIP on master: b27a2bc subdir
+
+ diff --git a/file b/file
+ index 257cc56..d26b33d 100644
+ --- a/file
+ +++ b/file
+ @@ -1 +1 @@
+ -foo
+ +working
+ EOF
+ git stash list -p actual
+ test_cmp expect actual
+'
+
+test_expect_success 'stash list --cc shows combined diff' '
+ cat expect -\EOF
+ stash@{0}: WIP on master: b27a2bc subdir
+
+ diff --cc file
+ index 257cc56,9015a7a..d26b33d
+ --- a/file
+ +++ b/file
+ @@@ -1,1 -1,1 +1,1 @@@
+ - foo
+-index
+ ++working
+ EOF
+ git stash list -p --cc actual
+ test_cmp expect actual
+'
+
test_done
--
2.1.0.rc0.286.g5c67d74
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
