Re: [PATCH v4 1/2] blame: prevent error if range ends past end of file

[Isabella Stephens <istephens@atlassian.com>]

My intention was to modify existing behaviour as little as possible,
but I agree clipping to the first line makes a lot more sense. That
raises the question though, do we clip to 1 and treat -L,-n as a valid
input, or clip to -1 so that this case be detected?

On 27/4/18 10:50 am, Junio C Hamano wrote:
> istephens@atlassian.com writes:
> 
>> diff --git a/line-range.c b/line-range.c
>> index 323399d16..023aee1f5 100644
>> --- a/line-range.c
>> +++ b/line-range.c
>> @@ -47,7 +47,7 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
>>  			else if (!num)
>>  				*ret = begin;
>>  			else
>> -				*ret = begin + num;
>> +				*ret = begin + num ? begin + num : -1;
> 
> When parsing "-L<something>,-20" to grab some lines before the line
> specified by <something>, if that something happens to be line #20,
> this gives -1 to *ret.  If it is line #19, *ret becomes -1, and if
> it is line #18 or before, *ret becomes -2, -3, ...
> 
> Is that what we really want here?  It is disturbing that only line
> #19 and #20 are treated identically in the above example.  If it
> were "if going backwards by -num lines from begin goes beyond the
> beginning of the file, clip it to the first line", I would
> understand it, but as written, I am not sure what the code is trying
> to do.
>