On 07/12/2016 11:16 PM, Ben Campbell wrote: > A fix for https://gitlab.kitware.com/cmake/cmake/issues/16196
Thanks! Here are some comments. > the pairs of file()/string() > commands seem a bit convoluted for extracting strings out of the header > file - is there a more idiomatic approach? You could use a single `file(STRINGS)` call and then use `foreach()` with the `IN LISTS` mode to iterate through them. Then use `if(MATCHES)` to extract the individual parts with the `CMAKE_MATCH_<n>` variables. > Also, I'm a bit concerned that they are polluting scope by leaking out > the GIFMAJ/GIFMIN/GIFREL working variables... how would I improve this? Prefix the working variables with _GIF_ and then unset() them at the end. If the version is now expected to work please also update `Tests/CMakeOnly/AllFindModules/CMakeLists.txt` to check that a version number is extracted. In a CMake build tree run `ctest -R CMakeOnly.AllFindModules -V` to run the test. Thanks, -Brad -- Powered by www.kitware.com Please keep messages on-topic and check the CMake FAQ at: http://www.cmake.org/Wiki/CMake_FAQ Kitware offers various services to support the CMake community. For more information on each offering, please visit: CMake Support: http://cmake.org/cmake/help/support.html CMake Consulting: http://cmake.org/cmake/help/consulting.html CMake Training Courses: http://cmake.org/cmake/help/training.html Visit other Kitware open-source projects at http://www.kitware.com/opensource/opensource.html Follow this link to subscribe/unsubscribe: http://public.kitware.com/mailman/listinfo/cmake-developers
