On Sun, Aug 19, 2012 at 4:17 PM, Stephen Kelly <span dir="ltr"><<a href="mailto:steveire@gmail.com" target="_blank">steveire@gmail.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im">Brad King wrote:<br>
<br>
> On 8/16/2012 11:42 AM, Stephen Kelly wrote:<br>
>> I thought that I could make -DNAME="value" appear twice with this code:<br>
>><br>
>>  add_definitions(-D"NAME=Value" -DNAME="Value")<br>
><br>
> CMake syntax treats quotes in the middle of an unquoted argument<br>
> literally.  The double quotes are part of the string.  This code:<br>
><br>
><br>
<a href="http://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmMakefile.cxx;hb=v2.8.9#l1269" target="_blank">http://cmake.org/gitweb?p=cmake.git;a=blob;f=Source/cmMakefile.cxx;hb=v2.8.9#l1269</a><br>
><br>
> does not recognize it as a definition flag.<br>
><br>
>> This might be intentional, but then the code in AppendDefines is dead and<br>
>> should be removed. If it is unintentional, then I have found a bug.<br>
><br>
> The code should be active if the value needs quoting on the<br>
> generated shell command line:<br>
><br>
>  set_property(TARGET foo PROPERTY COMPILE_DEFINITIONS "FOO=with space")<br>
<br>
</div>What you say makes sense. But maybe the comment I pointed out should be<br>
removed or made more clear. It doesn't seem to relate to what the code is<br>
actually about. What do you think?<br>
<div class="im"><br>
>> There is still Watcom escaping which I did not test though. As the<br>
>> current patch stores unescaped defines, the escaped defines may not be<br>
>> unique.<br>
><br>
> The defines CMake knows about will be unique.  It is only the<br>
> raw -D flag that leaks through the above-linked code without<br>
> being recognized that might create a duplicate.  Rather than<br>
> try to handle the historical pre-quoted case and guess what<br>
> the user meant we should either make no change or just add<br>
> a warning for it.<br>
<br>
</div>Ok, sounds fine to me. In that case, the branch is almost ready to merge to<br>
next. All that remains is to test the visual studio patch. Can I just merge<br>
the branch and fix any errors related to that which appear on the dashboard,<br>
or should I wait and ask someone to test it first?<br>
<br>
Thanks,<br>
<br>
Steve.<br>
<div class="HOEnZb"><div class="h5"><br>
<br>
<br>
--<br>
<br>
Powered by <a href="http://www.kitware.com" target="_blank">www.kitware.com</a><br>
<br>
Visit other Kitware open-source projects at <a href="http://www.kitware.com/opensource/opensource.html" target="_blank">http://www.kitware.com/opensource/opensource.html</a><br>
<br>
Please keep messages on-topic and check the CMake FAQ at: <a href="http://www.cmake.org/Wiki/CMake_FAQ" target="_blank">http://www.cmake.org/Wiki/CMake_FAQ</a><br>
<br>
Follow this link to subscribe/unsubscribe:<br>
<a href="http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers" target="_blank">http://public.kitware.com/cgi-bin/mailman/listinfo/cmake-developers</a><br>
</div></div></blockquote></div><br><div><br></div><div>I could run an Experimental based on this branch with Visual Studio and point you to the CDash results if you'd like.</div><div><br></div><div>Maybe not till tomorrow, though.</div>
<div><br></div>