On 09.10.08 17:41:05, Aleix wrote:
> Hi!
> after a sad tuesday of dealing with not working and after chatting a bit
> with apaku we decided to take a new approach. I had to to some dirty work so
> maybe someone concerned my want to take a look at it first.
>
> It is not fully working yet, though... it still needs some love.
> +class RunSettings : public KDevelop::ProjectConfigSkeleton
This shouldn't be a project-specific setting. Run configs should be
cross-project IMHO. If somebody wants to have a certain run configuration
in his project we should provide a way to "move" it to that project.
> Index: runconfig.cpp
> ===================================================================
> --- runconfig.cpp (revision 0)
> +++ runconfig.cpp (revision 0)
> @@ -0,0 +1,48 @@
> +// This file is generated by kconfig_compiler from runconfig.kcfg.
> +// All changes you do to this file will be lost.
> +
> +#include "runconfig.h"
> +
> +#include <klocale.h>
> +
> +#include <kglobal.h>
> +#include <QtCore/QFile>
> +
> +#include <kdebug.h>
> +
> +RunSettings::RunSettings(const QString& groupPrefix, const QString& config )
> + : KDevelop::ProjectConfigSkeleton( config )
> +{
> + setCurrentGroup( groupPrefix+QLatin1String( "-Run Options" ) );
> +
> + mExecutableItem = new KConfigSkeleton::ItemUrl( currentGroup(), QLatin1String( "Executable" ), mExecutable );
> + mExecutableItem->setLabel( i18n("Executable") );
I guess this is largely copied from the kconfig_compiler generated code?
> Index: runpreferences.h
> ===================================================================
> namespace KDevelop
> {
>
> -class RunPreferences : public ProjectKCModule<RunSettings>
> +class RunPreferences : public KCModule
Oh, that shouldn't have worked at all, for a ProjectConfigSkeleton you
should always use a ProjectKCModule.
> private:
> - bool configNameValid(const QString& name);
> + void addTarget(const QString& name);
> + void removeTarget(int index);
Maybe "target" isn't quite right here, we also want to support running an
arbitrary executable thats not a target in some project.
> + KConfig m_config;
Better use a KSharedConfig::Ptr instead of a full copy.
> };
>
> }
> Index: asktargetname.h
Hmm, with so many files related to run-preferences I'd like to see a new
subdir inside shell/settings that contains all these files. No need to have
a CMakeLists.txt in it, but just group the files together in there.
> +class AskTargetName : public QDialog
Hmm, do we need that for just a name? A dialog that pops up from another
dialog is bad usability.
> Index: CMakeLists.txt
> ===================================================================
> --- CMakeLists.txt (revision 869604)
> +++ CMakeLists.txt (working copy)
> @@ -1,16 +1,6 @@
>
> include_directories(
> ${CMAKE_SOURCE_DIR}
> - ${CMAKE_SOURCE_DIR}/sublime
> - ${CMAKE_SOURCE_DIR}/interfaces
> - ${CMAKE_SOURCE_DIR}/project
> - ${CMAKE_SOURCE_DIR}/project/interfaces
> - ${CMAKE_SOURCE_DIR}/language
> - ${CMAKE_SOURCE_DIR}/language/editor
> - ${CMAKE_SOURCE_DIR}/language/backgroundparser
> - ${CMAKE_SOURCE_DIR}/language/interfaces
> - ${CMAKE_SOURCE_DIR}/shell
> - ${CMAKE_SOURCE_DIR}/util
Oh, seems I missed something back when I removed all the include-dirs-stuff
in kdevplatform :)
> +RunPreferences::RunPreferences( QWidget *parent, const QVariantList &args )
> + : KCModule( RunPreferencesFactory::componentData(), parent, args )
> + , m_currentRunTarget(0)
> + , m_deletingCurrentRunTarget(false)
> + , m_args(args)
> + , m_config(m_args[0].toString(), KConfig::SimpleConfig )
> +{
> + QVBoxLayout * l = new QVBoxLayout(this);
> + QHBoxLayout * h = new QHBoxLayout(this);
> +
> + buttonDeleteTarget=new QPushButton(KIcon("list-remove"), QString(), this);
> + QPushButton *buttonNewTarget=new QPushButton(KIcon("list-add"), QString(), this);
> +
> + buttonDeleteTarget->setSizePolicy(QSizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed));
> + buttonNewTarget->setSizePolicy(QSizePolicy(QSizePolicy::Fixed, QSizePolicy::Fixed));
IMHO the kcmodule itself should also use a .ui file, thats a lot nicer to
edit and manage in case we want to add stuff or re-arrange.
Rest looks fine. Didn't test the patch but except maybe some little bugs it
should work fine. So please commit.
Andreas
--
Your boyfriend takes chocolate from strangers.