search

colcon / colcon-cmake

Extension for colcon to support CMake packages
http://colcon.readthedocs.io
Apache License 2.0
16 stars 25 forks source link

Match against capital PROJECT command #24

Closed mxgrey closed 5 years ago

mxgrey commented 5 years ago

CMake allows commands to be all lowercase or all uppercase, so we should check for a capital PROJECT in addition to lowercase project, or else we may miss some valid cases.

dirk-thomas commented 5 years ago

or else we may miss some valid cases.

Since CMake even allows mixed case for function names we might want to match that too.

dirk-thomas commented 5 years ago

The same applies to the other regular expressions, e.g. for finding dependencies.

mxgrey commented 5 years ago

I'm not very experienced with regex, so the latest commit kind of brute-forces an attempt at case insensitivity. If anyone knows a more elegant approach, please do share.

mxgrey commented 5 years ago

It seems Python re offers an argument flag re.IGNORECASE, so I've switched the searches to use that.

The only potential pitfall I can think of is that the search for pkg_check_modules also looks for a match against REQUIRED|QUIET|NO_CMAKE_PATH|NO_CMAKE_ENVIRONMENT_PATH, but now the case for those will be ignored, even though the arguments are technically case-sensitive. I don't imagine that will cause anyone any practical issues, though.

dirk-thomas commented 5 years ago

It seems Python re offers an argument flag re.IGNORECASE, so I've switched the searches to use that. The only potential pitfall I can think of is that the search for pkg_check_modules also looks for a match against REQUIRED|QUIET|NO_CMAKE_PATH|NO_CMAKE_ENVIRONMENT_PATH, ...

Making the whole regex case insensitive isn't an option for the reason you already mentioned.

Unfortunately making parts of the regex case insensitive with something like this ((?i:foo)bar) is only available as of Python 3.6 :disappointed:

dirk-thomas commented 5 years ago

I created #25 which uses the regex pattern available as of Python 3.6 when possible and falls back to expanding the string into lowercase / uppercase options. It also applies the patch to all matched function names.