line-range: skip trailing empty lines in funcname range

[spkrka]

cc: René Scharfe l.s.r@web.de
cc: Nikita Makarov n.makarov@yadro.com
cc: Kristofer Karlsson krka@spotify.com

[spkrka]

/cc Nikita Makarov n.makarov@yadro.com

[gitgitgadget]

User Nikita Makarov <n.makarov@yadro.com> has been added to the cc: list.

[spkrka]

/submit

[gitgitgadget]

Submitted as pull.2234.git.1789991377413.gitgitgadget@gmail.com

To fetch this version into FETCH_HEAD:

git fetch https://github.com/gitgitgadget/git/ pr-2234/spkrka/krka/line-range-trailing-blank-v1

To fetch this version to local tag pr-2234/spkrka/krka/line-range-trailing-blank-v1:

git fetch --no-tags https://github.com/gitgitgadget/git/ tag pr-2234/spkrka/krka/line-range-trailing-blank-v1

[gitgitgadget]

Junio C Hamano wrote on the Git mailing list (how to reply to this email):

"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> diff --git a/line-range.c b/line-range.c
> index b99f0d9895..44b52d4e34 100644
> --- a/line-range.c
> +++ b/line-range.c
> @@ -233,6 +233,11 @@ static const char *parse_range_funcname(
>  		(*end)++;
>  	}
>  
> +	/* exclude trailing empty lines from the function range */
> +	while (*end > *begin + 1 &&
> +	       nth_line_cb(cb_data, *end - 1)[0] == '\n')
> +		(*end)--;
> +

While the new behaviour may mean well, the implementation falls a
bit short of matching what "grep -W" did, doesn't it?  Over there we
treat a line with only whitespace characters on it as an empty line
that we want to ignore, but the above code does not allow anything
before the LF.  I do not know offhand if the above misbehaves on
CRLF text files, but if you treated a line with only whitespace
characters on it as an empty line, concerns on CRLF files would also
disappear ;-).

[gitgitgadget]

Kristofer Karlsson wrote on the Git mailing list (how to reply to this email):

On Mon, 21 Sept 2026 at 19:30, Junio C Hamano <gitster@pobox.com> wrote:
>
> "Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
>
> > diff --git a/line-range.c b/line-range.c
> > index b99f0d9895..44b52d4e34 100644
> > --- a/line-range.c
> > +++ b/line-range.c
> > @@ -233,6 +233,11 @@ static const char *parse_range_funcname(
> >               (*end)++;
> >       }
> >
> > +     /* exclude trailing empty lines from the function range */
> > +     while (*end > *begin + 1 &&
> > +            nth_line_cb(cb_data, *end - 1)[0] == '\n')
> > +             (*end)--;
> > +
>
> While the new behaviour may mean well, the implementation falls a
> bit short of matching what "grep -W" did, doesn't it?  Over there we
> treat a line with only whitespace characters on it as an empty line
> that we want to ignore, but the above code does not allow anything
> before the LF.  I do not know offhand if the above misbehaves on
> CRLF text files, but if you treated a line with only whitespace
> characters on it as an empty line, concerns on CRLF files would also
> disappear ;-).
>

Ah yes, a blank line is not the same as an empty line, good point.

I also noticed the old behavior is still in the documentation
for man git grep:

    -W, --function-context
        Show the surrounding text from the previous line
        containing a function name up to the one before
        the next function name, effectively showing the whole
        function in which the match was found.

And for man git log (in the -L section):

    If :<funcname> is given in place of <start> and <end>,
    it is a regular expression that denotes the range from
    the first funcname line that matches <funcname>, up to
    the next funcname line.  :<funcname> searches from the
    end of the previous -L range, if any, otherwise from the
    start of file.

So neither of these functions should do any blank-line trimming
according to the docs (but the docs can be changed).

So I suppose the question is what behavior we actually want,
and if these two use cases should be analogous or not.

Thanks,
Kristofer

[gitgitgadget]

User Kristofer Karlsson <krka@spotify.com> has been added to the cc: list.