Re: [PATCH] Avoid false positives in label detection in cpp diff hunk header regex.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 22.03.2013 23:32, schrieb Junio C Hamano:
> Johannes Sixt <j6t@xxxxxxxx> writes:
> 
>> Am 22.03.2013 16:02, schrieb Junio C Hamano:
>>> Vadim Zeitlin <vz-git@xxxxxxxxxxxx> 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.

> 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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]