Git commit 387fbca93c1d5cfd1840bf33ebf50fec867d32f9 by Milian Wolff.
Committed on 23/02/2014 at 19:07.
Pushed by mwolff into branch 'master'.

Fix various issues with the QMake build dir configuration.

The selected values where not saved properly. Also KConfig values
where missing when handled automatically by the KCM. And the code
in general duplicated too much or was just plain confusing.

This works much better in my tests now.

M  +44   -74   qmakebuilddirchooser.cpp
M  +12   -7    qmakebuilddirchooser.h
M  +6    -7    qmakebuilddirchooserdialog.cpp
M  +1    -7    qmakebuilder/qmakebuilderconfig.kcfg
M  +37   -24   qmakebuilder/qmakebuilderpreferences.cpp
M  +2    -0    qmakebuilder/qmakebuilderpreferences.h
M  +0    -2    qmakefile.h
M  +0    -1    qmakeprojectfile.cpp
M  +2    -2    tests/test_qmakefile.cpp

http://commits.kde.org/kdev-qmake/387fbca93c1d5cfd1840bf33ebf50fec867d32f9

diff --git a/qmakebuilddirchooser.cpp b/qmakebuilddirchooser.cpp
index 0022b8d..432eeba 100644
--- a/qmakebuilddirchooser.cpp
+++ b/qmakebuilddirchooser.cpp
@@ -26,6 +26,8 @@
 #include <KDebug>
 #include <KMessageWidget>
 =

+#include <project/path.h>
+
 #include <project/interfaces/ibuildsystemmanager.h>
 #include <project/interfaces/iprojectbuilder.h>
 =

@@ -33,6 +35,8 @@
 #include <interfaces/iruncontroller.h>
 #include <interfaces/iproject.h>
 =

+using namespace KDevelop;
+
 QMakeBuildDirChooser::QMakeBuildDirChooser(QWidget *parent, KDevelop::IPro=
ject* project)
     :  Ui::QMakeBuildDirChooser(), m_project(project)
 {
@@ -43,7 +47,7 @@ QMakeBuildDirChooser::QMakeBuildDirChooser(QWidget *paren=
t, KDevelop::IProject*
     status->setMessageType(KMessageWidget::Error);
     status->setWordWrap(true);
     kcfg_buildDir->setMode(KFile::Directory | KFile::LocalOnly);
-    kcfg_installPrefix->setMode(KFile::Directory);
+    kcfg_installPrefix->setMode(KFile::Directory | KFile::LocalOnly);
     kcfg_qmakeBin->setMode(KFile::File | KFile::ExistingOnly | KFile::Loca=
lOnly);
 }
 =

@@ -53,29 +57,28 @@ QMakeBuildDirChooser::~QMakeBuildDirChooser()
     //saveConfig();
 }
 =

-
 void QMakeBuildDirChooser::saveConfig()
 {
-    kDebug() << "Writing config";
-    KConfigGroup cg(m_project->projectConfiguration(), QMakeConfig::CONFIG=
_GROUP);
+    KConfigGroup config =3D KConfigGroup(m_project->projectConfiguration()=
, QMakeConfig::CONFIG_GROUP).group(buildDir());
+    saveConfig(config);
+}
 =

