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 p...@peff.net 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-30 Thread Junio C Hamano
On Wed, Jul 30, 2014 at 5:17 PM, Jeff King p...@peff.net 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


[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 p...@peff.net
---
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


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

2014-07-29 Thread Junio C Hamano
Jeff King p...@peff.net 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