<div dir="ltr">On Thu, Aug 21, 2008 at 9:01 AM, Brad King <span dir="ltr"><<a href="mailto:brad.king@kitware.com">brad.king@kitware.com</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="Ih2E3d">Philip Lowman wrote:<br>
> On Wed, Aug 20, 2008 at 10:54 AM, Brad King <<a href="mailto:brad.king@kitware.com">brad.king@kitware.com</a><br>
</div><div><div></div><div class="Wj3C7c">> <mailto:<a href="mailto:brad.king@kitware.com">brad.king@kitware.com</a>>> wrote:<br>
><br>
> The "set" command already supports unsetting:<br>
><br>
> set(FOO xyz)<br>
> set(FOO) # unsets<br>
> if(DEFINED FOO)<br>
> message("This message does not appear.")<br>
> endif(DEFINED FOO)<br>
><br>
> I think it should just be fixed for CACHE and ENV variables.<br>
><br>
><br>
> I knew that set() supported unsetting local variables but wasn't sure it<br>
> would be safe to extend it to CACHE and ENV variables as a bugfix.<br>
> Regardless of whether this is appropriate to do or not adding an unset()<br>
> command also seemed to make the language simpler to understand because:<br>
> set(FOO)<br>
> as someone else pointed out, is somewhat confusing. Without referencing<br>
> the documentation many programmers might think this defines a variable<br>
> called FOO and sets it to empty but what it really is doing is<br>
> *undefining* the variable FOO entirely if it exists.<br>
<br>
</div></div>Okay, I agree. It will be easier and more readable to add the unset<br>
command than to extend the "set" command to "unset" things :)<br>
<br>
However, this:<br>
<br>
unset(MY_CACHE_VARIABLE)<br>
<br>
should not remove the cache entry. It should only unset the CMake<br>
variable. In order to remove the cache entry, we should require<br>
<br>
unset(MY_CACHE_VARIABLE CACHE)<br>
<br>
Otherwise there is no way to remove the variable without removing the<br>
cache entry.<br>
<br>
One nice thing about the unset command is that it doesn't make sense to<br>
provide a value. Therefore we can add extra arguments like 'CACHE'<br>
without ambiguity.</blockquote><div><br>That makes sense. Would you like me to fix the patch and resubmit?<br></div></div><br>-- <br>Philip Lowman<br>
</div>