git-1.7.3 breakage: "git stash show xxx" doesn't show anything

Downstream bug: http://bugs.gentoo.org/338586

telling git-stash to show a specific stash no longer works with git-1.7.3:
  git stash show stash@{0}
  <no output!?>

Downgrading to dev-vcs/git-1.7.2.3 and it works fine.
Noticed on two stable amd64 systems.

Reproduction:
$ rm -rf foo && mkdir foo && cd foo
$ git init
Initialized empty Git repository in /home/vapier/foo/.git/
$ echo f > f && git add f && git commit -qmm
$ > f
$ git stash
Saved working directory and index state WIP on master: d287dea m
HEAD is now at d287dea m
$ git stash list | cat
stash@{0}: WIP on master: d287dea m
$ git stash show | cat
 f |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
$ git stash show stash@{0} | cat
<nothing!>

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

On 09/24/2010 02:19 PM, Robin H. Johnson wrote:

Downstream bug: http://bugs.gentoo.org/338586

telling git-stash to show a specific stash no longer works with git-1.7.3:
  git stash show stash@{0}
  <no output!?>

Downgrading to dev-vcs/git-1.7.2.3 and it works fine.
Noticed on two stable amd64 systems.

Reproduction:
$ rm -rf foo && mkdir foo && cd foo
$ git init
Initialized empty Git repository in /home/vapier/foo/.git/
$ echo f > f && git add f && git commit -qmm
$ > f
$ git stash
Saved working directory and index state WIP on master: d287dea m
HEAD is now at d287dea m
$ git stash list | cat
stash@{0}: WIP on master: d287dea m
$ git stash show | cat
 f |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)
$ git stash show stash@{0} | cat
<nothing!>

Probably,

diff --git a/git-stash.sh b/git-stash.sh
index 7ce818b..4fbfb62 100755
--- a/git-stash.sh
+++ b/git-stash.sh

@@ -265,7 +265,7 @@ parse_flags_and_rev()
        i_tree=
 
        REV=$(git rev-parse --no-flags --symbolic "$@" 2>/dev/null)
-       FLAGS=$(git rev-parse --no-revs -- "$@" 2>/dev/null)
+       FLAGS=$(git rev-parse --no-revs --flags "$@" 2>/dev/null)
 
        set -- $FLAGS
 

On Sep 24, 2010, at 4:01 PM, Brandon Casey wrote:

I bisected the issue to a9bf09e (detached-stash: simplify git stash show), which is when "git stash show" started using parse_flags_and_rev (via assert_stash_like()).

More worrying to me is that the tests for "git stash show" don't bother to test the output.  I'll be working on that now.

~~ Brian

From: Brandon Casey <redacted>

Recently, the 'stash show' functionality was broken for the case when a
stash-like argument was supplied.  Since, commit 9bf09e, 'stash show' when
supplied a stash-like argument prints nothing and still exists with a zero
status.  Unfortunately, the flaw slipped through the test suite cracks
since the output of 'stash show' was not verified to be correct.

Improve and expand on the existing tests so that this flaws is detected.

Signed-off-by: Brandon Casey <redacted>
---


On 09/24/2010 03:27 PM, Brian Gernhardt wrote:

I bisected the issue to a9bf09e (detached-stash: simplify git stash show),
which is when "git stash show" started using parse_flags_and_rev (via
assert_stash_like()).

More worrying to me is that the tests for "git stash show" don't bother
to test the output.  I'll be working on that now.

I was preparing these tests when your email came in.

hth,
Brandon


 t/t3903-stash.sh |   62 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 1 files changed, 58 insertions(+), 4 deletions(-)

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index a283dca..e8a7338 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh

@@ -406,7 +406,7 @@ test_expect_success 'stash branch - stashes on stack, stash-like argument' '
 	test $(git ls-files --modified | wc -l) -eq 1
 '
 
-test_expect_success 'stash show - stashes on stack, stash-like argument' '
+test_expect_failure 'stash show - stashes on stack, stash-like argument' '
 	git stash clear &&
 	test_when_finished "git reset --hard HEAD" &&
 	git reset --hard &&

@@ -416,16 +416,70 @@ test_expect_success 'stash show - stashes on stack, stash-like argument' '
 	echo bar >> file &&
 	STASH_ID=$(git stash create) &&
 	git reset --hard &&
