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

From: Алексей Шумкин <zapped@mail.ru>
Date: Jan 5, 2011, 11:53 UTC
Message-ID: <4510264083.20110105145302@mail.ru>
In-Reply-To: <201101041813.45053.trast@student.ethz.ch>

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 <zapped@mail.ru>

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

Show 6 quoted lines

>>  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: Алексей Крезов <zapped@mail.ru>

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 <trast@student.ethz.ch>

TR> in a reroll if you fix the above.