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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: May 30, 2018, 08:45 UTC
Message-ID: <CAPig+cTCDP0zJonDFXyv6Kue5y4PMPaveyjfvfTVNoifzYg-Og@mail.gmail.com>
In-Reply-To: <20180529053037.38015-2-istephens@atlassian.com>

On Tue, May 29, 2018 at 1:30 AM,  <istephens@atlassian.com> 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/

Show 20 quoted lines

> 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 <istephens@atlassian.com>
> ---
> 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<begin>,<end> 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.