-	git stash show ${STASH_ID}
+	cat >expected <<-EOF &&
+	 file |    1 +
+	 1 files changed, 1 insertions(+), 0 deletions(-)
+	EOF
+	git stash show ${STASH_ID} >actual &&
+	test_cmp expected actual
 '
-test_expect_success 'stash show - no stashes on stack, stash-like argument' '
+
+test_expect_failure 'stash show -p - stashes on stack, stash-like argument' '
+	git stash clear &&
+	test_when_finished "git reset --hard HEAD" &&
+	git reset --hard &&
+	echo foo >> file &&
+	git stash &&
+	test_when_finished "git stash drop" &&
+	echo bar >> file &&
+	STASH_ID=$(git stash create) &&
+	git reset --hard &&
+	cat >expected <<-EOF &&
+	diff --git a/file b/file
+	index 7601807..935fbd3 100644
+	--- a/file
+	+++ b/file
+	@@ -1 +1,2 @@
+	 baz
+	+bar
+	EOF
+	git stash show -p ${STASH_ID} >actual &&
+	test_cmp expected actual
+'
+
+test_expect_failure 'stash show - no stashes on stack, stash-like argument' '
+	git stash clear &&
+	test_when_finished "git reset --hard HEAD" &&
+	git reset --hard &&
+	echo foo >> file &&
+	STASH_ID=$(git stash create) &&
+	git reset --hard &&
+	cat >expected <<-EOF &&
+	 file |    1 +
+	 1 files changed, 1 insertions(+), 0 deletions(-)
+	EOF
+	git stash show ${STASH_ID} >actual &&
+	test_cmp expected actual
+'
+
+test_expect_failure 'stash show -p - no stashes on stack, stash-like argument' '
 	git stash clear &&
 	test_when_finished "git reset --hard HEAD" &&
 	git reset --hard &&
 	echo foo >> file &&
 	STASH_ID=$(git stash create) &&
 	git reset --hard &&
-	git stash show ${STASH_ID}
+	cat >expected <<-EOF &&
+	diff --git a/file b/file
+	index 7601807..71b52c4 100644
+	--- a/file
+	+++ b/file
+	@@ -1 +1,2 @@
+	 baz
+	+foo
+	EOF
+	git stash show -p ${STASH_ID} >actual &&
+	test_cmp expected actual
 '
 
 test_expect_success 'stash drop - fail early if specified stash is not a stash reference' '

-- 
1.7.3

On Sep 24, 2010, at 4:40 PM, Brandon Casey wrote:

From: Brandon Casey <redacted>

Recently, the 'stash show' functionality was broken for the case when a
stash-like argument was supplied.  Since, commit 9bf09e, 'stash show' when
supplied a stash-like argument prints nothing and still exists with a zero
status.  Unfortunately, the flaw slipped through the test suite cracks
since the output of 'stash show' was not verified to be correct.

Improve and expand on the existing tests so that this flaws is detected.

Signed-off-by: Brandon Casey <redacted>
---


On 09/24/2010 03:27 PM, Brian Gernhardt wrote:

quoted

I bisected the issue to a9bf09e (detached-stash: simplify git stash show),
which is when "git stash show" started using parse_flags_and_rev (via
assert_stash_like()).

More worrying to me is that the tests for "git stash show" don't bother
to test the output.  I'll be working on that now.

I was preparing these tests when your email came in.

I spent longer finding the source of the problem than you did.  I was just putting the finishing touches on it when I got this.  Yours is more through than mine, so I won't bother finishing.

~~ Brian

On 09/24/2010 03:43 PM, Brian Gernhardt wrote:

quoted

On 09/24/2010 03:27 PM, Brian Gernhardt wrote:

quoted

I bisected the issue to a9bf09e (detached-stash: simplify git stash show),
which is when "git stash show" started using parse_flags_and_rev (via
assert_stash_like()).

More worrying to me is that the tests for "git stash show" don't bother
to test the output.  I'll be working on that now.

I was preparing these tests when your email came in.

I spent longer finding the source of the problem than you did.  I was just
putting the finishing touches on it when I got this.  Yours is more through
than mine, so I won't bother finishing.

I hope you're just abandoning the tests you were creating, and _not_
abandoning the search for a fix.  The "solution" I offered is flawed
and breaks some of the other tests. :)

