line-range: reject -L line numbers less than 1

sunshineco · gitster · commit 5ce922a014f7 · 2013-08-06T14:48:55.000-07:00
Since inception, git-blame -L has been documented as accepting 1-based
line numbers. When handed a line number less than 1, -L's behavior is
undocumented and undefined; it's also nonsensical and should be
diagnosed as an error. Do so.

Signed-off-by: Eric Sunshine &lt;sunshine@sunshineco.com&gt;
Signed-off-by: Junio C Hamano &lt;gitster@pobox.com&gt;
diff --git a/line-range.c b/line-range.c
@@ -54,8 +54,11 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,
 	}
 	num = strtol(spec, &term, 10);
 	if (term != spec) {
-		if (ret)
+		if (ret) {
+			if (num <= 0)
+				die("-L invalid line number: %ld", num);
 			*ret = num;
+		}
 		return term;
 	}
 
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
@@ -185,15 +185,15 @@ test_expect_success 'blame -L Y,X (undocumented)' '
 	check_count -L6,3 B 1 B1 1 B2 1 D 1
 '
 
-test_expect_failure 'blame -L -X' '
+test_expect_success 'blame -L -X' '
 	test_must_fail $PROG -L-1 file
 '
 
-test_expect_failure 'blame -L 0' '
+test_expect_success 'blame -L 0' '
 	test_must_fail $PROG -L0 file
 '
 
-test_expect_failure 'blame -L ,0' '
+test_expect_success 'blame -L ,0' '
 	test_must_fail $PROG -L,0 file
 '
 
@@ -447,8 +447,8 @@ test_expect_success 'blame empty' '
 	check_count -h HEAD^^ -f incremental
 '
 
-test_expect_success 'blame -L 0 empty (undocumented)' '
-	check_count -h HEAD^^ -f incremental -L0
+test_expect_success 'blame -L 0 empty' '
+	test_must_fail $PROG -L0 incremental HEAD^^
 '
 
 test_expect_success 'blame -L 1 empty' '
@@ -463,8 +463,8 @@ test_expect_success 'blame half' '
 	check_count -h HEAD^ -f incremental I 1
 '
 
-test_expect_success 'blame -L 0 half (undocumented)' '
-	check_count -h HEAD^ -f incremental -L0 I 1
+test_expect_success 'blame -L 0 half' '
+	test_must_fail $PROG -L0 incremental HEAD^
 '
 
 test_expect_success 'blame -L 1 half' '
@@ -483,8 +483,8 @@ test_expect_success 'blame full' '
 	check_count -f incremental I 1
 '
 
-test_expect_success 'blame -L 0 full (undocumented)' '
-	check_count -f incremental -L0 I 1
+test_expect_success 'blame -L 0 full' '
+	test_must_fail $PROG -L0 incremental
 '
 
 test_expect_success 'blame -L 1 full' '

Original file line number	Diff line number	Diff line change
`@@ -54,8 +54,11 @@ static const char parse_loc(const char spec, nth_line_fn_t nth_line,`
`54`	`54`	`}`
`55`	`55`	`num = strtol(spec, &term, 10);`
`56`	`56`	`if (term != spec) {`
`57`		`- if (ret)`
	`57`	`+ if (ret) {`
	`58`	`+ if (num <= 0)`
	`59`	`+ die("-L invalid line number: %ld", num);`
`58`	`60`	`*ret = num;`
	`61`	`+ }`
`59`	`62`	`return term;`
`60`	`63`	`}`
`61`	`64`
Original file line number	Diff line number	Diff line change
`@@ -185,15 +185,15 @@ test_expect_success 'blame -L Y,X (undocumented)' '`
`185`	`185`	`check_count -L6,3 B 1 B1 1 B2 1 D 1`
`186`	`186`	`'`
`187`	`187`
`188`		`-test_expect_failure 'blame -L -X' '`
	`188`	`+test_expect_success 'blame -L -X' '`
`189`	`189`	`test_must_fail $PROG -L-1 file`
`190`	`190`	`'`
`191`	`191`
`192`		`-test_expect_failure 'blame -L 0' '`
	`192`	`+test_expect_success 'blame -L 0' '`
`193`	`193`	`test_must_fail $PROG -L0 file`
`194`	`194`	`'`
`195`	`195`
`196`		`-test_expect_failure 'blame -L ,0' '`
	`196`	`+test_expect_success 'blame -L ,0' '`
`197`	`197`	`test_must_fail $PROG -L,0 file`
`198`	`198`	`'`
`199`	`199`
`@@ -447,8 +447,8 @@ test_expect_success 'blame empty' '`
`447`	`447`	`check_count -h HEAD^^ -f incremental`
`448`	`448`	`'`
`449`	`449`
`450`		`-test_expect_success 'blame -L 0 empty (undocumented)' '`
`451`		`- check_count -h HEAD^^ -f incremental -L0`
	`450`	`+test_expect_success 'blame -L 0 empty' '`
	`451`	`+ test_must_fail $PROG -L0 incremental HEAD^^`
`452`	`452`	`'`
`453`	`453`
`454`	`454`	`test_expect_success 'blame -L 1 empty' '`
`@@ -463,8 +463,8 @@ test_expect_success 'blame half' '`
`463`	`463`	`check_count -h HEAD^ -f incremental I 1`
`464`	`464`	`'`
`465`	`465`
`466`		`-test_expect_success 'blame -L 0 half (undocumented)' '`
`467`		`- check_count -h HEAD^ -f incremental -L0 I 1`
	`466`	`+test_expect_success 'blame -L 0 half' '`
	`467`	`+ test_must_fail $PROG -L0 incremental HEAD^`
`468`	`468`	`'`
`469`	`469`
`470`	`470`	`test_expect_success 'blame -L 1 half' '`
`@@ -483,8 +483,8 @@ test_expect_success 'blame full' '`
`483`	`483`	`check_count -f incremental I 1`
`484`	`484`	`'`
`485`	`485`
`486`		`-test_expect_success 'blame -L 0 full (undocumented)' '`
`487`		`- check_count -f incremental -L0 I 1`
	`486`	`+test_expect_success 'blame -L 0 full' '`
	`487`	`+ test_must_fail $PROG -L0 incremental`
`488`	`488`	`'`
`489`	`489`
`490`	`490`	`test_expect_success 'blame -L 1 full' '`