Re: [PATCH 1/5] t1508 (at-combinations): more tests; document failures

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: May 2, 2013, 02:22 UTC
Message-ID: <CAMP44s31jyuGGG1Wn9D=D9udkJJdXQfwVAnHuOg02FWinpmn7Q@mail.gmail.com>
In-Reply-To: <7va9oe5y6k.fsf@alter.siamese.dyndns.org>

On Wed, May 1, 2013 at 1:53 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 9 quoted lines

> As you and Felipe seem to be aiming for the same "Let's allow users
> to say '@' when they mean HEAD", I'll let you two figure the best
> approach out.
>
> One productive way forward might be to come up with a common test
> script pieces to document what constructs that spell @ in place of
> HEAD should be supported, and much more importantly, what constructs
> that happen to have @ in them should not mistakenly trigger the new
> machinery.

The difference is basically that my approach is less intrusive; it only touches code that is relevant for revision parsing. This means the scope and side-effects are known; the same as @{-N}.

Ramkumar's approach has no precedent, so it's not clear what side-effects could there be. His approach relies on modifying branch_get() a function that barely has been touched since 2007, and has other side-effects, plus there isn't even a proper patch with the isolated change, and an explanation why it makes sense.

While it might or might not make sense to teach branch_get() to accept symbolic refs, it's not needed to achieve the desired functionality, either on my approach, or his.

Moreover, the symbolic-ref 'HEAD' is quite special, it's mentioned everywhere in the documentation, and the code has special cases for it. It's not reasonable to expect all relevant places to be updated for this functionality, and certainly 'Documentation/revisions.txt' is not the only one.

For example, in Ramkumar's approach:

 % git branch -u master @

Would replace '@' with HEAD, however:

 % git branch --edit-description @

Would not. These inconsistencies are not due to Ramkumar's code, but why would the user expect '@' to be replaced with anything if 'git branch' documentation doesn't mention any revision parsing, which is the only place where the '@' shortcut is documented.

In my opinion, if 'git branch X @{-1}' doesn't work, neither should 'git branch X @', and that's what my approach does.

My approach is isolated to revision parsing, which means we minimize the potential for surprises and unintended consequences.

Cheers.

-- 
Felipe Contreras