{"thread":{"id":"48612","subject":"[PATCH v6 1/2] blame: prevent error if range ends past end of file","startedAt":"2018-05-31T05:02:18Z","lastAt":"2018-06-15T18:28:10Z","messageCount":8,"participants":["istephens@atlassian.com","Junio C Hamano","Isabella Stephens"],"isPatch":true,"patchVersion":6,"patchTotal":2},"messages":[{"id":"348861","messageId":"20180531050030.43364-1-istephens@atlassian.com","threadId":"48612","inReplyTo":null,"subject":"[PATCH v6 1/2] blame: prevent error if range ends past end of file","fromName":"","fromEmail":"istephens@atlassian.com","sentAt":"2018-05-31T05:00:29Z","receivedAt":"2018-05-31T05:02:18Z","isPatch":true,"sender":{"key":"istephens@atlassian.com","avatar":"https://avatars.githubusercontent.com/u/30997546?v=4"},"body":"From: Isabella Stephens <istephens@atlassian.com>\n\nIf the -L option is used to specify a line range in git blame, and the\nend of the range is past the end of the file, git will fail with a fatal\nerror. This commit prevents such behavior - instead we display the blame\nfor existing lines within the specified range. Tests are amended\naccordingly.\n\nThis commit also fixes two corner cases. Blaming -L n,-(n+1) now blames\nthe first n lines of a file rather than from n to the end of the file.\nBlaming -L ,-n will be treated as -L 1,-n and blame the first line of\nthe file, rather than blaming the whole file.\n\nSigned-off-by: Isabella Stephens <istephens@atlassian.com>\n---\n builtin/blame.c               |  4 ++--\n line-range.c                  |  2 +-\n t/t8003-blame-corner-cases.sh | 12 ++++++++----\n 3 files changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 9dcb367b9..e1359b192 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -886,13 +886,13 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\t\t\t    nth_line_cb, &sb, lno, anchor,\n \t\t\t\t    &bottom, &top, sb.path))\n \t\t\tusage(blame_usage);\n-\t\tif (lno < top || ((lno || bottom) && lno < bottom))\n+\t\tif ((!lno && (top || bottom)) || lno < bottom)\n \t\t\tdie(Q_(\"file %s has only %lu line\",\n \t\t\t       \"file %s has only %lu lines\",\n \t\t\t       lno), path, lno);\n \t\tif (bottom < 1)\n \t\t\tbottom = 1;\n-\t\tif (top < 1)\n+\t\tif (top < 1 || lno < top)\n \t\t\ttop = lno;\n \t\tbottom--;\n \t\trange_set_append_unsafe(&ranges, bottom, top);\ndiff --git a/line-range.c b/line-range.c\nindex 323399d16..232c3909e 100644\n--- a/line-range.c\n+++ b/line-range.c\n@@ -47,7 +47,7 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,\n \t\t\telse if (!num)\n \t\t\t\t*ret = begin;\n \t\t\telse\n-\t\t\t\t*ret = begin + num;\n+\t\t\t\t*ret = begin + num > 0 ? begin + num : 1;\n \t\t\treturn term;\n \t\t}\n \t\treturn spec;\ndiff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh\nindex 661f9d430..c92a47b6d 100755\n--- a/t/t8003-blame-corner-cases.sh\n+++ b/t/t8003-blame-corner-cases.sh\n@@ -216,14 +216,18 @@ test_expect_success 'blame -L with invalid start' '\n '\n \n test_expect_success 'blame -L with invalid end' '\n-\ttest_must_fail git blame -L1,5 tres 2>errors &&\n-\ttest_i18ngrep \"has only 2 lines\" errors\n+\tgit blame -L1,5 tres >out &&\n+\ttest_line_count = 2 out\n '\n \n test_expect_success 'blame parses <end> part of -L' '\n \tgit blame -L1,1 tres >out &&\n-\tcat out &&\n-\ttest $(wc -l < out) -eq 1\n+\ttest_line_count = 1 out\n+'\n+\n+test_expect_success 'blame -Ln,-(n+1)' '\n+\tgit blame -L3,-4 nine_lines >out &&\n+\ttest_line_count = 3 out\n '\n \n test_expect_success 'indent of line numbers, nine lines' '\n-- \n2.14.3 (Apple Git-98)\n\n"},{"id":"348862","messageId":"20180531050030.43364-2-istephens@atlassian.com","threadId":"48612","inReplyTo":"20180531050030.43364-1-istephens@atlassian.com","subject":"[PATCH v6 2/2] log: prevent error if line range ends past end of file","fromName":"","fromEmail":"istephens@atlassian.com","sentAt":"2018-05-31T05:00:30Z","receivedAt":"2018-05-31T05:02:27Z","isPatch":true,"sender":{"key":"istephens@atlassian.com","avatar":"https://avatars.githubusercontent.com/u/30997546?v=4"},"body":"From: Isabella Stephens <istephens@atlassian.com>\n\nIf the -L option is used to specify a line range in git log, and the end\nof the range is past the end of the file, git will fail with a fatal\nerror. This commit prevents such behaviour - instead we perform the log\nfor existing lines within the specified range.\n\nThis commit also fixes a corner case where -L ,-n:file would be treated\nas a log over the whole file. Now we treat this as -L 1,-n:file and\nblame the first line of the file instead.\n\nSigned-off-by: Isabella Stephens <istephens@atlassian.com>\n---\n line-log.c          | 8 +++++---\n t/t4211-line-log.sh | 5 ++---\n 2 files changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/line-log.c b/line-log.c\nindex cdc2257db..4da40dec1 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -599,11 +599,13 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args)\n \t\t\t\t    lines, anchor, &begin, &end,\n \t\t\t\t    full_name))\n \t\t\tdie(\"malformed -L argument '%s'\", range_part);\n-\t\tif (lines < end || ((lines || begin) && lines < begin))\n-\t\t\tdie(\"file %s has only %lu lines\", name_part, lines);\n+\t\tif ((!lines && (begin || end)) || lines < begin)\n+\t\t\tdie(Q_(\"file %s has only %lu line\",\n+\t\t\t\t   \"file %s has only %lu lines\",\n+\t\t\t\t   lines), name_part, lines);\n \t\tif (begin < 1)\n \t\t\tbegin = 1;\n-\t\tif (end < 1)\n+\t\tif (end < 1 || lines < end)\n \t\t\tend = lines;\n \t\tbegin--;\n \t\tline_log_data_insert(&ranges, full_name, begin, end);\ndiff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh\nindex d0377fae5..436b13ad2 100755\n--- a/t/t4211-line-log.sh\n+++ b/t/t4211-line-log.sh\n@@ -60,7 +60,6 @@ test_bad_opts \"-L 1:nonexistent\" \"There is no path\"\n test_bad_opts \"-L 1:simple\" \"There is no path\"\n test_bad_opts \"-L '/foo:b.c'\" \"argument not .start,end:file\"\n test_bad_opts \"-L 1000:b.c\" \"has only.*lines\"\n-test_bad_opts \"-L 1,1000:b.c\" \"has only.*lines\"\n test_bad_opts \"-L :b.c\" \"argument not .start,end:file\"\n test_bad_opts \"-L :foo:b.c\" \"no match\"\n \n@@ -86,12 +85,12 @@ test_expect_success '-L ,Y (Y == nlines)' '\n \n test_expect_success '-L ,Y (Y == nlines + 1)' '\n \tn=$(expr $(wc -l <b.c) + 1) &&\n-\ttest_must_fail git log -L ,$n:b.c\n+\tgit log -L ,$n:b.c\n '\n \n test_expect_success '-L ,Y (Y == nlines + 2)' '\n \tn=$(expr $(wc -l <b.c) + 2) &&\n-\ttest_must_fail git log -L ,$n:b.c\n+\tgit log -L ,$n:b.c\n '\n \n test_expect_success '-L with --first-parent and a merge' '\n-- \n2.14.3 (Apple Git-98)\n\n"},{"id":"348953","messageId":"xmqqo9gvb8qt.fsf@gitster-ct.c.googlers.com","threadId":"48612","inReplyTo":"20180531050030.43364-2-istephens@atlassian.com","subject":"Re: [PATCH v6 2/2] log: prevent error if line range ends past end of file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-01T01:23:06Z","receivedAt":"2018-06-01T01:23:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"istephens@atlassian.com writes:\n\n> From: Isabella Stephens <istephens@atlassian.com>\n>\n> If the -L option is used to specify a line range in git log, and the end\n> of the range is past the end of the file, git will fail with a fatal\n> error. This commit prevents such behaviour - instead we perform the log\n> for existing lines within the specified range.\n>\n> This commit also fixes a corner case where -L ,-n:file would be treated\n> as a log over the whole file. Now we treat this as -L 1,-n:file and\n> blame the first line of the file instead.\n>\n> Signed-off-by: Isabella Stephens <istephens@atlassian.com>\n> ---\n>  line-log.c          | 8 +++++---\n>  t/t4211-line-log.sh | 5 ++---\n>  2 files changed, 7 insertions(+), 6 deletions(-)\n\nThanks.  Will queue.\n"},{"id":"348954","messageId":"xmqqh8mnb8j2.fsf@gitster-ct.c.googlers.com","threadId":"48612","inReplyTo":"20180531050030.43364-1-istephens@atlassian.com","subject":"Re: [PATCH v6 1/2] blame: prevent error if range ends past end of file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-01T01:27:45Z","receivedAt":"2018-06-01T01:27:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"istephens@atlassian.com writes:\n\n> From: Isabella Stephens <istephens@atlassian.com>\n>\n> If the -L option is used to specify a line range in git blame, and the\n> end of the range is past the end of the file, git will fail with a fatal\n> error. This commit prevents such behavior - instead we display the blame\n> for existing lines within the specified range. Tests are amended\n> accordingly.\n>\n> This commit also fixes two corner cases. Blaming -L n,-(n+1) now blames\n> the first n lines of a file rather than from n to the end of the file.\n> Blaming -L ,-n will be treated as -L 1,-n and blame the first line of\n> the file, rather than blaming the whole file.\n>\n> Signed-off-by: Isabella Stephens <istephens@atlassian.com>\n> ---\n>  builtin/blame.c               |  4 ++--\n>  line-range.c                  |  2 +-\n>  t/t8003-blame-corner-cases.sh | 12 ++++++++----\n>  3 files changed, 11 insertions(+), 7 deletions(-)\n\nDon't t800[12]-*.sh need adjustment for this change, too?\n"},{"id":"350234","messageId":"20180615062928.37165-1-istephens@atlassian.com","threadId":"48612","inReplyTo":"xmqqh8mnb8j2.fsf@gitster-ct.c.googlers.com","subject":"[PATCH v7 1/2] blame: prevent error if range ends past end of file","fromName":"","fromEmail":"istephens@atlassian.com","sentAt":"2018-06-15T06:29:27Z","receivedAt":"2018-06-15T06:29:53Z","isPatch":true,"sender":{"key":"istephens@atlassian.com","avatar":"https://avatars.githubusercontent.com/u/30997546?v=4"},"body":"From: Isabella Stephens <istephens@atlassian.com>\n\nIf the -L option is used to specify a line range in git blame, and the\nend of the range is past the end of the file, git will fail with a fatal\nerror. This commit prevents such behavior - instead we display the blame\nfor existing lines within the specified range. Tests are amended\naccordingly.\n\nThis commit also fixes two corner cases. Blaming -L n,-(n+1) now blames\nthe first n lines of a file rather than from n to the end of the file.\nBlaming -L ,-n will be treated as -L 1,-n and blame the first line of\nthe file, rather than blaming the whole file.\n\nSigned-off-by: Isabella Stephens <istephens@atlassian.com>\n---\n builtin/blame.c               |  4 ++--\n line-range.c                  |  2 +-\n t/annotate-tests.sh           |  4 ++--\n t/t8003-blame-corner-cases.sh | 12 ++++++++----\n 4 files changed, 13 insertions(+), 9 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 9dcb367b9..e1359b192 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -886,13 +886,13 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\t\t\t    nth_line_cb, &sb, lno, anchor,\n \t\t\t\t    &bottom, &top, sb.path))\n \t\t\tusage(blame_usage);\n-\t\tif (lno < top || ((lno || bottom) && lno < bottom))\n+\t\tif ((!lno && (top || bottom)) || lno < bottom)\n \t\t\tdie(Q_(\"file %s has only %lu line\",\n \t\t\t       \"file %s has only %lu lines\",\n \t\t\t       lno), path, lno);\n \t\tif (bottom < 1)\n \t\t\tbottom = 1;\n-\t\tif (top < 1)\n+\t\tif (top < 1 || lno < top)\n \t\t\ttop = lno;\n \t\tbottom--;\n \t\trange_set_append_unsafe(&ranges, bottom, top);\ndiff --git a/line-range.c b/line-range.c\nindex 323399d16..232c3909e 100644\n--- a/line-range.c\n+++ b/line-range.c\n@@ -47,7 +47,7 @@ static const char *parse_loc(const char *spec, nth_line_fn_t nth_line,\n \t\t\telse if (!num)\n \t\t\t\t*ret = begin;\n \t\t\telse\n-\t\t\t\t*ret = begin + num;\n+\t\t\t\t*ret = begin + num > 0 ? begin + num : 1;\n \t\t\treturn term;\n \t\t}\n \t\treturn spec;\ndiff --git a/t/annotate-tests.sh b/t/annotate-tests.sh\nindex 093832fef..6da48a2e0 100644\n--- a/t/annotate-tests.sh\n+++ b/t/annotate-tests.sh\n@@ -320,11 +320,11 @@ test_expect_success 'blame -L ,Y (Y == nlines)' '\n \n test_expect_success 'blame -L ,Y (Y == nlines + 1)' '\n \tn=$(expr $(wc -l <file) + 2) &&\n-\ttest_must_fail $PROG -L,$n file\n+\tcheck_count -L,$n A 1 B 1 B1 1 B2 1 \"A U Thor\" 1 C 1 D 1 E 1\n '\n \n test_expect_success 'blame -L ,Y (Y > nlines)' '\n-\ttest_must_fail $PROG -L,12345 file\n+\tcheck_count -L,12345 A 1 B 1 B1 1 B2 1 \"A U Thor\" 1 C 1 D 1 E 1\n '\n \n test_expect_success 'blame -L multiple (disjoint)' '\ndiff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh\nindex 661f9d430..c92a47b6d 100755\n--- a/t/t8003-blame-corner-cases.sh\n+++ b/t/t8003-blame-corner-cases.sh\n@@ -216,14 +216,18 @@ test_expect_success 'blame -L with invalid start' '\n '\n \n test_expect_success 'blame -L with invalid end' '\n-\ttest_must_fail git blame -L1,5 tres 2>errors &&\n-\ttest_i18ngrep \"has only 2 lines\" errors\n+\tgit blame -L1,5 tres >out &&\n+\ttest_line_count = 2 out\n '\n \n test_expect_success 'blame parses <end> part of -L' '\n \tgit blame -L1,1 tres >out &&\n-\tcat out &&\n-\ttest $(wc -l < out) -eq 1\n+\ttest_line_count = 1 out\n+'\n+\n+test_expect_success 'blame -Ln,-(n+1)' '\n+\tgit blame -L3,-4 nine_lines >out &&\n+\ttest_line_count = 3 out\n '\n \n test_expect_success 'indent of line numbers, nine lines' '\n-- \n2.14.3 (Apple Git-98)\n\n"},{"id":"350235","messageId":"20180615062928.37165-2-istephens@atlassian.com","threadId":"48612","inReplyTo":"20180615062928.37165-1-istephens@atlassian.com","subject":"[PATCH v7 2/2] log: prevent error if line range ends past end of file","fromName":"","fromEmail":"istephens@atlassian.com","sentAt":"2018-06-15T06:29:28Z","receivedAt":"2018-06-15T06:29:58Z","isPatch":true,"sender":{"key":"istephens@atlassian.com","avatar":"https://avatars.githubusercontent.com/u/30997546?v=4"},"body":"From: Isabella Stephens <istephens@atlassian.com>\n\nIf the -L option is used to specify a line range in git log, and the end\nof the range is past the end of the file, git will fail with a fatal\nerror. This commit prevents such behaviour - instead we perform the log\nfor existing lines within the specified range.\n\nThis commit also fixes a corner case where -L ,-n:file would be treated\nas a log over the whole file. Now we treat this as -L 1,-n:file and\nblame the first line of the file instead.\n\nSigned-off-by: Isabella Stephens <istephens@atlassian.com>\n---\n line-log.c          | 4 ++--\n t/t4211-line-log.sh | 5 ++---\n 2 files changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/line-log.c b/line-log.c\nindex cdc2257db..8ad074e58 100644\n--- a/line-log.c\n+++ b/line-log.c\n@@ -599,11 +599,11 @@ parse_lines(struct commit *commit, const char *prefix, struct string_list *args)\n \t\t\t\t    lines, anchor, &begin, &end,\n \t\t\t\t    full_name))\n \t\t\tdie(\"malformed -L argument '%s'\", range_part);\n-\t\tif (lines < end || ((lines || begin) && lines < begin))\n+\t\tif ((!lines && (begin || end)) || lines < begin)\n \t\t\tdie(\"file %s has only %lu lines\", name_part, lines);\n \t\tif (begin < 1)\n \t\t\tbegin = 1;\n-\t\tif (end < 1)\n+\t\tif (end < 1 || lines < end)\n \t\t\tend = lines;\n \t\tbegin--;\n \t\tline_log_data_insert(&ranges, full_name, begin, end);\ndiff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh\nindex d0377fae5..436b13ad2 100755\n--- a/t/t4211-line-log.sh\n+++ b/t/t4211-line-log.sh\n@@ -60,7 +60,6 @@ test_bad_opts \"-L 1:nonexistent\" \"There is no path\"\n test_bad_opts \"-L 1:simple\" \"There is no path\"\n test_bad_opts \"-L '/foo:b.c'\" \"argument not .start,end:file\"\n test_bad_opts \"-L 1000:b.c\" \"has only.*lines\"\n-test_bad_opts \"-L 1,1000:b.c\" \"has only.*lines\"\n test_bad_opts \"-L :b.c\" \"argument not .start,end:file\"\n test_bad_opts \"-L :foo:b.c\" \"no match\"\n \n@@ -86,12 +85,12 @@ test_expect_success '-L ,Y (Y == nlines)' '\n \n test_expect_success '-L ,Y (Y == nlines + 1)' '\n \tn=$(expr $(wc -l <b.c) + 1) &&\n-\ttest_must_fail git log -L ,$n:b.c\n+\tgit log -L ,$n:b.c\n '\n \n test_expect_success '-L ,Y (Y == nlines + 2)' '\n \tn=$(expr $(wc -l <b.c) + 2) &&\n-\ttest_must_fail git log -L ,$n:b.c\n+\tgit log -L ,$n:b.c\n '\n \n test_expect_success '-L with --first-parent and a merge' '\n-- \n2.14.3 (Apple Git-98)\n\n"},{"id":"350237","messageId":"db426610-90e3-e936-b630-a3d83e345e79@atlassian.com","threadId":"48612","inReplyTo":"xmqqh8mnb8j2.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v6 1/2] blame: prevent error if range ends past end of file","fromName":"Isabella Stephens","fromEmail":"istephens@atlassian.com","sentAt":"2018-06-15T06:36:13Z","receivedAt":"2018-06-15T06:36:21Z","isPatch":true,"sender":{"key":"istephens@atlassian.com","avatar":"https://avatars.githubusercontent.com/u/30997546?v=4"},"body":"> Don't t800[12]-*.sh need adjustment for this change, too?\n\nApologies. I will make sure I've run the whole test suite against my patches in future. I've sent through a 7th (and hopefully final) version.\n \n"},{"id":"350285","messageId":"xmqqefh729cb.fsf@gitster-ct.c.googlers.com","threadId":"48612","inReplyTo":"db426610-90e3-e936-b630-a3d83e345e79@atlassian.com","subject":"Re: [PATCH v6 1/2] blame: prevent error if range ends past end of file","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-06-15T18:28:04Z","receivedAt":"2018-06-15T18:28:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Isabella Stephens <istephens@atlassian.com> writes:\n\n>> Don't t800[12]-*.sh need adjustment for this change, too?\n>\n> Apologies. I will make sure I've run the whole test suite against\n> my patches in future. I've sent through a 7th (and hopefully\n> final) version.\n>  \n\nThanks.  Will queue and hopefully we can carry this in 'pu' (and\nlater down to 'next' and to 'master').\n\n"}]}