-Brandon

On Sep 24, 2010, at 4:50 PM, Brandon Casey wrote:

I hope you're just abandoning the tests you were creating, and _not_
abandoning the search for a fix.  The "solution" I offered is flawed
and breaks some of the other tests. :)

Actually, I think the solution you offered exposed what could be considered a bug in git-rev-parse.  The fact that it worked before was just an happy accident, I think...

$ ARGS="-q --index stash@{0}"
$ # Get only the revision arguments
$ git rev-parse --no-flags --symbolic $ARGS
stash@{0}
$ # What git-stash currently uses to get flags
$ git rev-parse --no-revs -- $ARGS
--
-q
--index
stash@{0}
$ # That was a lot more than just the flags
$ # What git-stash "should" use to get flags
$ git rev-parse --no-revs --flags $ARGS
--index
$ # Huh, it ate -q, let's try --
$ git rev-parse --no-revs --flags -- $ARGS
$ # No, that's not right either...

git-stash's current code "FLAGS=$(git rev-parse --no-revs -- "$@")" simply returns all of the arguments including a starting --.  The issue is that git-rev-parse eats a -q parameter.  There's no way to distinguish between arguments for rev-parse and arguments it's supposed to parse.  Generally this isn't an issue.

The simple way to deal with this is to check for -q before using rev-parse.  The better way is to either get rev-parse to stop eating the -q somehow or to switch git-stash to parseopts.

Simple patch coming soon.

~~ Brian

Currently git-stash uses `git rev-parse --no-revs -- "$@"` to set its
FLAGS variable.  This is the same as `FLAGS="-- $@"`.  It should use
`git rev-parse --no-revs --flags "$@"`, but that eats any "-q" or
"--quiet" argument.  So move the check for quiet before rev-parse.

Signed-off-by: Brian Gernhardt <redacted>
---

 Not the most elegant solution, but it works.

 I think we want to add the ability for git rev-parse to understand
 `git rev-parse --no-revs --flags -- "$@"`, but I'm not sure if that
 would break anything else and don't have the time to do it right now.

 git-stash.sh     |   15 +++++++++++----
 t/t3903-stash.sh |    8 ++++----
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 7ce818b..b44da41 100755
--- a/git-stash.sh
+++ b/git-stash.sh

@@ -264,8 +264,18 @@ parse_flags_and_rev()
 	b_tree=
 	i_tree=
 
+	# Work around rev-parse --flags eating -q
+	for opt
+	do
+		case "$opt" in
+			-q|--quiet)
+				GIT_QUIET=t
+			;;
+		esac
+	done
+
 	REV=$(git rev-parse --no-flags --symbolic "$@" 2>/dev/null)
-	FLAGS=$(git rev-parse --no-revs -- "$@" 2>/dev/null)
+	FLAGS=$(git rev-parse --no-revs --flags "$@" 2>/dev/null)
 
 	set -- $FLAGS
 

@@ -273,9 +283,6 @@ parse_flags_and_rev()
 	while test $# -ne 0
 	do
 		case "$1" in
-			-q|--quiet)
-				GIT_QUIET=-t
-			;;
 			--index)
 				INDEX_OPTION=--index
 			;;

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index e8a7338..9ed2396 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh

@@ -406,7 +406,7 @@ test_expect_success 'stash branch - stashes on stack, stash-like argument' '
 	test $(git ls-files --modified | wc -l) -eq 1
 '
 
-test_expect_failure 'stash show - stashes on stack, stash-like argument' '
+test_expect_success 'stash show - stashes on stack, stash-like argument' '
 	git stash clear &&
 	test_when_finished "git reset --hard HEAD" &&
 	git reset --hard &&

@@ -424,7 +424,7 @@ test_expect_failure 'stash show - stashes on stack, stash-like argument' '
 	test_cmp expected actual
 '
 
-test_expect_failure 'stash show -p - stashes on stack, stash-like argument' '
+test_expect_success 'stash show -p - stashes on stack, stash-like argument' '
 	git stash clear &&
 	test_when_finished "git reset --hard HEAD" &&
 	git reset --hard &&

@@ -447,7 +447,7 @@ test_expect_failure 'stash show -p - stashes on stack, stash-like argument' '
 	test_cmp expected actual
 '
 
