Re: [PATCH v2] blame: prevent error if range ends past end of file

From: SZEDER Gábor <szeder.dev@gmail.com>
Date: Oct 26, 2017, 15:31 UTC
Message-ID: <20171026153111.21813-1-szeder.dev@gmail.com>
In-Reply-To: <20171026070108.4185-1-istephens@atlassian.com>

> If the -L option is used to specify a line range in git blame, and the
> end of the range is past the end of the file, at present git will fail
> with a fatal error. This commit prevents such behaviour - instead the
> blame is display for any existing lines within the specified range.

s/is display/is displayed/ ?

'git log' has a very similar -L option, which errors out, too, if the end of the line range is past the end of the file. IMHO the interpretation of the line range -L<start>,<end> should be kept consistent in the two commands, and 'git log' shouldn't error out, either.

Show 39 quoted lines

> Signed-off-by: Isabella Stephens <istephens@atlassian.com>
> ---
>  builtin/blame.c               | 4 ++--
>  t/t8003-blame-corner-cases.sh | 5 +++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/builtin/blame.c b/builtin/blame.c
> index 67adaef4d..b5b9db147 100644
> --- a/builtin/blame.c
> +++ b/builtin/blame.c
> @@ -878,13 +878,13 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
>  				    nth_line_cb, &sb, lno, anchor,
>  				    &bottom, &top, sb.path))
>  			usage(blame_usage);
> -		if (lno < top || ((lno || bottom) && lno < bottom))
> +		if ((lno || bottom) && lno < bottom)
>  			die(Q_("file %s has only %lu line",
>  			       "file %s has only %lu lines",
>  			       lno), path, lno);
>  		if (bottom < 1)
>  			bottom = 1;
> -		if (top < 1)
> +		if (top < 1 || lno < top)
>  			top = lno;
>  		bottom--;
>  		range_set_append_unsafe(&ranges, bottom, top);
> diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
> index 661f9d430..32b3788fe 100755
> --- a/t/t8003-blame-corner-cases.sh
> +++ b/t/t8003-blame-corner-cases.sh
> @@ -216,8 +216,9 @@ test_expect_success 'blame -L with invalid start' '
>  '
>  
>  test_expect_success 'blame -L with invalid end' '
> -	test_must_fail git blame -L1,5 tres 2>errors &&
> -	test_i18ngrep "has only 2 lines" errors
> +	git blame -L1,5 tres >out &&
> +	cat out &&
> +	test $(wc -l < out) -eq 2

Please use the test_line_count helper instead, as it will output a helpful error message on failure, making the 'cat out' above unnecessary.

Show 7 quoted lines

>  '
>  
>  test_expect_success 'blame parses <end> part of -L' '
> -- 
> 2.14.1
> 
>