Re: [PATCH] Add new @ shortcut for HEAD

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

 



On Tue, Apr 30, 2013 at 1:17 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
> On Tue, Apr 30, 2013 at 1:10 PM, Felipe Contreras
> <felipe.contreras@xxxxxxxxx> wrote:
>> On Tue, Apr 30, 2013 at 12:54 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
>>> On Tue, Apr 30, 2013 at 2:35 AM, Felipe Contreras
>>> <felipe.contreras@xxxxxxxxx> wrote:
>>>> So we can type '@' instead of 'HEAD@', or rather 'HEAD'. So now we can
>>>> use 'git show @~1', and all that goody goodness.
>>>
>>> I like this. I haven't spent a lot of time on thinking about
>>> ambiguation. But I think we're safe there. '@' is not overloaded much
>>> like ':', '^' or '~'.
>>>
>>>> This patch allows 'HEAD@' to be the same as 'HEAD@{0}', and similarly with
>>>> 'master@'.
>>>
>>> I'm a bit reluctant to this. It looks like incomplete syntax to me as
>>> '@' has always been followed by '{'. Can we have the lone '@' candy
>>> but reject master@ and HEAD@? There's no actual gain in writing
>>> master@ vs master@{0}.
>>
>> That's what I tried first, but it just didn't feel elegant to have one
>> check for this case only. foo@ does follow naturally, and it doesn't
>> hurt.
>>
>>>> +'@'::
>>>> +  '@' alone is a shortcut for 'HEAD'
>>>> +
>>>
>>> And this does not explain about HEAD@ or master@. But because I prefer
>>> the candy part only. This documentation part looks good :)
>>
>> Yeah, there's no point in documenting things that are not useful for
>> the user. The fact that HEAD@ is translated to HEAD is just an
>> implementation detail.
>
> Exactly. As it's implementation detail, it should not be exposed to
> user as "huh?" moments when they type "HEAD@". I'm may be paranoid,
> but if some user finds it nice (or just different) to try master@ in
> scripts, then we change implementation details and master@ no longer
> works, people could be upset. Undefined behavior syntax should be kept
> to minimum. And to answer your other mail regarding the harmlessness
> of @{-1}@, I'd rather pay some extra code than leave some loose ends
> like that.

I don't see it as a loose end.

--- a/sha1_name.c
+++ b/sha1_name.c
@@ -443,8 +443,11 @@ static int get_sha1_basic(const char *str, int
len, unsigned char *sha1)
                return 0;

        empty_at = len && str[len-1] == '@';
-       if (empty_at)
+       if (empty_at) {
+               reflog_len = 0;
                len = len-1;
+               goto next;
+       }

        /* basic@{time or number or -number} format to query ref-log */
        reflog_len = at = 0;
@@ -460,6 +463,7 @@ static int get_sha1_basic(const char *str, int
len, unsigned char *sha1)
                }
        }

+next:

-- 
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]