test 1165: improve pattern matching #12903
Conversation
|
Two failed checks seems to be unrelated. |
tests/test1165.pl
Outdated
| @@ -44,7 +44,7 @@ sub scanconf { | |||
| my ($f)=@_; | |||
| open S, "<$f"; | |||
| while(<S>) { | |||
| if(/(CURL_DISABLE_[A-Z_]+)/g) { | |||
| if(/(?:^|[^A-Za-z0-9\-])(CURL_DISABLE_[A-Z0-9_]+)(?:$|[^A-Za-z0-9\-])/g) { | |||
There was a problem hiding this comment.
This cannot be correct, since it puts the wrong part into $1.
Why not just extend the pattern? Surely it doesn't do any false positive matches elsewhere?
if(/(CURL_DISABLE_[A-Z0-9_]+)/g) {There was a problem hiding this comment.
Nope. :)
It is non-capturing group because of ?:
(?:^|[^A-Za-z0-9\-]) is matched but not captured.
The same for (?:$|[^A-Za-z0-9\-]).
It works correctly, I tested it with modified script version with printing all matched results.
Right now there are no false positives here even without the start and end patterns. But this is the test, it must catch incorrect code changes.
There was a problem hiding this comment.
Ok thanks, that goes beyond what I knew. I still think it's overly complicated for no good reason.
tests/test1165.pl
Outdated
| @@ -67,9 +67,9 @@ sub scanconf_cmake { | |||
| my ($f)=@_; | |||
| open S, "<$f"; | |||
| while(<S>) { | |||
| if(/(CURL_DISABLE_[A-Z_]+)/g) { | |||
| if(/(?:^|[^A-Za-z0-9\-])(CURL_DISABLE_[A-Z0-9_]+)(?:$|[^A-Za-z0-9\-])/g) { | |||
There was a problem hiding this comment.
It works as well.
I don't create PRs without proper local testing.
I added debug printing of the results and results are fine.
tests/test1165.pl
Outdated
| @@ -85,7 +85,7 @@ sub scan_file { | |||
| my ($source)=@_; | |||
| open F, "<$source"; | |||
| while(<F>) { | |||
| while(s/(CURL_DISABLE_[A-Z_]+)//) { | |||
| while(s/(?:^|[^A-Za-z0-9\-])(CURL_DISABLE_[A-Z0-9_]+)(?:$|[^A-Za-z0-9\-])//) { | |||
There was a problem hiding this comment.
This one is correct too.
Karlson2k
changed the title
|
@bagder The code works correctly. Let me know, please, if you still think that code should be simplified by removing non-capturing groups. |
Karlson2k
force-pushed
the
test1165_fix_01
branch
from
19a4c36
to
bb220d3
Compare
|
Force-pushed with correct commit text (and rebased) |
I think so. The regexes should be as simple as possible to help readers understand then. |
* Fix excluded digits at the end of the symbols ('CURL_DISABLE_POP3'
was checked as 'CURL_DISABLE_POP')
Karlson2k
force-pushed
the
test1165_fix_01
branch
from
bb220d3
to
1efdf80
Compare
|
Updated with non-capturing groups removed, as requested. |
|
Two build failures seem to be unrelated. |
|
Thanks! |
Karlson2k
changed the title
CURL_DISABLE_POP3was checked asCURL_DISABLE_POP)Discovered while working on PR #12897