-test_expect_failure 'stash show - no stashes on stack, stash-like argument' '
+test_expect_success 'stash show - no stashes on stack, stash-like argument' '
 	git stash clear &&
 	test_when_finished "git reset --hard HEAD" &&
 	git reset --hard &&

@@ -462,7 +462,7 @@ test_expect_failure 'stash show - no stashes on stack, stash-like argument' '
 	test_cmp expected actual
 '
 
-test_expect_failure 'stash show -p - no stashes on stack, stash-like argument' '
+test_expect_success 'stash show -p - no stashes on stack, stash-like argument' '
 	git stash clear &&
 	test_when_finished "git reset --hard HEAD" &&
 	git reset --hard &&

-- 
1.7.3.234.g7bba3

Currently git-stash uses `git rev-parse --no-revs -- "$@"` to set its
FLAGS variable.  This is the same as `FLAGS="-- $@"`.  It should use
`git rev-parse --no-revs --flags "$@"`, but that eats any "-q" or
"--quiet" argument.  So move the check for quiet before rev-parse.

Signed-off-by: Brian Gernhardt <redacted>
---

 Not the most elegant solution, but it works.

 I think we want to add the ability for git rev-parse to understand
 `git rev-parse --no-revs --flags -- "$@"`, but I'm not sure if that
 would break anything else and don't have the time to do it right now.

 (This time with the right CC list.)

 git-stash.sh     |   15 +++++++++++----
 t/t3903-stash.sh |    8 ++++----
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 7ce818b..b44da41 100755
--- a/git-stash.sh
+++ b/git-stash.sh

@@ -264,8 +264,18 @@ parse_flags_and_rev()
 	b_tree=
 	i_tree=
 
+	# Work around rev-parse --flags eating -q
+	for opt
+	do
+		case "$opt" in
+			-q|--quiet)
+				GIT_QUIET=t
+			;;
+		esac
+	done
+
 	REV=$(git rev-parse --no-flags --symbolic "$@" 2>/dev/null)
-	FLAGS=$(git rev-parse --no-revs -- "$@" 2>/dev/null)
+	FLAGS=$(git rev-parse --no-revs --flags "$@" 2>/dev/null)
 
 	set -- $FLAGS
 

@@ -273,9 +283,6 @@ parse_flags_and_rev()
 	while test $# -ne 0
 	do
 		case "$1" in
-			-q|--quiet)
-				GIT_QUIET=-t
-			;;
 			--index)
 				INDEX_OPTION=--index
 			;;

diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh
index e8a7338..9ed2396 100755
--- a/t/t3903-stash.sh
+++ b/t/t3903-stash.sh

@@ -406,7 +406,7 @@ test_expect_success 'stash branch - stashes on stack, stash-like argument' '
 	test $(git ls-files --modified | wc -l) -eq 1
 '
 
-test_expect_failure 'stash show - stashes on stack, stash-like argument' '
+test_expect_success 'stash show - stashes on stack, stash-like argument' '
 	git stash clear &&
 	test_when_finished "git reset --hard HEAD" &&
 	git reset --hard &&

@@ -424,7 +424,7 @@ test_expect_failure 'stash show - stashes on stack, stash-like argument' '
 	test_cmp expected actual
 '
 
-test_expect_failure 'stash show -p - stashes on stack, stash-like argument' '
+test_expect_success 'stash show -p - stashes on stack, stash-like argument' '
 	git stash clear &&
 	test_when_finished "git reset --hard HEAD" &&
 	git reset --hard &&

@@ -447,7 +447,7 @@ test_expect_failure 'stash show -p - stashes on stack, stash-like argument' '
 	test_cmp expected actual
 '
 
-test_expect_failure 'stash show - no stashes on stack, stash-like argument' '
+test_expect_success 'stash show - no stashes on stack, stash-like argument' '
 	git stash clear &&
 	test_when_finished "git reset --hard HEAD" &&
 	git reset --hard &&

@@ -462,7 +462,7 @@ test_expect_failure 'stash show - no stashes on stack, stash-like argument' '
 	test_cmp expected actual
 '
 
-test_expect_failure 'stash show -p - no stashes on stack, stash-like argument' '
+test_expect_success 'stash show -p - no stashes on stack, stash-like argument' '
 	git stash clear &&
 	test_when_finished "git reset --hard HEAD" &&
 	git reset --hard &&

