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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 27, 2018, 00:50 UTC
Message-ID: <xmqq7eot1nhl.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180426074502.78318-2-istephens@atlassian.com>

istephens@atlassian.com writes:

Show 10 quoted lines

> 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.