Re: [PATCH 2/2] userdiff: improve Fortran xfuncname regex

From: Elijah Newren <newren@gmail.com>
Date: Aug 13, 2020, 14:04 UTC
Message-ID: <CABPp-BFwX-m=BryaxkSyh-126O_0GQxyqRvFnKeOQ66Feiq4Kw@mail.gmail.com>
In-Reply-To: <8014D105-0957-4A45-B375-1027E070C74E@gmail.com>

Hi Philippe,

On Thu, Aug 13, 2020 at 5:45 AM Philippe Blain <levraiphilippeblain@gmail.com> wrote:

Show 49 quoted lines

>
> Hi Elijah,
>
> > Le 12 août 2020 à 22:10, Elijah Newren <newren@gmail.com> a écrit :
> >
> > On Wed, Aug 12, 2020 at 3:34 PM Philippe Blain via GitGitGadget
> > <gitgitgadget@gmail.com> wrote:
> >>
> >> From: Philippe Blain <levraiphilippeblain@gmail.com>
> >>
> >> The third part of the Fortran xfuncname regex wants to match the
> >> beginning of a subroutine or function, so it allows for all characters
> >> except `'`, `"` or whitespace before the keyword 'function' or
> >> 'subroutine'. This is meant to match the 'recursive', 'elemental' or
> >> 'pure' keywords, as well as function return types, and to prevent
> >> matches inside strings.
> >>
> >> However, the negated set does not contain the `!` comment character,
> >> so a line with an end-of-line comment containing the keyword 'function' or
> >> 'subroutine' followed by another word is mistakenly chosen as a hunk header.
> >>
> >> Improve the regex by adding `!` to the negated set.
> >>
> >> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
> >> ---
> >> t/t4018/fortran-comment-keyword | 1 -
> >> userdiff.c                      | 2 +-
> >> 2 files changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/t/t4018/fortran-comment-keyword b/t/t4018/fortran-comment-keyword
> >> index c5dbdb4c61..e9206a5379 100644
> >> --- a/t/t4018/fortran-comment-keyword
> >> +++ b/t/t4018/fortran-comment-keyword
> >> @@ -8,7 +8,6 @@
> >>       real funcB  ! grid function b
> >>
> >>       real ChangeMe
> >> -      integer broken
> >>
> >>       end subroutine RIGHT
> >>
> >
> > This change seems orthogonal to the explanation in the commit message.
> > What is its purpose, and does it belong in this commit or a different
> > one?
>
> If you take a look at t/t4018/README, the way to mark t4018 tests as "known failures"
> is to insert "broken" somewhere in the file. Since I'm fixing the regex in this commit to be able
> to cope with the situation in t/t4018/fortran-comment-keyword, I'm unmarking this test as broken.

Ah, gotcha. I guess that's what I get for trying to review a random patch outside my area of expertise. :-) Thanks for explaining how this works to me.

Elijah