-- 
1.7.3.234.g7bba3

The detached-stash series regressed support for
   git stash show stash@{0}

due to a faulty assumption that
git rev-parse --no-revs stash@{0} would treat
stash@{0} as a revision reference and thus
not display it.

This patch restores the behaviour of git stash show
so that only flag like options are assigned
to the FLAGS variable and thus does not
depend git rev-parse behaviour so strongly.

It has been tested with Brandon Casey's improved t3903 tests.

Signed-off-by: Jon Seymour <redacted>
---
 Apologies for the breakage.
 Brandon: thanks for improving the tests.

 git-stash.sh |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-stash.sh b/git-stash.sh
index 7ce818b..c2f1b2a 100755
--- a/git-stash.sh
+++ b/git-stash.sh

@@ -282,7 +282,7 @@ parse_flags_and_rev()
 			--)
 				:
 			;;
-			*)
+			-*)
 				FLAGS="${FLAGS}${FLAGS:+ }$1"
 			;;
 		esac

-- 
1.7.2.12.g8788e.dirty

Thanks Brian. I've submitted what I hope is the simplest possible
rectification of my regression - apologies for not catching this
myself!

jon.

On Sat, Sep 25, 2010 at 8:15 AM, Brian Gernhardt
[off-list ref] wrote:

The detached-stash series regressed support for
   git stash show stash@{0}

due to a faulty assumption that:
   git rev-parse --no-revs -- stash@{0}

would treat stash@{0} as a revision reference and
thus not output it.

This patch restores the behaviour of git stash show
so that git rev-parse is not used for parsing flags
and only flag like options are assigned to
the FLAGS variable.

It has been tested with Brandon Casey's improved t3903 tests.

Signed-off-by: Jon Seymour <redacted>
---
 git-stash.sh |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

This revision further simplifies the parsing code
by removing use of git rev-parse for FLAGS parsing
altogether.

diff --git a/git-stash.sh b/git-stash.sh
index 7ce818b..8b18bb5 100755
--- a/git-stash.sh
+++ b/git-stash.sh

@@ -265,9 +265,6 @@ parse_flags_and_rev()
 	i_tree=
 
 	REV=$(git rev-parse --no-flags --symbolic "$@" 2>/dev/null)
-	FLAGS=$(git rev-parse --no-revs -- "$@" 2>/dev/null)
-
-	set -- $FLAGS
 
 	FLAGS=
 	while test $# -ne 0

@@ -282,7 +279,7 @@ parse_flags_and_rev()
 			--)
 				:
 			;;
-			*)
+			-*)
 				FLAGS="${FLAGS}${FLAGS:+ }$1"
 			;;
 		esac

-- 
1.7.2.14.g132f5.dirty

On Sep 24, 2010, at 11:32 PM, Jon Seymour wrote:

due to a faulty assumption that:
  git rev-parse --no-revs -- stash@{0}

This assumption is faulty, it should be "git rev-parse --no-revs --flags stash@{0}", which works properly for all revision arguments and flags _except_ -q and --quiet.

This revision further simplifies the parsing code
by removing use of git rev-parse for FLAGS parsing
altogether.

That is simpler, and does fix this specific issue.  However, I would strongly argue that "git rev-parse --no-revs --flags" is broken.  I really don't have the time tonight or probably this weekend to work on it, but git-rev-parse should only take "-q" and "--quiet" for itself if "--verify" was passed.  (Since that is the only mode in which rev-parse uses quiet, AFAIK.)

Possibly rev-parse should also (or instead) separate "arguments for rev-parse" and "arguments rev-parse is parsing" using the standard "--".  I don't know if this will affect any current users.

~~ Brian

On Sat, Sep 25, 2010 at 2:45 PM, Brian Gernhardt
[off-list ref] wrote:

On Sep 24, 2010, at 11:32 PM, Jon Seymour wrote:

quoted

due to a faulty assumption that:
  git rev-parse --no-revs -- stash@{0}

This assumption is faulty, it should be "git rev-parse --no-revs --flags stash@{0}", which works properly for all revision arguments and flags _except_ -q and --quiet.

Agreed. If I recall the evolution of the code, I originally had
--no-revs --flags, but then found -q was being eaten. The only way to
prevent it being eaten was to remove --flags and add --, which "fixed"
the -q problem but created the git stash show xxx problem. Of course,
had my unit tests been more thorough, I would have picked this, but
alas they were not.