-    // Write entries to builds group
-    // current entries are handled by KConfig in config dialog, import dia=
log handles it itself
-    KConfigGroup currentBuild =3D cg.group(buildDir().toLocalFile());
-    currentBuild.writeEntry(QMakeConfig::QMAKE_BINARY, qmakeBin().toLocalF=
ile());
-    currentBuild.writeEntry(QMakeConfig::INSTALL_PREFIX, installPrefix().t=
oLocalFile());
-    currentBuild.writeEntry(QMakeConfig::EXTRA_ARGUMENTS, extraArgs());
-    currentBuild.writeEntry(QMakeConfig::BUILD_TYPE, buildType());
-    currentBuild.sync();
-    cg.sync();
+void QMakeBuildDirChooser::saveConfig(KConfigGroup& config)
+{
+    kDebug() << "Writing config for" << buildDir() << "to config" << confi=
g.name();
+
+    config.writeEntry(QMakeConfig::QMAKE_BINARY, qmakeBin());
+    config.writeEntry(QMakeConfig::INSTALL_PREFIX, installPrefix());
+    config.writeEntry(QMakeConfig::EXTRA_ARGUMENTS, extraArgs());
+    config.writeEntry(QMakeConfig::BUILD_TYPE, buildType());
+    config.sync();
 }
 =

 void QMakeBuildDirChooser::loadConfig()
 {
-    KUrl proposedBuildUrl( m_project->folder().toLocalFile() + "/build" );
-    proposedBuildUrl.cleanPath();
+    Path proposedBuildPath( m_project->path(), "build" );
     KConfigGroup cg(m_project->projectConfiguration(), QMakeConfig::CONFIG=
_GROUP);
-    loadConfig(cg.readEntry(QMakeConfig::BUILD_FOLDER, proposedBuildUrl.to=
LocalFile()));
+    loadConfig(cg.readEntry(QMakeConfig::BUILD_FOLDER, proposedBuildPath.t=
oLocalFile()));
 }
 =

 void QMakeBuildDirChooser::loadConfig(const QString& config)
@@ -85,10 +88,10 @@ void QMakeBuildDirChooser::loadConfig(const QString& co=
nfig)
     const KConfigGroup build =3D cg.group(config);
 =

     // sets values into fields
-    setQmakeBin( KUrl(QMakeConfig::qmakeBinary(m_project)) );
-    setBuildDir( KUrl(config) );
-    setInstallPrefix( KUrl(build.readEntry(QMakeConfig::INSTALL_PREFIX, QS=
tring())) );
-    setExtraArgs( build.readEntry(QMakeConfig::EXTRA_ARGUMENTS, QString())=
);
+    setQmakeBin( QMakeConfig::qmakeBinary(m_project) );
+    setBuildDir( config );
+    setInstallPrefix( build.readEntry(QMakeConfig::INSTALL_PREFIX, QString=
()) );
+    setExtraArgs( build.readEntry(QMakeConfig::EXTRA_ARGUMENTS, QString())=
 );
     setBuildType( build.readEntry(QMakeConfig::BUILD_TYPE, 0) );
 }
 =

