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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]