Junio C Hamano <gits...@pobox.com> writes:

> Thomas Rast <tr...@student.ethz.ch> writes:
>> From: Bo Yang <struggleyb....@gmail.com>
>> 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 <struggleyb....@gmail.com>
>> Signed-off-by: Thomas Rast <tr...@student.ethz.ch>
>> ---
>>  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
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to