quoted

This revision further simplifies the parsing code
by removing use of git rev-parse for FLAGS parsing
altogether.

That is simpler, and does fix this specific issue.  However, I would strongly argue that "git rev-parse --no-revs --flags" is broken.  I really don't have the time tonight or probably this weekend to work on it, but git-rev-parse should only take "-q" and "--quiet" for itself if "--verify" was passed.  (Since that is the only mode in which rev-parse uses quiet, AFAIK.)

This seems like a reasonable compromise to me.

Possibly rev-parse should also (or instead) separate "arguments for rev-parse" and "arguments rev-parse is parsing" using the standard "--".  I don't know if this will affect any current users.

I suspect this could be problematic.

Current behaviour:

   $ git rev-parse --flags -X -- Y -Z
   -X

Here -- is being used to say, don't interpret anything after -- as
being flag-like even if it could be flag-like.

So, if a command foo uses git rev-parse to parse its own arguments:

    foo -X -- Y -Z

then I suspect, it would only want to treat -X as being flag-like,
from the user of foo's point of view, both Y and -Z are not meant to
be interpreted by foo.

It might make sense for --flags to be an instruction to git rev-parse
not to interpret any subsequent arguments as git rev-parse options so
that:

  $ git rev-parse --flags -q --no-flags -- --revs-only

would output:

  -q --no-flags

That is, prevent -q being eaten by git rev-parse, but preserve
existing interpretation of --.

~~ Brian

| Added Brandon to distribution - apologies for earlier omission.

jon.

On Sat, Sep 25, 2010 at 1:32 PM, Jon Seymour [off-list ref] wrote:

Current git rev-parse behaviour makes --flags hard to use if the remaining
arguments to git rev-parse contain an option that would otherwise be interpreted
as an option by git rev-parse itself.

So, for example:

  $> git rev-parse --flags -q -X
  -X

Normally one might expect to use -- to prevent -q being interpreted:

  $> git rev-parse --flags -- -q -X
  -q -X

But we can't really use -- in this way, because commands that use
git rev-parse might reasonably expect:

  $> git rev-parse --flags -Y -- -q -X
  -Y

That is, -Y to be regarded as a flag but everything after -- to be uninterpreted.

This proposed change modifies git rev-parse so that git rev-parse stops
interpreting flag arguments as options to git rev-parse once --flags is
interpreted. We also exit early once -- is found.

Tests will follow in subsequent iterations of this patch, if the consensus
is that the approach is correct.

Signed-off-by: Jon Seymour <redacted>
---
 builtin/rev-parse.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index a5a1c86..9e340c7 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c

@@ -497,8 +497,15 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				/* Pass on the "--" if we show anything but files.. */
 				if (filter & (DO_FLAGS | DO_REVS))
 					show_file(arg);
+				if (!(filter & DO_NONFLAGS)) {
+					return 0;
+				}
 				continue;
 			}
+			if (!(filter & DO_NONFLAGS)) {
+				 show_flag(arg);
+				 continue;
+			}
 			if (!strcmp(arg, "--default")) {
 				def = argv[i+1];
 				i++;

-- 
1.7.2.14.gbde03.dirty

On Sat, Sep 25, 2010 at 5:19 PM, Jon Seymour [off-list ref] wrote:

Current git rev-parse behaviour makes --flags hard to use if the remaining
...

Note that I haven't run the test suite yet to see if it breaks
anything. I'd also
expect to write additional tests and update the documentation.

jon.

On Sat, Sep 25, 2010 at 5:19 PM, Jon Seymour [off-list ref] wrote:

On Sat, Sep 25, 2010 at 5:19 PM, Jon Seymour [off-list ref] wrote:

quoted

Current git rev-parse behaviour makes --flags hard to use if the remaining
...

Note that I haven't run the test suite yet to see if it breaks
anything. I'd also
expect to write additional tests and update the documentation.

jon.

Mmmm...almost certainly not going to regress anything, since there
does not seem to be a test or script that uses --flags. Ahem.

jon.

This patch has been superceded by:

    [PATCH] stash: simplify parsing fixes

which applies on the Brian's fix that has been applied to maint.

On Sat, Sep 25, 2010 at 1:32 PM, Jon Seymour [off-list ref] wrote: