git context diff does not recognize shell function name

git context diff does not recognize shell function name

[Windl, Ulrich]

[-- Attachment #1: Type: text/plain, Size: 858 bytes --]

Hi!

In git 2.51.0 I found a minor issue:
"git diff" did not show the corresponding shell function name. Admittedly I used a somewhat uncommon line-breaking:

The context in question looks like this:
---
...
run_query()
{
    local p_l_0="$1" LOG_BASE="$2" SUFFIX="$3" URL_BASE="$4" URL="$5" \
_COUNT="${6:-$COUNT}" RESPONSE_CHECKER="$7"
    local me="${FUNCNAME[0]}" m
...
---

The context diff looked like this:
---
...
@@ -193,6 +188,36 @@ get_params()
... 
run_query()
 {
@@ -201,10 +226,9 @@ _COUNT="${6:-$COUNT}" RESPONSE_CHECKER="$7"
     local me="${FUNCNAME[0]}" m
...
---
("..." is deleted lines)
So this may be a combination of unfortunate context sequencing and the first line of the new context being unusual.
The second chunk would still be the same context, namely "run_query()".

Kind regards,
Ulrich Windl


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5581 bytes --]

Re: git context diff does not recognize shell function name

[Junio C Hamano]

"Windl, Ulrich" <u.windl@ukr.de> writes:

> Hi!
>
> In git 2.51.0 I found a minor issue:

That is a release from more than a year ago, but I do not think we
had any change to userdiff (i.e., built-in patterns that define what
line is the function header for various languages) since then (we
added Swift and tweaked scheme).  So for practical purposes, I would
expect that whatever you saw in Git 2.51 for shell to be reproducible
in today's code.

Or are you reporting that Git 2.50 (or something older than 2.51)
behaved in a way you prefer, and 2.51 behaves differently?

> "git diff" did not show the corresponding shell function
> name. Admittedly I used a somewhat uncommon line-breaking:
>
> The context in question looks like this:
> ---
> ...
> run_query()
> {
>     local p_l_0="$1" LOG_BASE="$2" SUFFIX="$3" URL_BASE="$4" URL="$5" \
> _COUNT="${6:-$COUNT}" RESPONSE_CHECKER="$7"
>     local me="${FUNCNAME[0]}" m
> ...
> ---
>
> The context diff looked like this:
> ---
> ...
> @@ -193,6 +188,36 @@ get_params()
> ... 
> run_query()
>  {

I assume that your e-mail message is whitespace damaged and
run_query() is actually indented by one space, being a context line
together with the opening brace next line?  The header stating
get_params() is expected, as the "function name" do not lookahead to
find run_query() in the hunk it is going to show.

> @@ -201,10 +226,9 @@ _COUNT="${6:-$COUNT}" RESPONSE_CHECKER="$7"

It is not that the line that begins run_query()'s definition does
not look like a beginning of a function.  It is that this _COUNT=
line does look like a beginning of a new thing, and because it comes
after the start of run_query().

The name of the file would affect which set of regular expressions
are used to sift lines into function headers and other lines.  When
the file has no known suffix with language specific rules defined, I
think we just pick any line that begins with a non-whitespace
character, which explains why the line that begins with
_COUNT=... overrides the line that begins with run_query() which
would have been a candidate as another and later candidate.

>      local me="${FUNCNAME[0]}" m
> ...
> ---
> ("..." is deleted lines)
> So this may be a combination of unfortunate context sequencing and
> the first line of the new context being unusual.  The second chunk
> would still be the same context, namely "run_query()".