Re: [PATCH] t/t3903-stash: improve testing of git-stash show

From: Brian Gernhardt <brian@gernhardtsoftware.com>
Date: Sep 24, 2010, 21:49 UTC
Message-ID: <953B8952-8928-43B3-A05D-6AEDC5D4B565@gernhardtsoftware.com>
In-Reply-To: <UNSmPBO95_JSGKkZEyMl028XH9fmWCis39YCV_49wZs7FghyXRqw2Q@cipher.nrlssc.navy.mil>

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