Junio C Hamano <[email protected]> writes:
> Thomas Rast <[email protected]> writes:
>
>> From: Bo Yang <[email protected]>
>>
>> We want to use the same style of -L n,m argument for 'git log -L' as
>> for git-blame. Refactor the argument parsing of the range arguments
>> from builtin/blame.c to the (new) file that will hold the 'git log -L'
>> logic.
>>
>> To accommodate different data structures in blame and log -L, the file
>> contents are abstracted away; parse_range_arg takes a callback that it
>> uses to get the contents of a line of the (notional) file.
>>
>> The new test is for a case that made me pause during debugging: the
>> 'blame -L with invalid end' test was the only one that noticed an
>> outright failure to parse the end *at all*. So make a more explicit
>> test for that.
>>
>> Signed-off-by: Bo Yang <[email protected]>
>> Signed-off-by: Thomas Rast <[email protected]>
>> ---
>> Documentation/blame-options.txt | 19 +------
>> Documentation/line-range-format.txt | 18 +++++++
>> Makefile | 2 +
>> builtin/blame.c | 99 +++-------------------------------
>> line-log.c | 105
>> ++++++++++++++++++++++++++++++++++++
>> line-log.h | 23 ++++++++
>
> Was this churn necessary?
>
> It is strange to move existing functions that will be tweaked to be
> shared by two different codepaths (blame and line-log) to the new
> user.
>
> The only effect this has, as opposed to tweaking the functions in
> place and making them extern, is to make it harder to see the tweaks
> made while moving the lines, and also make it more cumbersome to
> determine the lineage of the code later.
>
> It would have been understandable if they were moved to a new
> library-ish file (perhaps "line-range.[ch]"); even though that
> approach shares the same downsides, at least it would have a better
> excuse "We will share this, so let's move it to a neutral third
> place to allow us to hide the implementation details from both
> users". The arrangement this patch series makes does not even have
> that excuse. The final implementation still stay with one of the
> users; the only difference is that it is away from the original user
> and close to the new user.
Even though I am moving from builtin/blame.c to line-log.c? I would
otherwise have to call from a rather lib-ish file into a "frontend"
file. I always figured I wasn't supposed to do that.
--
Thomas Rast
trast@{inf,student}.ethz.ch
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
