CLI: Allow to mix options and property assignments

The strict separation we had before was annoying to users wanting to add
options to a previous command line using the shell history, because the
new option could not simply be appended if any property assignments were
present.

[ChangeLog] Command-line options do not have to precede property
assignments anymore.

Change-Id: I46fab716b2ff045adaf138db5194c3eba5b6818b
Reviewed-by: Joerg Bornemann <joerg.bornemann@qt.io>

2 files changed, 68 insertions, 77 deletions

diff --git a/src/app/qbs/parser/parsercommand.cpp b/src/app/qbs/parser/parsercommand.cpp
index 77ecb4ba8..69ee4db49 100644
--- a/src/app/qbs/parser/parsercommand.cpp
+++ b/src/app/qbs/parser/parsercommand.cpp
@@ -44,7 +44,7 @@
 #include <logging/translator.h>
 #include <tools/error.h>
 #include <tools/hostosinfo.h>
-#include <tools/set.h>
+#include <tools/qbsassert.h>
 
 #include <QtCore/qmap.h>
 
@@ -57,10 +57,8 @@ Command::~Command()
 
 void Command::parse(QStringList &input)
 {
-    parseOptions(input);
-    parseMore(input);
-    if (!input.isEmpty())
-        throwError(Tr::tr("Extraneous input '%1'").arg(input.join(QLatin1Char(' '))));
+    while (!input.empty())
+        parseNext(input);
 }
 
 bool Command::canResolve() const
@@ -68,13 +66,7 @@ bool Command::canResolve() const
     return supportedOptions().contains(CommandLineOption::FileOptionType);
 }
 
