Allow spaces in RunProgramArguments

This changes the syntax of the RunProgramArguments entry in
the config.xml. <Argument> sub tags need to be used to specify
the RunProgramArguments now.

Task-number: QTIFW-227
Change-Id: Ife007bf660cdcf4ff5e8e31bb9bb5ac2e5037616
Reviewed-by: Leena Miettinen <riitta-leena.miettinen@theqtcompany.com>
Reviewed-by: Kai Koehne <kai.koehne@theqtcompany.com>

11 files changed, 134 insertions, 26 deletions

diff --git a/doc/examples/config.xml b/doc/examples/config.xml
index c3b79486a..9b40f42bd 100644
--- a/doc/examples/config.xml
+++ b/doc/examples/config.xml
@@ -10,7 +10,9 @@
     <Logo>logo.png</Logo>
     <Watermark>watermark.png</Watermark>
     <RunProgram></RunProgram>
-    <RunProgramArguments></RunProgramArguments>
+    <RunProgramArguments>
+        <Argument></Argument>
+    </RunProgramArguments>
     <RunProgramDescription></RunProgramDescription>
     <StartMenuDir>Some Application Entry Dir</StartMenuDir>
     <UninstallerName>SDKMaintenanceTool</UninstallerName>
diff --git a/doc/installerfw.qdoc b/doc/installerfw.qdoc
index a8cc7f531..c2a267dab 100644
--- a/doc/installerfw.qdoc
+++ b/doc/installerfw.qdoc
@@ -220,7 +220,8 @@
                 the action.
         \row
             \li RunProgramArguments
-            \li Arguments passed to the program specified in RunProgram.
+            \li Arguments passed to the program specified in RunProgram. You can add several
+                \c Argument elements that each specify an argument to \c {RunProgram}.
         \row
             \li RunProgramDescription
             \li Text shown next to the check box for running the program after
