On 07/01/2016 11:48 AM, Ruslan Baratov wrote:
> On 01-Jul-16 15:58, Brad King wrote:
>> Please try to structure the logic right in the if/elseif part.
>> Matching in those also sets `CMAKE_MATCH_*` variables so one
>> does not need to double-match.
> 'elseif' part try to find 'archive.tar.gz' in '${fname}', then 'string(REGEX' 
> try to match it in '${url}'. There is no reusing of 'CMAKE_MATCH_*'.

I don't think we should duplicate the

  "(\\.|=)(7z|tar|tar\\.bz2|tar\\.gz|tar\\.xz|tbz2|tgz|txz|zip)$"

expression.  The stripping of ?.* can be done earlier, or done as
part of the main match.

>> Also, the `([^/]*)\\?.*` part of the regex should be more
>> like `([^/?]*)\\?.*` to avoid eagerly matching early `?`.
> We can't have question mark ('?') in path as far as I understand, it should 
> be percent-encoded, will be |'%3F'.|||
> https://en.wikipedia.org/wiki/Uniform_Resource_Locator#Syntax
> https://en.wikipedia.org/wiki/Percent-encoding

In that case it won't hurt to exclude `?` and `#` from the
allowed matching.

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

Reply via email to