Re: [RFC/PATCH 3/2] stash: show combined diff with "stash show"

2014-07-30 Thread Junio C Hamano
On Wed, Jul 30, 2014 at 5:17 PM, Jeff King  wrote:
>
> Like I said, I'm iffy on this part of the series for that reason. But
> I'm curious: what do you think should happen in such a use case when
> there are staged contents in the index? Right now we completely ignore
> them.

I think ignoring is absolutely the right thing ;-)

Unlike "stash pop", which is "try to bring me back to exactly the same state",
it is a strong indication that the user wants to further tweak the previous work
to grab a patch and apply to the working tree---and while you are working
inside the working tree, you haven't started adding the index contents, so
"git diff" after such an operation should show what you grabbed out of the
stash, applied to the working tree, relative to what you had in the index.
--
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


Re: [RFC/PATCH 3/2] stash: show combined diff with "stash show"

2014-07-30 Thread Jeff King
On Tue, Jul 29, 2014 at 11:13:37AM -0700, Junio C Hamano wrote:

> Jeff King  writes:
> 
> > ... People might be doing things like "git stash show | git
> > apply", and would want to ignore the index content ...
> 
> FWIW, that is exactly how I use "git stash show -p" most of the time.

Like I said, I'm iffy on this part of the series for that reason. But
I'm curious: what do you think should happen in such a use case when
there are staged contents in the index? Right now we completely ignore
them.

-Peff
--
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


Re: [RFC/PATCH 3/2] stash: show combined diff with "stash show"

2014-07-29 Thread Junio C Hamano
Jeff King  writes:

> ... People might be doing things like "git stash show | git
> apply", and would want to ignore the index content ...

FWIW, that is exactly how I use "git stash show -p" most of the time.

--
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


[RFC/PATCH 3/2] stash: show combined diff with "stash show"

2014-07-29 Thread Jeff King
A stash may store not only working tree changes, but also
changes to the index. However, "git stash show" will only
ever show the working tree changes. We can instead show both
as a combined diff, but use "--simplify-combined-diff" so
that we show a normal pairwise diff in the common case that
there were no index changes.

Signed-off-by: Jeff King 
---
I also considered this on top of the other two, but it probably _isn't_
a good idea. People might be doing things like "git stash show | git
apply", and would want to ignore the index content (of course it could
still work in the cases where the index was unchanged, and maybe it is a
good thing that it would break when there are index changes that the
user might be forgetting about).

As you can see from the changes in the test scripts, though, the
implementation isn't quite there either. There's no way to convince "git
show" to produce _no_ pretty-printed output (the best we can do is an
extra blank line). And of course the resulting diffs are single-parent
combined diffs, not "real" pairwise diffs.

We can fix those, but as I said, I'm not sure it is a good change even
without those warts.

 git-stash.sh |  3 ++-
 t/t3903-stash.sh | 42 ++
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index a0246d5..9b13853 100755
--- a/git-stash.sh
+++ b/git-stash.sh
@@ -304,7 +304,8 @@ list_stash () {
 show_stash () {
assert_stash_like "$@"
 
-   git diff ${FLAGS:---stat} $b_commit $w_commit
+   git show --pretty=format: --simplify-combined-diff \
+   ${FLAGS:---stat} $w_commit
 }
 
 #
diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index 54154d2..045db51 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh
@@ -460,6 +460,7 @@ test_expect_success 'stash show format defaults to --stat' '
STASH_ID=$(git stash create) &&
git reset --hard &&
cat >expected <<-EOF &&
+
 file | 1 +
 1 file changed, 1 insertion(+)
EOF
@@ -477,7 +478,7 @@ test_expect_success 'stash show - stashes on stack, 
stash-like argument' '
echo bar >> file &&
STASH_ID=$(git stash create) &&
git reset --hard &&
-   echo "1 0   file" >expected &&
+   { echo; echo "1 0   file"; } >expected &&
git stash show --numstat ${STASH_ID} >actual &&
test_cmp expected actual
 '
@@ -493,11 +494,12 @@ test_expect_success 'stash show -p - stashes on stack, 
stash-like argument' '
STASH_ID=$(git stash create) &&
git reset --hard &&
cat >expected <<-EOF &&
-   diff --git a/file b/file
-   index 7601807..935fbd3 100644
+
+   diff --cc file
+   index 7601807..935fbd3
--- a/file
+++ b/file
-   @@ -1 +1,2 @@
+   @@ -1,1 +1,2 @@
 baz
+bar
EOF
@@ -512,7 +514,7 @@ test_expect_success 'stash show - no stashes on stack, 
stash-like argument' '
echo foo >> file &&
STASH_ID=$(git stash create) &&
git reset --hard &&
-   echo "1 0   file" >expected &&
+   { echo; echo "1 0   file"; } >expected &&
git stash show --numstat ${STASH_ID} >actual &&
test_cmp expected actual
 '
@@ -525,11 +527,12 @@ test_expect_success 'stash show -p - no stashes on stack, 
stash-like argument' '
STASH_ID=$(git stash create) &&
git reset --hard &&
cat >expected <<-EOF &&
-   diff --git a/file b/file
-   index 7601807..71b52c4 100644
+
+   diff --cc file
+   index 7601807..71b52c4
--- a/file
+++ b/file
-   @@ -1 +1,2 @@
+   @@ -1,1 +1,2 @@
 baz
+foo
EOF
@@ -537,6 +540,29 @@ test_expect_success 'stash show -p - no stashes on stack, 
stash-like argument' '
test_cmp expected actual
 '
 
+test_expect_success 'stash show -p will show modified index' '
+   git stash clear &&
+   test_when_finished "git reset --hard HEAD" &&
+   git reset --hard &&
+   echo index >file &&
+   git add file &&
+   echo working >file &&
+   git stash &&
+   cat >expect <<-\EOF &&
+
+   diff --cc file
+   index 7601807,9015a7a..d26b33d
+   --- a/file
+   +++ b/file
+   @@@ -1,1 -1,1 +1,1 @@@
+   - baz
+-index
+   ++working
+   EOF
+   git stash show -p >actual &&
+   test_cmp expect actual
+'
+
 test_expect_success 'stash drop - fail early if specified stash is not a stash 
reference' '
git stash clear &&
test_when_finished "git reset --hard HEAD && git stash clear" &&
-- 
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