diff --git a/src/libs/installer/packagemanagercore.cpp b/src/libs/installer/packagemanagercore.cpp
index 6e4716b03..d97985e46 100644
--- a/src/libs/installer/packagemanagercore.cpp
+++ b/src/libs/installer/packagemanagercore.cpp
@@ -1726,6 +1726,19 @@ QString PackageManagerCore::value(const QString &key, const QString &defaultValu
 }
 
 /*!
+    \qmlmethod stringlist installer::values(string key, stringlist defaultValue = [])
+
+    Returns the installer value for \a key. If \a key is not known to the system, \a defaultValue is
+    returned. Additionally, on Windows, \a key can be a registry key.
+
+    \sa value
+*/
+QStringList PackageManagerCore::values(const QString &key, const QStringList &defaultValue) const
+{
+    return d->m_data.value(key, defaultValue).toStringList();
+}
+
+/*!
     \qmlmethod void installer::setValue(string key, string value)
 
     Sets the installer value for \a key to \a value.
diff --git a/src/libs/installer/packagemanagercore.h b/src/libs/installer/packagemanagercore.h
index e2a7f979e..507998b15 100644
--- a/src/libs/installer/packagemanagercore.h
+++ b/src/libs/installer/packagemanagercore.h
@@ -158,6 +158,7 @@ public:
     Q_INVOKABLE bool containsValue(const QString &key) const;
     Q_INVOKABLE void setValue(const QString &key, const QString &value);
     Q_INVOKABLE QString value(const QString &key, const QString &defaultValue = QString()) const;
+    Q_INVOKABLE QStringList values(const QString &key, const QStringList &defaultValue = QStringList()) const;
 
     //a way to have global flags share able from a component script to another one
     Q_INVOKABLE bool sharedFlag(const QString &key) const;
diff --git a/src/libs/installer/packagemanagergui.cpp b/src/libs/installer/packagemanagergui.cpp
index b85075e0e..a09f8090d 100644
--- a/src/libs/installer/packagemanagergui.cpp
+++ b/src/libs/installer/packagemanagergui.cpp
@@ -2441,9 +2441,9 @@ void FinishedPage::handleFinishClicked()
 {
     const QString program =
         packageManagerCore()->replaceVariables(packageManagerCore()->value(scRunProgram));
-    const QStringList args = packageManagerCore()->replaceVariables(
-        packageManagerCore()->value(scRunProgramArguments)).split(QLatin1Char(' '),
-        QString::SkipEmptyParts);
+
+    const QStringList args = packageManagerCore()->replaceVariables(packageManagerCore()
+        ->values(scRunProgramArguments));
     if (!m_runItCheckBox->isChecked() || program.isEmpty())
         return;
 
diff --git a/src/libs/installer/settings.cpp b/src/libs/installer/settings.cpp
index b47076ad7..6006e6349 100644
--- a/src/libs/installer/settings.cpp
+++ b/src/libs/installer/settings.cpp
@@ -40,6 +40,7 @@
 #include <QtCore/QFileInfo>
 #include <QtCore/QStringList>
 
+#include <QRegularExpression>
 #include <QXmlStreamReader>
 
 using namespace QInstaller;
@@ -90,23 +91,45 @@ static void raiseError(QXmlStreamReader &reader, const QString &error, Settings:
     }
 }
 
-static QStringList readTranslations(QXmlStreamReader &reader, Settings::ParseMode parseMode)
+static QStringList readArgumentAttributes(QXmlStreamReader &reader, Settings::ParseMode parseMode,
+                                          const QString &tagName, bool lc = false)
 {
-    QStringList translations;
-    while (reader.readNextStartElement()) {
-        if (reader.name() == QLatin1String("Translation")) {
-            translations.append(reader.readElementText().toLower());
-        } else {
-            raiseError(reader, QString::fromLatin1("Unexpected element '%1'.").arg(reader.name().toString()),
-                parseMode);
-        }
+    QStringList arguments;
 
-        if (!reader.attributes().isEmpty()) {
-            raiseError(reader, QString::fromLatin1("Unexpected attribute for element '%1'.").arg(reader
-                .name().toString()), parseMode);
+    while (QXmlStreamReader::TokenType token = reader.readNext()) {
+        switch (token) {
+            case QXmlStreamReader::StartElement: {
+                if (!reader.attributes().isEmpty()) {
+                    raiseError(reader, QString::fromLatin1("Unexpected attribute for element '%1'.")
+                        .arg(reader.name().toString()), parseMode);
+                    return arguments;
+                } else {
+                    if (reader.name().toString() == tagName) {
+                        (lc) ? arguments.append(reader.readElementText().toLower()) :
+                               arguments.append(reader.readElementText());
+                    } else {
+                        raiseError(reader, QString::fromLatin1("Unexpected element '%1'.").arg(reader.name()
+                            .toString()), parseMode);
+                        return arguments;
+                    }
+                }
+            }
+            break;
+            case QXmlStreamReader::Characters: {
+                if (reader.isWhitespace())
+                    continue;
+                arguments.append(reader.text().toString().split(QRegularExpression(QLatin1String("\\s+")),
+                    QString::SkipEmptyParts));
+            }
+            break;
+            case QXmlStreamReader::EndElement: {
+                return arguments;
+            }
+            default:
+            break;
         }
     }
-    return translations;
+    return arguments;
 }
 
 static QSet<Repository> readRepositories(QXmlStreamReader &reader, bool isDefault, Settings::ParseMode parseMode)
@@ -246,9 +269,11 @@ Settings Settings::fromFileAndPrefix(const QString &path, const QString &prefix,
         if (s.d->m_data.contains(name))
             reader.raiseError(QString::fromLatin1("Element '%1' has been defined before.").arg(name));
 
-         if (name == scTranslations) {
-            s.setTranslations(readTranslations(reader, parseMode));
-         } else if (name == scRemoteRepositories) {
+        if (name == scTranslations) {
+            s.setTranslations(readArgumentAttributes(reader, parseMode, QLatin1String("Translation"), true));
+        } else if (name == scRunProgramArguments) {
+            s.setRunProgramArguments(readArgumentAttributes(reader, parseMode, QLatin1String("Argument")));
+        } else if (name == scRemoteRepositories) {
             s.addDefaultRepositories(readRepositories(reader, true, parseMode));
         } else {
             s.d->m_data.insert(name, reader.readElementText(QXmlStreamReader::SkipChildElements));
@@ -386,11 +411,20 @@ QString Settings::runProgram() const
     return d->m_data.value(scRunProgram).toString();
 }
 
-QString Settings::runProgramArguments() const
+QStringList Settings::runProgramArguments() const
 {
-    return d->m_data.value(scRunProgramArguments).toString();
+    const QVariant variant = d->m_data.values(scRunProgramArguments);
+    if (variant.canConvert<QStringList>())
+        return variant.value<QStringList>();
+    return QStringList();
 }
 
+void Settings::setRunProgramArguments(const QStringList &arguments)
+{
+    d->m_data.insert(scRunProgramArguments, arguments);
+}
+
+
 QString Settings::runProgramDescription() const
 {
     return d->m_data.value(scRunProgramDescription).toString();
@@ -615,6 +649,5 @@ QStringList Settings::translations() const
 
 void Settings::setTranslations(const QStringList &translations)
 {
-    d->m_data.remove(scTranslations);
     d->m_data.insert(scTranslations, translations);
 }
diff --git a/src/libs/installer/settings.h b/src/libs/installer/settings.h
index ea33b24aa..2128d6383 100644
--- a/src/libs/installer/settings.h
+++ b/src/libs/installer/settings.h
@@ -95,8 +95,10 @@ public:
     QString applicationVersion() const;
 
     QString runProgram() const;
-    QString runProgramArguments() const;
+    QStringList runProgramArguments() const;
+    void setRunProgramArguments(const QStringList &arguments);
     QString runProgramDescription() const;
+
     QString startMenuDir() const;
     QString targetDir() const;
     QString adminTargetDir() const;
diff --git a/tests/auto/installer/settings/data/unexpectedattribute_config.xml b/tests/auto/installer/settings/data/unexpectedattribute_config.xml
new file mode 100644
index 000000000..67cbb4eea
--- /dev/null
+++ b/tests/auto/installer/settings/data/unexpectedattribute_config.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Installer>
+    <Name>Your application</Name>
+    <Version>1.2.3</Version>
+    <RunProgramArguments>
+       <Argument foo="bar">Test</Argument>
+    </RunProgramArguments>
+</Installer>
diff --git a/tests/auto/installer/settings/data/unexpectedtag_config.xml b/tests/auto/installer/settings/data/unexpectedtag_config.xml
new file mode 100644
index 000000000..f8bc87b60
--- /dev/null
+++ b/tests/auto/installer/settings/data/unexpectedtag_config.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<Installer>
+    <Name>Your application</Name>
+    <Version>1.2.3</Version>
+    <RunProgramArguments>
+       <Foo>Bar</Foo>
+    </RunProgramArguments>
+</Installer>
diff --git a/tests/auto/installer/settings/settings.qrc b/tests/auto/installer/settings/settings.qrc
index 2dbffd927..c86847993 100644
--- a/tests/auto/installer/settings/settings.qrc
+++ b/tests/auto/installer/settings/settings.qrc
@@ -7,5 +7,7 @@
         <file>data/tutorial_config.xml</file>
         <file>data/unknown_element_config.xml</file>
         <file>data/minimal_config_tag_defaults.xml</file>
+        <file>data/unexpectedtag_config.xml</file>
+        <file>data/unexpectedattribute_config.xml</file>
     </qresource>
 </RCC>
diff --git a/tests/auto/installer/settings/tst_settings.cpp b/tests/auto/installer/settings/tst_settings.cpp
index 6567df11f..54d6dc66e 100644
--- a/tests/auto/installer/settings/tst_settings.cpp
+++ b/tests/auto/installer/settings/tst_settings.cpp
@@ -21,6 +21,8 @@ private slots:
     void loadUnknownElementConfigInStrictParseMode();
     void loadUnknownElementConfigInRelaxedParseMode();
     void loadMinimalConfigTagDefaults();
+    void loadUnexpectedAttributeConfig();
+    void loadUnexpectedTagConfig();
 };
 
 void tst_Settings::loadTutorialConfig()
@@ -57,7 +59,7 @@ void tst_Settings::loadTutorialConfig()
     QCOMPARE(settings.wizardStyle(), QString());
     QCOMPARE(settings.titleColor(), QString());
     QCOMPARE(settings.runProgram(), QString());
-    QCOMPARE(settings.runProgramArguments(), QString());
+    QCOMPARE(settings.runProgramArguments(), QStringList());
     QCOMPARE(settings.runProgramDescription(), QString());
     QCOMPARE(settings.adminTargetDir(), QString());
     QCOMPARE(settings.removeTargetDir(), QLatin1String("true"));
@@ -169,6 +171,42 @@ void tst_Settings::loadMinimalConfigTagDefaults()
     QCOMPARE(settings.maintenanceToolIniFile(), QLatin1String("maintenancetool.ini"));
 }
 
+void tst_Settings::loadUnexpectedAttributeConfig()
+{
+    QTest::ignoreMessage(QtDebugMsg, "create Error-Exception: \"Error in "
+        ":///data/unexpectedattribute_config.xml, line 6, column 27: Unexpected attribute "
+        "for element 'Argument'.\" ");
+
+    try {
+        Settings::fromFileAndPrefix(":///data/unexpectedattribute_config.xml", ":///data");
+    } catch (const Error &error) {
+        QCOMPARE(error.message(), QLatin1String("Error in :///data/unexpectedattribute_config.xml,"
+           " line 6, column 27: Unexpected attribute for element 'Argument'."));
+        return;
+    }
+    QFAIL("No exception thrown");
+
+    return;
+}
+
+void tst_Settings::loadUnexpectedTagConfig()
+{
+    QTest::ignoreMessage(QtDebugMsg, "create Error-Exception: \"Error in "
+        ":///data/unexpectedtag_config.xml, line 6, column 12: Unexpected element 'Foo'.\" ");
+
+    try {
+        Settings::fromFileAndPrefix(":///data/unexpectedtag_config.xml", ":///data");
+    } catch (const Error &error) {
+        QCOMPARE(error.message(), QLatin1String("Error in :///data/unexpectedtag_config.xml,"
+           " line 6, column 12: Unexpected element 'Foo'."));
+        return;
+    }
+    QFAIL("No exception thrown");
+
+    return;
+}
+
+
 QTEST_MAIN(tst_Settings)
 
 #include "tst_settings.moc"