[Top][All Lists]

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

Re: Regexp linting scan

From: Mattias Engdegård
Subject: Re: Regexp linting scan
Date: Thu, 5 Dec 2019 12:38:27 +0100

5 dec. 2019 kl. 03.10 skrev Michael Welsh Duggan <address@hidden>:

>> -  "^\\(mailto:\\)\\([^?]+\\)*\\(\\?\\(.*\\)\\)*"
>> +  "^\\(mailto:\\)\\([^?]+\\)?\\(\\?\\(.*\\)\\)*"
> Wouldn't "^\\(mailto:\\)\\([^?]*\\)\\(\\?\\(.*\\)\\)*" make more sense?
> Maybe even without the grouping, as it doesn't seem to be used.  If I'm
> not mistaken (and I might be) this matches "mailto:"; followed by anything
> as long as it contains at most one question mark.

Group 2 is actually used (see rfc2368-mailto-prequery-index). Although your 
suggested change does not alter the set of matched strings, the code would need 
to be altered as well, since group 2 would then be the empty string instead of 
nil when it did not match anything.
(The group around mailto: is harder to justify.)

>> -               (while (verilog-re-search-forward-quick "\\.\\s 
>> *\\([a-zA-Z0-9`_$]+\\)*\\s *(\\s *\\1\\s *)\\s *" end-pt t)
>> +               (while (verilog-re-search-forward-quick "\\.\\s 
>> *\\([a-zA-Z0-9`_$]+\\)?\\s *(\\s *\\1\\s *)\\s *" end-pt t)
> Similarly, in this one I think you could also replace the `+' with a `*'
> and leave out the `?'.

Group 1 is used in a backref, which only matches if the group matched. With 
your change, the regexp would match ".()", which isn't matched at present.

The repetition-of-repetition check does not complain about \(X+\)? for these 
reasons. It is also not a performance concern.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]