Re: [PATCH] Avoid false positives in label detection in cpp diff hunk header regex.
- From
Johannes Sixt <j6t@kdbg.org>
- Date
- Mar 22, 2013, 23:11 UTC
- Message-ID
- <514CE53F.3080308@kdbg.org>
- In-Reply-To
- <7vhak35ami.fsf@alter.siamese.dyndns.org>
Am 22.03.2013 23:32, schrieb Junio C Hamano:
Show 32 quoted lines
> Johannes Sixt <j6t@kdbg.org> writes:
>
>> Am 22.03.2013 16:02, schrieb Junio C Hamano:
>>> Vadim Zeitlin <vz-git@zeitlins.org> writes:
>>>
>>>> A C++ method start such as
>>>>
>>>> void
>>>> foo::bar()
>>>>
>>>> wasn't recognized by cpp diff driver as it mistakenly included "foo::bar" as a
>>>> label. However the colon in a label can't be followed by another colon, so
>>>> recognize this case specially to correctly detect C++ methods using this style.
>>
>> Much appreciated!
>>
>>>> PATTERNS("cpp",
>>>> /* Jump targets or access declarations */
>>>> - "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:.*$\n"
>>>> + "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:].*$|$)\n"
>>>
>>> Hmm. Wouldn't "find a word (possibly after indentation), colon and
>>> then either a non-colon or end of line" be sufficient and simpler?
>>> iow, something like...
>>>
>>> "!^[ \t]*[A-Za-z_][A-Za-z_0-9]*:([^:]|$)"
>>
>> Yes, indeed. We don't need to match more than necessary in a negative
>> pattern. The \n must still remain, though.
>
> ... because \n is not for matching against the text, but merely to
> separate the regular expressions, right?Correct.
Show 6 quoted lines
> I also wonder if > > label : > > should also be caught, or is it too weird format to be worth > supporting?
It's easy to support, by inserting another [ \t] before the first colon. So, why not?
-- Hannes