Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx> writes:
>> > My POV is that if it's easy to use the same function (and so the same
>> > set of range descriptors) for git log and git range-diff then do so.
>> > This yields a consistent behaviour which is IMHO better than preventing
>> > people to do things that are considered strange today.
>>
>> ... I am OK with that point of view. It certainly is simpler to
>> explain to end users.
>
> It seems you understood my argument :-)
So it seems ;-).
>> Having said that, it would make it much harder to implement
>> efficiently, though. For example, when your user says
>>
>> git range-diff A B
>>
>> to compare "git log A" (all the way down to the root) and "git log
>> B" (likewise), you'd certainly optimize the older common part of the
>> history out, essentially turning it into
>>
>> git range-diff A..B B..A
>>
>> or its moral equivalent
>>
>> git range-diff A...B
>>
>> But you cannot apply such an optimization blindly. When the user
>> gives A..B and B..A as two args, you somehow need to notice that
>> you shouldn't rewrite it to "A..B...B..A", and for that, you'd still
>> need some "parsing" of these args.
>
> I agree that for a long history
>
> git range-diff A B
>
> is an expensive request and I wouldn't invest too many cycles optimizing
> it.
Well, your devil's advocate can argue that accepting an input that
can easily make the tool unusable would be irresponsible, though.
And there are two possible ways out:
(1) declare that optimizing "A B" into "A...B" is way too difficult
to do in general, and find a good enough way to see if A and B
individually gives a "range" that should be manageable; or
(2) invest cycles to optimize, so your users do not have to care.
I think the series takes the former approach, and I find it
understandable.
It is a different matter if the way found and implemented in the
patch is "good enough" to tell if a given argument names a
manageable range. You said something about "rev^{/^here are
two..dots}" that can be mistaken as a "good enough" range, but it
actually names a revision and all the way down to the root.
> (And if I'd optimize it, it wouldn't be done using textual
> combination of the two strings but by checking if the two ranges
> intersect.
Yup, that is in line with what I mumbled earlier about
setup_revisions() and inspecting the rev_info.pending, I think.
>> So, I dunno. Limiting the second form to only forms that the
>> implementation does not have to do such optimization would certainly
>> make it simpler for Dscho to implement ;-)
>
> I don't want to make it more complicated for Dscho, I'm happy if I can
> in the near future use range-diff with $rev1^! $ref2^! . So feel free to
> ignore me.
>
> Best regards
> Uwe
