Re: [PATCH] blame: prevent error if range ends past end of file
On 30/5/18 6:45 pm, Eric Sunshine wrote: >> diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt >> @@ -152,6 +152,16 @@ Also you can use a regular expression to specify the >> line range: >> which limits the annotation to the body of the `hello` subroutine. >> >> +A range that begins or ends outside the bounds of the file will >> +blame the relevant lines. For example: >> + >> + git blame -L 10,-20 foo >> + git blame -L 10,+20 foo >> + >> +will respectively blame the first 10 and last 11 lines of a >> +20 line file. However, blaming a line range that is entirely >> +outside the bounds of the file will fail. > > This documentation seems misplaced. Rather than inserting it after the > discussion of -L/regex/, a more natural place would be just above > -L/regex/ where -L, is discussed. > > However, I am not at all convinced that this behavior should be > documented to this level of detail. Doing so assigns too much emphasis > to what should be intuitive, thus wastes readers' time wondering why > it is so heavily emphasized. At _most_, I would think you could say > merely: > > A range that begins or ends outside the bounds of the file > will be clipped to the file's extent. > > and drop the example and discussion of the example results altogether. > > In fact, because this new behavior is what most users will intuitively > expect, it might be perfectly reasonable to not say anything about it > at all (that is, don't modify git-blame.txt) Thanks for reviewing Eric. I've submitted a v6 patch in response to your feedback. I agree that given the behavior is intuitive it's not necessary to document this change, so I've reverted the change to git-blame.txt entirely.
Re: [PATCH] blame: prevent error if range ends past end of file
On Tue, May 29, 2018 at 1:30 AM, wrote: > 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, git will fail with a fatal > error. This commit prevents such behavior - instead we display the blame > for existing lines within the specified range. Makes sense; the new behavior is intuitive and more friendly. > Tests and documentation are ammended accordingly. s/ammended/amended/ > This commit also fixes two corner cases. Blaming -L n,-(n+1) now blames > the first n lines of a file rather than from n to the end of the file. > Blaming -L ,-n will be treated as -L 1,-n and blame the first line of > the file, rather than blaming the whole file. > > Signed-off-by: Isabella Stephens > --- > diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt > @@ -152,6 +152,16 @@ Also you can use a regular expression to specify the > line range: > which limits the annotation to the body of the `hello` subroutine. > > +A range that begins or ends outside the bounds of the file will > +blame the relevant lines. For example: > + > + git blame -L 10,-20 foo > + git blame -L 10,+20 foo > + > +will respectively blame the first 10 and last 11 lines of a > +20 line file. However, blaming a line range that is entirely > +outside the bounds of the file will fail. This documentation seems misplaced. Rather than inserting it after the discussion of -L/regex/, a more natural place would be just above -L/regex/ where -L, is discussed. However, I am not at all convinced that this behavior should be documented to this level of detail. Doing so assigns too much emphasis to what should be intuitive, thus wastes readers' time wondering why it is so heavily emphasized. At _most_, I would think you could say merely: A range that begins or ends outside the bounds of the file will be clipped to the file's extent. and drop the example and discussion of the example results altogether. In fact, because this new behavior is what most users will intuitively expect, it might be perfectly reasonable to not say anything about it at all (that is, don't modify git-blame.txt). Thanks.
[PATCH] blame: prevent error if range ends past end of file
From: Isabella Stephens
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, git will fail with a fatal
error. This commit prevents such behavior - instead we display the blame
for existing lines within the specified range. Tests and documentation
are ammended accordingly.
This commit also fixes two corner cases. Blaming -L n,-(n+1) now blames
the first n lines of a file rather than from n to the end of the file.
Blaming -L ,-n will be treated as -L 1,-n and blame the first line of
the file, rather than blaming the whole file.
Signed-off-by: Isabella Stephens
---
Documentation/git-blame.txt | 10 ++
builtin/blame.c | 4 ++--
line-range.c | 2 +-
t/t8003-blame-corner-cases.sh | 12
4 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index 16323eb80..8cb81f57a 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -152,6 +152,16 @@ Also you can use a regular expression to specify the line
range:
which limits the annotation to the body of the `hello` subroutine.
+A range that begins or ends outside the bounds of the file will
+blame the relevant lines. For example:
+
+ git blame -L 10,-20 foo
+ git blame -L 10,+20 foo
+
+will respectively blame the first 10 and last 11 lines of a
+20 line file. However, blaming a line range that is entirely
+outside the bounds of the file will fail.
+
When you are not interested in changes older than version
v2.6.18, or changes older than 3 weeks, you can use revision
range specifiers similar to 'git rev-list':
diff --git a/builtin/blame.c b/builtin/blame.c
index 9dcb367b9..e1359b192 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -886,13 +886,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 && (top || 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/line-range.c b/line-range.c
index 323399d16..232c3909e 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 > 0 ? begin + num : 1;
return term;
}
return spec;
diff --git a/t/t8003-blame-corner-cases.sh b/t/t8003-blame-corner-cases.sh
index 661f9d430..c92a47b6d 100755
--- a/t/t8003-blame-corner-cases.sh
+++ b/t/t8003-blame-corner-cases.sh
@@ -216,14 +216,18 @@ 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 &&
+ test_line_count = 2 out
'
test_expect_success 'blame parses part of -L' '
git blame -L1,1 tres >out &&
- cat out &&
- test $(wc -l < out) -eq 1
+ test_line_count = 1 out
+'
+
+test_expect_success 'blame -Ln,-(n+1)' '
+ git blame -L3,-4 nine_lines >out &&
+ test_line_count = 3 out
'
test_expect_success 'indent of line numbers, nine lines' '
--
2.14.3 (Apple Git-98)