@@ -96,31 +99,18 @@ bool QMakeBuildDirChooser::isValid(QString *message)
 {
     bool valid =3D true;
     QString msg;
-    if(qmakeBin().isEmpty())
-    {
+    if(qmakeBin().isEmpty()) {
         msg =3D i18n("Please specify path to QMake binary.");
         valid =3D false;
-    }
-    else if(!qmakeBin().isValid())
-    {
-        msg =3D  i18n("QMake binary path is invalid.");
-        valid =3D false;
-    }
-    else if(!qmakeBin().isLocalFile())
-    {
-        msg =3D i18n("QMake binary must be a local path.");
-        valid =3D false;
-    }
-    else
-    {
-        QFileInfo info(qmakeBin().toLocalFile());
-        if(!info.isFile())
-        {
+    } else {
+        QFileInfo info(qmakeBin());
+        if(!info.exists()) {
+            msg =3D i18n("Qmake binary \"%1\" does not exist.", qmakeBin()=
);
+            valid =3D false;
+        } else if(!info.isFile()) {
             msg =3D i18n("QMake binary is not a file.");
             valid =3D false;
-        }
-        else if(!info.isExecutable())
-        {
+        } else if(!info.isExecutable()) {
             msg =3D i18n("QMake binary is not executable.");
             valid =3D false;
         } else {
@@ -140,27 +130,6 @@ bool QMakeBuildDirChooser::isValid(QString *message)
         msg =3D i18n("Please specify a build folder.");
         valid =3D false;
     }
-    else if(!buildDir().isValid())
-    {
-        msg =3D i18n("Build folder is invalid.");
-        valid =3D false;
-    }
-    else if(!buildDir().isLocalFile())
-    {
-        msg =3D i18n("Build folder must be a local path.");
-        valid =3D false;
-    }
-
-    if(!installPrefix().isEmpty() && !installPrefix().isValid())
-    {
-        msg =3D i18n("Install prefix is invalid (may also be left empty)."=
);
-        valid =3D false;
-    }
-    if(!installPrefix().isEmpty() && !installPrefix().isLocalFile())
-    {
-        msg =3D i18n("Install prefix must be a local path (may also be lef=
t empty).");
-        valid =3D false;
-    }
 =

     if (message)
     {
@@ -176,43 +145,44 @@ bool QMakeBuildDirChooser::isValid(QString *message)
     return valid;
 }
 =

-KUrl QMakeBuildDirChooser::qmakeBin() const
+QString QMakeBuildDirChooser::qmakeBin() const
 {
-    return kcfg_qmakeBin->url();
+    return kcfg_qmakeBin->url().toLocalFile();
 }
 =

-KUrl QMakeBuildDirChooser::buildDir() const
+QString QMakeBuildDirChooser::buildDir() const
 {
-    return kcfg_buildDir->url();
+    return kcfg_buildDir->url().toLocalFile();
 }
 =

-KUrl QMakeBuildDirChooser::installPrefix() const
+QString QMakeBuildDirChooser::installPrefix() const
 {
-    return kcfg_installPrefix->url();
+    return kcfg_installPrefix->url().toLocalFile();
 }
 =

 int QMakeBuildDirChooser::buildType() const
 {
     return kcfg_buildType->currentIndex();
 }
+
 QString QMakeBuildDirChooser::extraArgs() const
 {
     return kcfg_extraArgs->text();
 }
 =

-void QMakeBuildDirChooser::setQmakeBin(const KUrl& url)
+void QMakeBuildDirChooser::setQmakeBin(const QString& binary)
 {
-    kcfg_qmakeBin->setUrl(url);
+    kcfg_qmakeBin->setUrl(KUrl::fromPath(binary));
 }
 =

-void QMakeBuildDirChooser::setBuildDir(const KUrl& url)
+void QMakeBuildDirChooser::setBuildDir(const QString& buildDir)
 {
-    kcfg_buildDir->setUrl(url);
+    kcfg_buildDir->setUrl(KUrl::fromPath(buildDir));
 }
 =

-void QMakeBuildDirChooser::setInstallPrefix(const KUrl& url)
+void QMakeBuildDirChooser::setInstallPrefix(const QString& prefix)
 {
-    kcfg_installPrefix->setUrl(url);
+    kcfg_installPrefix->setUrl(KUrl::fromPath(prefix));
 }
 =

 void QMakeBuildDirChooser::setBuildType(int type)
diff --git a/qmakebuilddirchooser.h b/qmakebuilddirchooser.h
index c7daee9..4b11399 100644
--- a/qmakebuilddirchooser.h
+++ b/qmakebuilddirchooser.h
@@ -47,11 +47,16 @@ public:
     bool isValid(QString *message=3D0);
 =

     /**
-     * Saves current date to builds hash (not to current values).
+     * Saves current data to this build dir's config group (not to current=
 values).
      */
     virtual void saveConfig();
 =

     /**
+     * Saves current data to the given config group.
+     */
+    void saveConfig(KConfigGroup& config);
+
+    /**
      * Loads current config (or a new one) into fields.
      */
     void loadConfig();
@@ -61,15 +66,15 @@ public:
      */
     void loadConfig(const QString &config);
 =

-    KUrl qmakeBin() const;
-    KUrl buildDir() const;
-    KUrl installPrefix() const;
+    QString qmakeBin() const;
+    QString buildDir() const;
+    QString installPrefix() const;
     int buildType() const;
     QString extraArgs() const;
 =

-    void setQmakeBin(const KUrl &url);
-    void setBuildDir(const KUrl &url);
-    void setInstallPrefix(const KUrl &url);
+    void setQmakeBin(const QString &binary);
+    void setBuildDir(const QString &buildDir);
+    void setInstallPrefix(const QString &prefix);
     void setBuildType(int type);
     void setExtraArgs(const QString &args);
 =

diff --git a/qmakebuilddirchooserdialog.cpp b/qmakebuilddirchooserdialog.cpp
index b7ff49b..95d0a8f 100644
--- a/qmakebuilddirchooserdialog.cpp
+++ b/qmakebuilddirchooserdialog.cpp
@@ -53,14 +53,13 @@ QMakeBuildDirChooserDialog::~QMakeBuildDirChooserDialog=
()
 =

 void QMakeBuildDirChooserDialog::saveConfig()
 {
-    KConfigGroup cg(m_project->projectConfiguration(), QMakeConfig::CONFIG=
_GROUP);
-    cg.writeEntry(QMakeConfig::QMAKE_BINARY, qmakeBin().toLocalFile());
-    cg.writeEntry(QMakeConfig::BUILD_FOLDER, buildDir().toLocalFile());
-    cg.writeEntry(QMakeConfig::INSTALL_PREFIX, installPrefix().toLocalFile=
());
-    cg.writeEntry(QMakeConfig::EXTRA_ARGUMENTS, extraArgs());
-    cg.writeEntry(QMakeConfig::BUILD_TYPE, buildType());
-    cg.sync();
+    // store this builds config
     QMakeBuildDirChooser::saveConfig();
+
+    // also save as current values
+    KConfigGroup config(m_project->projectConfiguration(), QMakeConfig::CO=
NFIG_GROUP);
+    QMakeBuildDirChooser::saveConfig(config);
+    config.writeEntry(QMakeConfig::BUILD_FOLDER, buildDir());
 }
 =

 =

diff --git a/qmakebuilder/qmakebuilderconfig.kcfg b/qmakebuilder/qmakebuild=
erconfig.kcfg
index 26efa87..4fe09ab 100644
--- a/qmakebuilder/qmakebuilderconfig.kcfg
+++ b/qmakebuilder/qmakebuilderconfig.kcfg
@@ -2,11 +2,5 @@
 <!DOCTYPE kcfg SYSTEM "http://www.kde.org/standards/kcfg/1.0/kcfg.dtd">
 <kcfg>
   <kcfgfile arg=3D"true"/>
-  <group name=3D"QMake_Builder">
-	  <entry name=3D"qmakeBin"      key=3D"QMake_Binary"    type=3D"Url"    />
-	  <entry name=3D"buildDir"      key=3D"Build_Folder"    type=3D"Url"    />
-	  <entry name=3D"installPrefix" key=3D"Install_Prefix"  type=3D"Url"    />
-	  <entry name=3D"buildType"     key=3D"Build_Type"      type=3D"Int"    />
-	  <entry name=3D"extraArgs"     key=3D"Extra_Arguments" type=3D"String" />
-  </group>
+  <!-- save/restore is handled manually -->
 </kcfg>
diff --git a/qmakebuilder/qmakebuilderpreferences.cpp b/qmakebuilder/qmakeb=
uilderpreferences.cpp
index 85c23f9..a6bcd64 100644
--- a/qmakebuilder/qmakebuilderpreferences.cpp
+++ b/qmakebuilder/qmakebuilderpreferences.cpp
@@ -59,13 +59,16 @@ QMakeBuilderPreferences::QMakeBuilderPreferences(QWidge=
t* parent, const QVariant
 =

     m_chooserUi =3D new QMakeBuildDirChooser(m_prefsUi->groupBox, project(=
));
     m_chooserUi->kcfg_buildDir->setEnabled(false);  // build directory MUS=
T NOT be changed here
-    connect(m_chooserUi->kcfg_qmakeBin, SIGNAL(textChanged(QString)), this=
, SLOT(validate()));
-    connect(m_chooserUi->kcfg_buildDir, SIGNAL(textChanged(QString)), this=
, SLOT(validate()));
-    connect(m_chooserUi->kcfg_installPrefix, SIGNAL(textChanged(QString)),=
 this, SLOT(validate()));
+    connect(m_chooserUi->kcfg_qmakeBin, SIGNAL(textChanged(QString)), this=
, SLOT(changed()));
+    connect(m_chooserUi->kcfg_buildDir, SIGNAL(textChanged(QString)), this=
, SLOT(changed()));
+    connect(m_chooserUi->kcfg_installPrefix, SIGNAL(textChanged(QString)),=
 this, SLOT(changed()));
+    connect(m_chooserUi->kcfg_buildType, SIGNAL(currentIndexChanged(int)),=
 this, SLOT(changed()));
+    connect(m_chooserUi->kcfg_extraArgs, SIGNAL(textChanged(QString)), thi=
s, SLOT(changed()));
     l->addWidget( w );
     addConfig( QMakeBuilderSettings::self(), w );
 =

     connect(m_prefsUi->buildDirCombo, SIGNAL(currentIndexChanged(QString))=
, this, SLOT(loadOtherConfig(QString)));
+    connect(m_prefsUi->buildDirCombo, SIGNAL(currentIndexChanged(QString))=
, this, SLOT(changed()));
     connect(m_prefsUi->addButton, SIGNAL(pressed()), this, SLOT(addBuildCo=
nfig()));
     connect(m_prefsUi->removeButton, SIGNAL(pressed()), this, SLOT(removeB=
uildConfig()));
     connect(this, SIGNAL(changed(bool)), this, SLOT(validate()));
@@ -83,17 +86,19 @@ QMakeBuilderPreferences::~QMakeBuilderPreferences()
 void QMakeBuilderPreferences::load()
 {
     kDebug() << "loading data";
-    KCModule::load();
     // refresh combobox
     KConfigGroup cg(project()->projectConfiguration(), QMakeConfig::CONFIG=
_GROUP);
-    KUrl buildPath =3D cg.readEntry(QMakeConfig::BUILD_FOLDER, KUrl());
+    const QString buildPath =3D cg.readEntry(QMakeConfig::BUILD_FOLDER, QS=
tring());
 =

     // update build list (this will trigger loadOtherConfig if signals are=
 still connected)
     disconnect(m_prefsUi->buildDirCombo, SIGNAL(currentIndexChanged(QStrin=
g)), this, SLOT(loadOtherConfig(QString)));
     m_prefsUi->buildDirCombo->clear();
     m_prefsUi->buildDirCombo->insertItems(0, cg.groupList());
-    m_prefsUi->buildDirCombo->setCurrentItem(buildPath.toLocalFile());
-    kDebug() << "Loaded" << m_prefsUi->buildDirCombo->count() << (m_prefsU=
i->buildDirCombo->count() > 1);
+    if (m_prefsUi->buildDirCombo->contains(buildPath)) {
+        m_prefsUi->buildDirCombo->setCurrentItem(buildPath);
+        m_chooserUi->loadConfig(buildPath);
+    }
+    kDebug() << "Loaded" << cg.groupList() << buildPath;
     m_prefsUi->removeButton->setEnabled(m_prefsUi->buildDirCombo->count() =
> 1);
     connect(m_prefsUi->buildDirCombo, SIGNAL(currentIndexChanged(QString))=
, this, SLOT(loadOtherConfig(QString)));
 =

@@ -108,9 +113,11 @@ void QMakeBuilderPreferences::save()
 =

     if(m_chooserUi->isValid(&errormsg))
     {
-        // data is valid: save
-        KCModule::save();
+        // data is valid: save, once in the build dir's data and also as c=
urrent data
         m_chooserUi->saveConfig();
+        KConfigGroup config(project()->projectConfiguration(), QMakeConfig=
::CONFIG_GROUP);
+        m_chooserUi->saveConfig(config);
+        config.writeEntry(QMakeConfig::BUILD_FOLDER, m_chooserUi->buildDir=
());
     }
     else
     {
@@ -129,33 +136,38 @@ void QMakeBuilderPreferences::loadOtherConfig(const Q=
String& config)
 {
     kDebug() << "Loding config "<<config;
     kDebug() << "Change state " << managedWidgetChangeState();
-    // changes must be saved before switch
-    if(managedWidgetChangeState())
-    {
-        int ret =3D KMessageBox::questionYesNoCancel(this,
-                      i18n("Current changes will be lost. Would you want t=
o save them?"));
-        if(ret =3D=3D KMessageBox::Yes)
-        {
-            save();
-        }
-        else if (ret =3D=3D KMessageBox::Cancel)
-        {
-            return;
-        }
+    if (!verifyChanges()) {
+        return;
     }
     m_chooserUi->loadConfig(config);
     save();  // since current config has changed, it must be saved immedia=
teley
-    load();  // cause the combobox to be recalculated
 }
 =

+bool QMakeBuilderPreferences::verifyChanges()
+{
+    // changes must be saved before switch
+    if (managedWidgetChangeState()) {
+        int ret =3D KMessageBox::questionYesNoCancel(this, i18n("Current c=
hanges will be lost. Would you want to save them?"));
+        if (ret =3D=3D KMessageBox::Yes) {
+            save();
+        } else if (ret =3D=3D KMessageBox::Cancel) {
+            return false;
+        }
+    }
+    return true;
+}
 =

 void QMakeBuilderPreferences::addBuildConfig()
 {
+    if (!verifyChanges()) {
+        return;
+    }
     kDebug() << "Adding a new config.";
     // for more simpicity, just launch regular dialog
     QMakeBuildDirChooserDialog *dlg =3D new QMakeBuildDirChooserDialog(pro=
ject());
     if(dlg->exec() =3D=3D QDialog::Accepted) {
-        loadOtherConfig(dlg->buildDir().toLocalFile());
+        m_prefsUi->buildDirCombo->setCurrentItem(dlg->buildDir(), true);
+        m_prefsUi->removeButton->setEnabled(m_prefsUi->buildDirCombo->coun=
t() > 1);
         //TODO run qmake
     }
 }
@@ -167,6 +179,7 @@ void QMakeBuilderPreferences::removeBuildConfig()
     KConfigGroup cg(project()->projectConfiguration(), QMakeConfig::CONFIG=
_GROUP);
 =

     m_prefsUi->buildDirCombo->removeItem(m_prefsUi->buildDirCombo->current=
Index());
+    m_prefsUi->removeButton->setEnabled(m_prefsUi->buildDirCombo->count() =
> 1);
     cg.group(removed).deleteGroup(KConfigBase::Persistent);
 =

     if(QDir(removed).exists())
diff --git a/qmakebuilder/qmakebuilderpreferences.h b/qmakebuilder/qmakebui=
lderpreferences.h
index 6b7090f..b53ba54 100644
--- a/qmakebuilder/qmakebuilderpreferences.h
+++ b/qmakebuilder/qmakebuilderpreferences.h
@@ -51,6 +51,8 @@ public slots:
     void validate();
 =

 private:
+    bool verifyChanges();
+
     Ui::QMakeConfig* m_prefsUi;
     QMakeBuildDirChooser* m_chooserUi;
 };
diff --git a/qmakefile.h b/qmakefile.h
index c9d57b0..a35d135 100644
--- a/qmakefile.h
+++ b/qmakefile.h
@@ -25,8 +25,6 @@
 #include <QtCore/QStack>
 #include <QtCore/QStringList>
 =

-#include <KUrl>
-
 #include "qmakefilevisitor.h"
 =

 namespace KDevelop
diff --git a/qmakeprojectfile.cpp b/qmakeprojectfile.cpp
index 0bb48b7..1d42f12 100644
--- a/qmakeprojectfile.cpp
+++ b/qmakeprojectfile.cpp
@@ -25,7 +25,6 @@
 #include <QtCore/QDir>
 =

 #include <kprocess.h>
-#include <kurl.h>
 #include <kdebug.h>
 =

 #include "parser/ast.h"
diff --git a/tests/test_qmakefile.cpp b/tests/test_qmakefile.cpp
index 3364a5f..4748b34 100644
--- a/tests/test_qmakefile.cpp
+++ b/tests/test_qmakefile.cpp
@@ -554,8 +554,8 @@ void TestQMakeFile::globbing()
     QVERIFY(pro.read());
 =

     QStringList actual;
-    foreach(const KUrl& url, pro.files()) {
-        actual << url.pathOrUrl().remove(tempDir.name());
+    foreach(QString path, pro.files()) {
+        actual << path.remove(tempDir.name());
     }
     qSort(actual);
     qSort(matches);