Subject: Re[2]: [PATCH 1/3] Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal

From: Алексей Шумкин <[email-masked]>
Date: Wed, 5 Jan 2011 14:53:02 +0300

Hello, Thomas

TR> But the last line very conspicuously already mentions 'class', so why
TR> does it fail?
Yes, As you already discovered that last line match for class/record
definition but not for class methods.

I did as you said I changed commit message (also included
"Acked-by:"). So should I re-submit patch to the maillist as a new one
or as an answer to this thread?

TR> Zapped wrote:
>> Signed-off-by: Zapped <[email-masked]>

TR> As Junio already said, please provide a real name for the sign-off.
TR> But I also found the commit message and content confusing, probably
TR> because I haven't programmed Pascal for 15 years.

TR> You said

TR>   Fixes bug: git-diff: class methods are not detected in hunk headers for Pascal

>>  PATTERNS("pascal",
>> -      "^((procedure|function|constructor|destructor|interface|"
>> +      "^(((class[ \t]+)?(procedure|function)|constructor|destructor|interface|"
>>               "implementation|initialization|finalization)[ \t]*.*)$"
>>        "\n"
>>        "^(.*=[ \t]*(class|record).*)$",

TR> But the last line very conspicuously already mentions 'class', so why
TR> does it fail?

TR> I had to look up a bit of Pascal syntax.  Google helped with

TR>   http://www.freepascal.org/docs-html/ref/ref.html

TR> which answers this.  Also, as stated in SubmittingPatches, we
TR> generally word the messages as stating the behaviour of the changed
TR> version in the present tense.  So a better commit message would be

TR>   userdiff: match Pascal class methods

TR>   Class declarations were already covered by the second pattern, but
TR>   class methods have the 'class' keyword in front too.  Account for
TR>   it.

TR>   Signed-off-by: Алексей Крезов <[email-masked]>

TR> Ok, now I feel silly for only having a two-liner despite my
TR> complaints.

TR> That being said, I have now verified that the patch is good, and, you
TR> can include my

TR>   Acked-by: Thomas Rast <[email-masked]>

TR> in a reroll if you fix the above.