-
Notifications
You must be signed in to change notification settings - Fork 658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix cmake header #1491
Fix cmake header #1491
Conversation
* Version number can have multiple digits. * Tabs are handled as white space. * Trailing comments are ignored.
r'^[ \t]*CMAKE_MINIMUM_REQUIRED[ \t]*' | ||
r'\([ \t]*VERSION[ \t]*\d+(\.\d+)*[ \t]*' | ||
r'([ \t]FATAL_ERROR)?[ \t]*\)[ \t]*' | ||
r'(#[^\n]*)?$' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this last expression for? Consume an arbitrary number of comments? Is that really needed for the analysis?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you please shed some more light on this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The intention is to ignore comments at the end of the line, e.g. in
cmake_minimum_required(version 3.13) # CMake version check
including an empty comment at the end of the line:
cmake_minimum_required(version 3.13) #
Thanks, merged! |
* Fixed guessing of CMake by header. * Version number can have multiple digits. * Tabs are handled as white space. * Trailing comments are ignored. * Cleaned up regex to detect CMake header.
The current regex to guess CMake by header has several flaws this patch fixes:
Reference: https://cmake.org/cmake/help/v3.13/manual/cmake-language.7.html#syntax