git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: MOUMITA DHAR <dhar61595@gmail.com>
Cc: "Eric Sunshine" <sunshine@sunshineco.com>,
	"Junio C Hamano" <gitster@pobox.com>,
	"René Scharfe" <l.s.r@web.de>,
	"Atharva Raykar" <raykar.ath@gmail.com>,
	"D. Ben Knoble" <ben.knoble@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH v4 1/1][GSOC] userdiff: extend Bash pattern to cover more shell function forms
Date: Sat, 10 May 2025 14:40:39 +0200	[thread overview]
Message-ID: <ada6ff3b-1311-4d47-a098-7abc0ea9c46b@kdbg.org> (raw)
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:
> 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

> "^[ \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]*"
> "(\\{|\\(\\(?|\\[\\[)"

> 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


  reply	other threads:[~2025-05-10 12:40 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-11 11:46 [PATCH 0/1] [GSOC 2025] [Newbie] userdiff: add built-in pattern for shell scripts Moumita
2025-02-11 11:46 ` [PATCH 1/1] Added built in function recognition for shell Moumita
2025-02-15 14:37   ` Johannes Sixt
2025-02-18 15:35 ` [PATCH v2 0/1] [PATCH v2 0/1] [GSOC 2025] [Newbie] userdiff: add built-in pattern for shell scripts Moumita
2025-02-18 15:35   ` [PATCH v2 1/1] userdiff: extend Bash pattern to cover more shell function forms Moumita
2025-02-18 19:30     ` Junio C Hamano
2025-02-22 18:15       ` Johannes Sixt
2025-02-24 16:28         ` Junio C Hamano
2025-02-18 23:38     ` Junio C Hamano
2025-02-22 18:14     ` Johannes Sixt
2025-02-18 17:30   ` [PATCH v2 0/1] [PATCH v2 0/1] [GSOC 2025] [Newbie] userdiff: add built-in pattern for shell scripts Eric Sunshine
2025-03-28 20:05   ` [PATCH v3 0/1] userdiff: improve Bash function and word regex patterns Moumita
2025-03-28 20:05     ` [PATCH v3 1/1] userdiff: extend Bash pattern to cover more shell function forms Moumita
2025-03-29 19:26     ` [PATCH v3 0/1] userdiff: improve Bash function and word regex patterns Junio C Hamano
2025-03-30 12:28       ` MOUMITA DHAR
2025-03-30 13:39     ` [PATCH v4 0/1][GSOC] userdiff:Added newlines at the end of the test cases Moumita
2025-03-30 13:39       ` [PATCH v4 1/1][GSOC] userdiff: extend Bash pattern to cover more shell function forms Moumita
2025-05-02 21:27         ` Junio C Hamano
2025-05-06 16:30         ` Johannes Sixt
2025-05-10 11:37           ` MOUMITA DHAR
2025-05-10 12:40             ` Johannes Sixt [this message]
2025-05-11 12:58       ` [PATCH v5 0/1] Added the closing ")" to make sure is not unbalanced and corrected the tests for word diff Moumita
2025-05-11 12:58         ` [PATCH v5 1/1] userdiff: extend Bash pattern to cover more shell function forms Moumita
2025-05-11 13:28         ` Moumita
2025-05-11 13:28           ` Moumita
2025-05-11 13:37         ` Moumita
2025-05-11 14:11         ` [PATCH v6 0/1] Added the newline after the test in t/4018 Moumita
2025-05-11 14:11           ` [PATCH v6 1/1] userdiff: extend Bash pattern to cover more shell function forms Moumita
2025-05-13 18:50             ` Junio C Hamano
2025-05-14  6:33               ` MOUMITA DHAR
2025-05-16  7:25             ` Johannes Sixt
2025-05-17 13:09               ` Junio C Hamano
2025-05-18  7:41                 ` Johannes Sixt
2025-05-16 14:45           ` [PATCH v7 0/1] Updated the word diff regex for Bash scripts Moumita
2025-05-16 14:45             ` [PATCH v7 1/1] userdiff: extend Bash pattern to cover more shell function forms Moumita
2025-05-16 17:45               ` Johannes Sixt
2025-05-16 21:56                 ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ada6ff3b-1311-4d47-a098-7abc0ea9c46b@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=ben.knoble@gmail.com \
    --cc=dhar61595@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=raykar.ath@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).