Re: [PATCH 1/2] t1506: more test for @{upstream} syntax
- From
Jeff King <peff@peff.net>
- Date
- Jan 27, 2010, 11:40 UTC
- Message-ID
- <20100127114009.GE6262@coredump.intra.peff.net>
- In-Reply-To
- <7vpr4wy2lk.fsf@alter.siamese.dyndns.org>
On Tue, Jan 26, 2010 at 01:32:07PM -0800, Junio C Hamano wrote:
> I don't know if it deals with things like "@{-1}@{u}@{now}"; the users
> should have every right to expect it to, but I didn't consciously try to
> make that work with this patch.Nice. This also fixes "git log -g @{-1}". Static uses like "git show @{u}@{1.week.ago}" and "git show @{-1}@{1.week.ago}" were already fine, so I think the bug was really confined to the reflog walker (and your fix is therefore correct).
Using "git show @{-1}@{u}" is still broken, though.I tried tracing the parsing through get_sha1_basic and interpret_branch_name, but it's pretty confusing. Especially as we seem to deal with @{upstream}, @{now}, and @{-1} at different places.
I think the patch below does what we want, but the whole thing feels overly complicated to me, especially with the split of parsing @{...} between get_sha1_basic and interpret_branch_name. I guess we have spots that don't take reflogs but do take branch names, but I think the code would be much simpler if the syntax were parsed in one place, and then we threw out or complained about bogus semantics (like "checkout @{now}").
---
diff --git a/sha1_name.c b/sha1_name.c index ed4c028..ef8f3fa 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -881,8 +881,28 @@ int interpret_branch_name(const char *name, struct strbuf *buf) if (!len) return len; /* syntax Ok, not enough switches */ - if (0 < len) - return len; /* consumed from the front */ + if (0 < len && len == namelen) + return len; /* consumed all */ + else if (0 < len) { + /* we have extra data, which might need further processing */ + struct strbuf tmp = STRBUF_INIT; + int used = buf->len; + int ret; + + strbuf_add(buf, name + len, namelen - len); + ret = interpret_branch_name(buf->buf, &tmp); + /* that data was not interpreted, remove our cruft */ + if (ret < 0) { + strbuf_setlen(buf, used); + return len; + } + strbuf_reset(buf); + strbuf_addbuf(buf, &tmp); + strbuf_release(&tmp); + /* tweak for size of {-N} versus expanded ref name */ + return ret - used + len; + } + cp = strchr(name, '@'); if (!cp) return -1;