KDE/kdevplatform

[Aleix Pol]

Actually here, I think that we shouldn't let KDevelop to be compiled without
Kompare.
It's a very powerful tool for code development and I don't know if we want
to have a KDevelop version without it.

On Tue, Aug 4, 2009 at 2:01 AM, Aleix Pol <aleixpol at kde.org> wrote:

>
>
> On Tue, Aug 4, 2009 at 1:23 AM, Andreas Pakulat <apaku at gmx.de> wrote:
>
>> On 03.08.09 22:57:29, Aleix Pol Gonzalez wrote:
>> > SVN commit 1006580 by apol:
>> >
>> > Properly look for Kompare, the CMake way.
>>
>> Why? IMHO its just unnecessary overhead compared to what "finding
>> kompare" actually means. All we need is find 1 single header file.
>
> Yes, and in CMake we find using find_package.
> I can change to find file if you prefer.
>
>
>>
>>
>> Apart from that:
>>
>> > +macro_log_feature(KOMPARE_FOUND "Kompare" "KPart to view file
>> differences"
>> > +                        "http://www.caffeinated.me.uk/kompare/" FALSE
>> ""
>> > +                        "Required for difference checking")
>>
>> This website is completely outdated, if we put a webpage here it should
>> at least point to the right sources and list the current developers.
>
>
> Ok, I can change to: http://www.google.es/search?q=kompare
> I agree we should specify it's on kdesdk. I'll add it later
>
>
>>
>> > +configure_file( ${CMAKE_CURRENT_SOURCE_DIR}/config-kdevplatform.h.cmake
>> > +                ${CMAKE_CURRENT_BINARY_DIR}/config-kdevplatform.h )
>> > +
>>
>> Even more code to execute.
>
>
> 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).
> I don't think it's superfluous anyway.
>
>
>>
>> >  include (KDE4Defaults)
>> >  include (MacroWriteBasicCMakeVersionFile)
>> >  include (MacroLibrary)
>>
>> Why is the above stuff before these? It should be after it.
>
>
> 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.
>
>
>>
>>
>> > -macro_log_feature(KOMPARE_INCLUDES "Kompare Part" "Enhance code
>> generation user feedback" "" FALSE)
>> > -if(ENABLE_KOMPARE)
>> > -    message(STATUS "Kompare interfaces FOUND. Enabling Kompare Support
>> ")
>> > -    add_definitions(-DKOMPARE_ENABLED=true)
>> > -else(ENABLE_KOMPARE)
>> > -    message(STATUS "Kompare interfaces NOT FOUND. Kompare Support
>> cannot be enabled")
>> > -    add_definitions(-DKOMPARE_ENABLED=false)
>> > -endif(ENABLE_KOMPARE)
>> > -
>> > -
>> >  set( kdevplatformlanguage_LIB_UI
>> >      codegen/ui/newclass.ui
>> >      codegen/ui/overridevirtuals.ui
>> > --- trunk/KDE/kdevplatform/language/codegen/komparesupport.cpp
>> #1006579:1006580
>> > @@ -18,8 +18,9 @@
>> >   */
>> >
>> >  #include "komparesupport.h"
>> > +#include <config-kdevplatform.h>
>> >
>> > -#if KOMPARE_ENABLED == true
>> > +#ifdef HAVE_KOMPARE
>> >
>> >  #include <stddef.h>
>> >  #include <cstddef>
>> > --- trunk/KDE/kdevplatform/language/codegen/komparesupport.h
>> #1006579:1006580
>> > @@ -20,6 +20,8 @@
>> >  #ifndef KOMPARESUPPORT_H
>> >  #define KOMPARESUPPORT_H
>> >
>> > +#include <config-kdevplatform.h>
>> > +
>> >  class QWidget;
>> >  class QString;
>> >
>> > @@ -31,8 +33,12 @@
>> >  class KompareWidgets
>> >  {
>> >    public:
>> > -    static const bool enabled = KOMPARE_ENABLED;
>> > -
>> > +#ifdef WITH_KOMPARE
>> > +    static const bool enabled = 1;
>> > +#else
>> > +    static const bool enabled = 0;
>> > +#endif
>> > +
>> >      KompareWidgets();
>> >      ~KompareWidgets();
>>
>> This is _exactly_ what I do not want. the amount of ifdefs inside our
>> code should be kept to the absolute possible mininmum. Not to mention
>> that bool's actually should have proper true/fals initialization -
>> without relying on the compiler converting int to bool.
>
>
> Fixed, my fault. Actually here I think that komparewidget shouldn't be
> compiled at all, it was my next step.
>
>
>>
>> IMHO this should be reverted.
>>
>
>>
>> Andreas
>>
>> --
>> Of course you have a purpose -- to find a purpose.
>>
>> _______________________________________________
>> KDevelop-devel mailing list
>> KDevelop-devel at kdevelop.org
>> https://barney.cs.uni-potsdam.de/mailman/listinfo/kdevelop-devel
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20090804/d280e203/attachment.html>