Re: [PATCH v4 1/1][GSOC] userdiff: extend Bash pattern to cover more shell function forms

From: Johannes Sixt <j6t@kdbg.org>
Date: May 10, 2025, 12:40 UTC
Message-ID: <ada6ff3b-1311-4d47-a098-7abc0ea9c46b@kdbg.org>
In-Reply-To: <CAF=ncLbaxYPRx79wYVgpMFuV7UGSObMD+A3zTssWvtVJmXHXVg@mail.gmail.com>

The way how you cited the message makes it very difficult to see what is citation, code, or your message text. Please insert sufficient blank lines in your replies. Also, indentation may help to declare what is the code that you talk about.

Am 10.05.25 um 13:37 schrieb MOUMITA DHAR:

Show 26 quoted lines

> On Tue, 6 May 2025 at 22:00, Johannes Sixt <j6t@kdbg.org> wrote:
>>> diff --git a/userdiff.c b/userdiff.c
>>> index 340c4eb4f7..4c77c7e0f6 100644
>>> --- a/userdiff.c
>>> +++ b/userdiff.c
>>> @@ -64,15 +64,27 @@ PATTERNS("bash",
>>>            /* Bashism identifier with optional parentheses */
>>>            "(function[ \t]+[a-zA-Z_][a-zA-Z0-9_]*(([ \t]*\\([ \t]*\\))|([ \t]+))"
>>>        ")"
>>> -      /* Optional whitespace */
>>> -      "[ \t]*"
>>> -      /* Compound command starting with `{`, `(`, `((` or `[[` */
>>> -      "(\\{|\\(\\(?|\\[\\[)"
>>> +      /* Everything after the function header is captured  */
>>> +      ".*$"
>>
>> I remember suggesting to capture everything after the function header.
>> However, If I am not mistaken, this does not do what I intended (and as
>> written it means that a pointless matching operation happens). The hunk
>> header shows everything that is in the outermost parentheses (group).
>> This catch-all, however, is even outside the outermost group and not
>> captured. It should be above the closing parenthesis that we see in the
>> context.
> > I am sorry I want to understand a thing  , we want everything from
> the function name to the end of the line to be the hunk header
> right ? So in the pattern

Show 15 quoted lines

> "^[ \t]*"> /* Start of captured text */
> "("
> "("
>      /* POSIX identifier with mandatory parentheses */
>      "[a-zA-Z_][a-zA-Z0-9_]*[ \t]*\\([ \t]*\\))"
> "|"
>      /* Bashism identifier with optional parentheses */
>      "(function[ \t]+[a-zA-Z_][a-zA-Z0-9_]*(([ \t]*\\([ \t]*\\))|([ \t]+))"
> ")"
> /* Optional whitespace */
> "[ \t]*"
> /* Compound command starting with `{`, `(`, `((` or `[[` */
> "(\\{|\\(\\(?|\\[\\[)"
> /* End of captured text */
> ")"

> if I replace

> "[ \t]*"
> "(\\{|\\(\\(?|\\[\\[)"

Show 6 quoted lines

> part with .*$ then will it not capture the
> entire line ? I mean the outermost group ends here-
> 
>  /* End of captured text */
> ")"
> right ? What am I getting wrong ?

It is my error, sorry. I see now that your intended change was correct.

However, in my tests, a comment after the function name is not captured. The reason is that the parentheses are not balanced. My editor tells me that the parenthesis in "(function" is matched with the lone ")" in the next line, but the latter is intended to match up with the "(" above the '/* POSIX' comment. There is something fishy going on and needs a fix.

To test, apply this:

diff --git a/t/t4018/bash-posix-style-function b/t/t4018/bash-posix-style-function
index a4d144856e..673c51b89e 100644
--- a/t/t4018/bash-posix-style-function
+++ b/t/t4018/bash-posix-style-function
@@ -1,4 +1,4 @@
-RIGHT() {
+afunc () { # RIGHT
 
     ChangeMe
 }

-- Hannes