Re: [PATCH 4/4 v4] sha1_name.c: teach get_sha1_1 "-" shorthand for "@{-1}"

From: Siddharth Kannan <kannan.siddharth12@gmail.com>
Date: Feb 20, 2017, 14:21 UTC
Message-ID: <CAN-3QhoXBnLWyfuUsuvvRMYNnoupMrQHxE_G=ysyA_14KX4Yrw@mail.gmail.com>
In-Reply-To: <xmqq8tp6x8b6.fsf@gitster.mtv.corp.google.com>

On 17 February 2017 at 00:38, Junio C Hamano <gitster@pobox.com> wrote:

Show 5 quoted lines

> Having said all that, I do not think the remainder of the code is
> prepared to take "-", not yet anyway [*1*], so turning "-" into
> "@{-1}" this patch does before it calls get_sha1_basic(), while it
> is not an ideal final state, is probably an acceptable milestone to
> stop at.

So, is it okay to stop with just supporting "-" and not support things like "-@{yesterday}"?

Matthieu's comments on the matter:

    Siddharth Kannan <kannan.siddharth12@gmail.com> writes:

    > As per Matthieu's comments, I have updated the tests, but there
is still one
    > thing that is not working: log -@{yesterday} or log -@{2.days.ago}

    Note that I did not request that these things work, just that they seem
    to be relevant tests: IMHO it's OK to reject them, but for example we
    don't want them to segfault. And having a test is a good hint that you
    thought about what could happen and to document it.

[Quoted from email <vpqa89mnl4z.fsf@anie.imag.fr>]

Show 5 quoted lines

>
> It is a separate matter if this patch is sufficient to produce
> correct results, though.  I haven't studied the callers of this
> change to make sure yet, and may find bugs in this approach later.
>

-- 
Best Regards,

- Siddharth Kannan.