-void Command::addAllToAdditionalArguments(QStringList &input)
-{
-    while (!input.isEmpty())
-        addOneToAdditionalArguments(input.takeFirst());
-}
-
-void Command::addOneToAdditionalArguments(const QString &argument)
+void Command::parsePropertyAssignment(const QString &argument)
 {
     const auto throwError = [argument](const QString &msgTemplate) {
         ErrorInfo error(msgTemplate.arg(argument));
@@ -107,56 +99,59 @@ QList<CommandLineOption::Type> Command::actualSupportedOptions() const
     return options;
 }
 
-void Command::parseOptions(QStringList &input)
-{
-    Set<CommandLineOption *> usedOptions;
-    while (!input.isEmpty()) {
-        const QString optionString = input.first();
-        if (!optionString.startsWith(QLatin1Char('-')))
-            break;
-        if (optionString == QLatin1String("--"))
-            break;
-        input.removeFirst();
-        if (optionString.count() == 1)
-            throwError(Tr::tr("Empty options are not allowed."));
-
-        // Split up grouped short options.
-        if (optionString.at(1) != QLatin1Char('-') && optionString.count() > 2) {
-            QString parameter;
-            for (int i = optionString.count(); --i > 0;) {
-                const QChar c = optionString.at(i);
-                if (c.isDigit()) {
-                    parameter.prepend(c);
-                } else {
-                    if (!parameter.isEmpty()) {
-                        input.prepend(parameter);
-                        parameter.clear();
-                    }
-                    input.prepend(QLatin1Char('-') + c);
+void Command::parseOption(QStringList &input)
+{
+    const QString optionString = input.first();
+    QBS_CHECK(optionString.startsWith(QLatin1Char('-')));
+    input.removeFirst();
+    if (optionString.count() == 1)
+        throwError(Tr::tr("Empty options are not allowed."));
+
+    // Split up grouped short options.
+    if (optionString.at(1) != QLatin1Char('-') && optionString.count() > 2) {
+        QString parameter;
+        for (int i = optionString.count(); --i > 0;) {
+            const QChar c = optionString.at(i);
+            if (c.isDigit()) {
+                parameter.prepend(c);
+            } else {
+                if (!parameter.isEmpty()) {
+                    input.prepend(parameter);
+                    parameter.clear();
                 }
+                input.prepend(QLatin1Char('-') + c);
             }
-            if (!parameter.isEmpty())
-                throwError(Tr::tr("Unknown numeric option '%1'.").arg(parameter));
-            continue;
         }
+        if (!parameter.isEmpty())
+            throwError(Tr::tr("Unknown numeric option '%1'.").arg(parameter));
+        return;
+    }
 
-        bool matchFound = false;
-        foreach (const CommandLineOption::Type optionType, actualSupportedOptions()) {
-            CommandLineOption * const option = optionPool().getOption(optionType);
-            if (option->shortRepresentation() != optionString
-                    && option->longRepresentation() != optionString) {
-                continue;
-            }
-            if (usedOptions.contains(option) && !option->canAppearMoreThanOnce())
-                throwError(Tr::tr("Option '%1' cannot appear more than once.").arg(optionString));
-            option->parse(type(), optionString, input);
-            usedOptions << option;
-            matchFound = true;
-            break;
+    bool matchFound = false;
+    foreach (const CommandLineOption::Type optionType, actualSupportedOptions()) {
+        CommandLineOption * const option = optionPool().getOption(optionType);
+        if (option->shortRepresentation() != optionString
+                && option->longRepresentation() != optionString) {
+            continue;
         }
-        if (!matchFound)
-            throwError(Tr::tr("Unknown option '%1'.").arg(optionString));
+        if (m_usedOptions.contains(option) && !option->canAppearMoreThanOnce())
+            throwError(Tr::tr("Option '%1' cannot appear more than once.").arg(optionString));
+        option->parse(type(), optionString, input);
+        m_usedOptions << option;
+        matchFound = true;
+        break;
     }
+    if (!matchFound)
+        throwError(Tr::tr("Unknown option '%1'.").arg(optionString));
+}
+
+void Command::parseNext(QStringList &input)
+{
+    QBS_CHECK(!input.empty());
+    if (input.first().startsWith(QLatin1Char('-')))
+        parseOption(input);
+    else
+        parsePropertyAssignment(input.takeFirst());
 }
 
 QString Command::supportedOptionsDescription() const
@@ -182,11 +177,6 @@ void Command::throwError(const QString &reason)
     throw error;
 }
 
-void Command::parseMore(QStringList &input)
-{
-    addAllToAdditionalArguments(input);
-}
-
 
 QString ResolveCommand::shortDescription() const
 {
@@ -400,16 +390,14 @@ QList<CommandLineOption::Type> RunCommand::supportedOptions() const
     return installOptions();
 }
 
-void RunCommand::parseMore(QStringList &input)
+void RunCommand::parseNext(QStringList &input)
 {
-    // Build variants and properties
-    while (!input.isEmpty()) {
-        const QString arg = input.takeFirst();
-        if (arg == QLatin1String("--"))
-            break;
-        addOneToAdditionalArguments(arg);
+    QBS_CHECK(!input.empty());
+    if (input.first() != QLatin1String("--")) {
+        Command::parseNext(input);
+        return;
     }
-
+    input.removeFirst();
     m_targetParameters = input;
     input.clear();
 }
@@ -571,14 +559,14 @@ QList<CommandLineOption::Type> HelpCommand::supportedOptions() const
     return QList<CommandLineOption::Type>();
 }
 
-void HelpCommand::parseMore(QStringList &input)
+void HelpCommand::parseNext(QStringList &input)
 {
     if (input.isEmpty())
         return;
     if (input.count() > 1)
         throwError(Tr::tr("Cannot describe more than one command."));
     m_command = input.takeFirst();
-    Q_ASSERT(input.isEmpty());
+    QBS_CHECK(input.empty());
 }
 
 } // namespace qbs
diff --git a/src/app/qbs/parser/parsercommand.h b/src/app/qbs/parser/parsercommand.h
index f5f1d00e9..3df74b467 100644
--- a/src/app/qbs/parser/parsercommand.h
+++ b/src/app/qbs/parser/parsercommand.h
@@ -42,6 +42,8 @@
 #include "commandlineoption.h"
 #include "commandtype.h"
 
+#include <tools/set.h>
+
 namespace qbs {
 class CommandLineOptionPool;
 
@@ -63,19 +65,20 @@ protected:
     Command(CommandLineOptionPool &optionPool) : m_optionPool(optionPool) {}
 
     const CommandLineOptionPool &optionPool() const { return m_optionPool; }
-    void addAllToAdditionalArguments(QStringList &input);
-    void addOneToAdditionalArguments(const QString &argument);
     QString supportedOptionsDescription() const;
     [[noreturn]] void throwError(const QString &reason);
 
+    virtual void parseNext(QStringList &input);
+
 private:
     QList<CommandLineOption::Type> actualSupportedOptions() const;
-    void parseOptions(QStringList &input);
+    void parseOption(QStringList &input);
+    void parsePropertyAssignment(const QString &argument);
 
-    virtual void parseMore(QStringList &input);
     virtual QList<CommandLineOption::Type> supportedOptions() const = 0;
 
     QStringList m_additionalArguments;
+    Internal::Set<CommandLineOption *> m_usedOptions;
     const CommandLineOptionPool &m_optionPool;
 };
 
@@ -156,7 +159,7 @@ private:
     QString longDescription() const;
     QString representation() const;
     QList<CommandLineOption::Type> supportedOptions() const;
-    void parseMore(QStringList &input);
+    void parseNext(QStringList &input);
 
     QStringList m_targetParameters;
 };
@@ -239,7 +242,7 @@ private:
     QString longDescription() const;
     QString representation() const;
     QList<CommandLineOption::Type> supportedOptions() const;
-    void parseMore(QStringList &input);
+    void parseNext(QStringList &input);
 
     QString m_command;
 };