<br><br><div class="gmail_quote">On Tue, Aug 4, 2009 at 1:23 AM, Andreas Pakulat <span dir="ltr"><<a href="mailto:apaku@gmx.de">apaku@gmx.de</a>></span> wrote:<br><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="im">On 03.08.09 22:57:29, Aleix Pol Gonzalez wrote:<br>
> SVN commit 1006580 by apol:<br>
><br>
> Properly look for Kompare, the CMake way.<br>
<br>
</div>Why? IMHO its just unnecessary overhead compared to what "finding<br>
kompare" actually means. All we need is find 1 single header file.</blockquote><div>Yes, and in CMake we find using find_package.<br>I can change to find file if you prefer.<br> <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
<br>
Apart from that:<br>
<div class="im"><br>
> +macro_log_feature(KOMPARE_FOUND "Kompare" "KPart to view file differences"<br>
> +                        "<a href="http://www.caffeinated.me.uk/kompare/" target="_blank">http://www.caffeinated.me.uk/kompare/</a>" FALSE ""<br>
> +                        "Required for difference checking")<br>
<br>
</div>This website is completely outdated, if we put a webpage here it should<br>
at least point to the right sources and list the current developers.</blockquote><div> </div><div>Ok, I can change to: <a href="http://www.google.es/search?q=kompare">http://www.google.es/search?q=kompare</a><br>I agree we should specify it's on kdesdk. I'll add it later<br>
 <br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div class="im"><br>
> +configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/config-kdevplatform.h.cmake<br>
> +                ${CMAKE_CURRENT_BINARY_DIR}/config-kdevplatform.h )<br>
> +<br>
<br>
</div>Even more code to execute.</blockquote><div> </div><div>I wonder what you mean. Every single cmake project use configure file for conditional compilation inside files. I don't think is that much overhead anyway. If cmake parsing (either cmake's or our's) is not fast enough is not because of this (actually we don't do anything in configure_file).<br>
I don't think it's superfluous anyway.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
<div class="im"><br>
>  include (KDE4Defaults)<br>
>  include (MacroWriteBasicCMakeVersionFile)<br>
>  include (MacroLibrary)<br>
<br>
</div>Why is the above stuff before these? It should be after it.</blockquote><div><br>I wanted to add it with the other find_packages. If you prefer to have it below I'll move it. It is not a problem.<br> <br></div>
<blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
<div><div></div><div class="h5"><br>
> -macro_log_feature(KOMPARE_INCLUDES "Kompare Part" "Enhance code generation user feedback" "" FALSE)<br>
> -if(ENABLE_KOMPARE)<br>
> -    message(STATUS "Kompare interfaces FOUND. Enabling Kompare Support ")<br>
> -    add_definitions(-DKOMPARE_ENABLED=true)<br>
> -else(ENABLE_KOMPARE)<br>
> -    message(STATUS "Kompare interfaces NOT FOUND. Kompare Support cannot be enabled")<br>
> -    add_definitions(-DKOMPARE_ENABLED=false)<br>
> -endif(ENABLE_KOMPARE)<br>
> -<br>
> -<br>
>  set( kdevplatformlanguage_LIB_UI<br>
>      codegen/ui/newclass.ui<br>
>      codegen/ui/overridevirtuals.ui<br>
> --- trunk/KDE/kdevplatform/language/codegen/komparesupport.cpp #1006579:1006580<br>
> @@ -18,8 +18,9 @@<br>
>   */<br>
><br>
>  #include "komparesupport.h"<br>
> +#include <config-kdevplatform.h><br>
><br>
> -#if KOMPARE_ENABLED == true<br>
> +#ifdef HAVE_KOMPARE<br>
><br>
>  #include <stddef.h><br>
>  #include <cstddef><br>
> --- trunk/KDE/kdevplatform/language/codegen/komparesupport.h #1006579:1006580<br>
> @@ -20,6 +20,8 @@<br>
>  #ifndef KOMPARESUPPORT_H<br>
>  #define KOMPARESUPPORT_H<br>
><br>
> +#include <config-kdevplatform.h><br>
> +<br>
>  class QWidget;<br>
>  class QString;<br>
><br>
> @@ -31,8 +33,12 @@<br>
>  class KompareWidgets<br>
>  {<br>
>    public:<br>
> -    static const bool enabled = KOMPARE_ENABLED;<br>
> -<br>
> +#ifdef WITH_KOMPARE<br>
> +    static const bool enabled = 1;<br>
> +#else<br>
> +    static const bool enabled = 0;<br>
> +#endif<br>
> +<br>
>      KompareWidgets();<br>
>      ~KompareWidgets();<br>
<br>
</div></div>This is _exactly_ what I do not want. the amount of ifdefs inside our<br>
code should be kept to the absolute possible mininmum. Not to mention<br>
that bool's actually should have proper true/fals initialization -<br>
without relying on the compiler converting int to bool. </blockquote><div> </div><div>Fixed, my fault. Actually here I think that komparewidget shouldn't be compiled at all, it was my next step.<br><br></div><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<br>
<br>
IMHO this should be reverted. <br></blockquote><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><br>
<br>
Andreas<br>
<font color="#888888"><br>
--<br>
Of course you have a purpose -- to find a purpose.<br>
<br>
_______________________________________________<br>
KDevelop-devel mailing list<br>
<a href="mailto:KDevelop-devel@kdevelop.org">KDevelop-devel@kdevelop.org</a><br>
<a href="https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel" target="_blank">https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel</a><br>
</font></blockquote></div><br>