SDKtool: Fix return values and improve error reporting

Change-Id: I1a44a39d5cd96be48608fdb4fab252a51046971e
Reviewed-by: David Faure <david.faure@kdab.com>

src/tools/sdktool/addkeysoperation.cpp

@@ -59,8 +59,10 @@ bool AddKeysOperation::setArguments(const QStringList &args)
             continue;
         }
 
-        if (next.isNull())
+        if (next.isNull()) {
+            std::cerr << "Missing value for key '" << qPrintable(current) << "'." << std::endl << std::endl;
             return false;
+        }
 
         ++i;
         KeyValuePair pair(current, next);
@@ -81,10 +83,10 @@ int AddKeysOperation::execute() const
 
     map = addKeys(map, m_data);
     if (map.isEmpty())
-        return 1;
+        return -4;
 
     // Write data again:
-    return save(map, m_file) ? 0 : 2;
+    return save(map, m_file) ? 0 : -5;
 }
 
 #ifdef WITH_TESTS

src/tools/sdktool/addkitoperation.cpp

@@ -80,12 +80,12 @@ QString AddKitOperation::helpText() const
 
 QString AddKitOperation::argumentsHelpText() const
 {
-    return QLatin1String("    --id <ID>                                  id of the new kit.\n"
+    return QLatin1String("    --id <ID>                                  id of the new kit (required).\n"
-                         "    --name <NAME>                              display name of the new kit.\n"
+                         "    --name <NAME>                              display name of the new kit (required).\n"
                          "    --icon <PATH>                              icon of the new kit.\n"
                          "    --debuggerengine <ENGINE>                  debuggerengine of the new kit.\n"
                          "    --debugger <PATH>                          debugger of the new kit.\n"
-                         "    --devicetype <TYPE>                        device type of the new kit.\n"
+                         "    --devicetype <TYPE>                        device type of the new kit (required).\n"
                          "    --sysroot <PATH>                           sysroot of the new kit.\n"
                          "    --toolchain <ID>                           tool chain of the new kit.\n"
                          "    --qt <ID>                                  Qt of the new kit.\n"
@@ -198,6 +198,13 @@ bool AddKitOperation::setArguments(const QStringList &args)
     if (m_icon.isEmpty())
         m_icon = QLatin1String(":///DESKTOP///");
 
+    if (m_id.isEmpty())
+        std::cerr << "No id given for kit." << std::endl << std::endl;
+    if (m_displayName.isEmpty())
+        std::cerr << "No name given for kit." << std::endl << std::endl;
+    if (m_deviceType.isEmpty())
+        std::cerr << "No devicetype given for kit." << std::endl << std::endl;
+
     return !m_id.isEmpty() && !m_displayName.isEmpty() && !m_deviceType.isEmpty();
 }
 

src/tools/sdktool/addtoolchainoperation.cpp

@@ -66,10 +66,10 @@ QString AddToolChainOperation::helpText() const
 
 QString AddToolChainOperation::argumentsHelpText() const
 {
-    return QLatin1String("    --id <ID>                                  id of the new tool chain.\n"
+    return QLatin1String("    --id <ID>                                  id of the new tool chain (required).\n"
-                         "    --name <NAME>                              display name of the new tool chain.\n"
+                         "    --name <NAME>                              display name of the new tool chain (required).\n"
-                         "    --path <PATH>                              path to the compiler.\n"
+                         "    --path <PATH>                              path to the compiler (required).\n"
-                         "    --abi <ABI STRING>                         ABI of the compiler.\n"
+                         "    --abi <ABI STRING>                         ABI of the compiler (required).\n"
                          "    --supportedAbis <ABI STRING>,<ABI STRING>  list of ABIs supported by the compiler.\n"
                          "    <KEY> <TYPE:VALUE>                         extra key value pairs\n");
 }
@@ -80,58 +80,66 @@ bool AddToolChainOperation::setArguments(const QStringList &args)
         const QString current = args.at(i);
         const QString next = ((i + 1) < args.count()) ? args.at(i + 1) : QString();
 
-        if (current == QLatin1String("--id")) {
+        if (next.isNull() && current.startsWith(QLatin1String("--"))) {
-            if (next.isNull())
+            std::cerr << "No parameter for option '" << qPrintable(current) << "' given." << std::endl << std::endl;
             return false;
+        }
+
+        if (current == QLatin1String("--id")) {
             ++i; // skip next;
             m_id = next;
             continue;
         }
 
         if (current == QLatin1String("--name")) {
-            if (next.isNull())
-                return false;
             ++i; // skip next;
             m_displayName = next;
             continue;
         }
 
         if (current == QLatin1String("--path")) {
-            if (next.isNull())
-                return false;
             ++i; // skip next;
             m_path = next;
             continue;
         }
 
         if (current == QLatin1String("--abi")) {
-            if (next.isNull())
-                return false;
             ++i; // skip next;
             m_targetAbi = next;
             continue;
         }
 
         if (current == QLatin1String("--supportedAbis")) {
-            if (next.isNull())
-                return false;
             ++i; // skip next;
             m_supportedAbis = next;
             continue;
         }
 
-        if (next.isNull())
+        if (next.isNull()) {
+            std::cerr << "No value given for key '" << qPrintable(current) << "'.";
             return false;
+        }
         ++i; // skip next;
         KeyValuePair pair(current, next);
-        if (!pair.value.isValid())
+        if (!pair.value.isValid()) {
+            std::cerr << "Value for key '" << qPrintable(current) << "' is not valid.";
             return false;
+        }
         m_extra << pair;
     }
 
     if (m_supportedAbis.isEmpty())
         m_supportedAbis = m_targetAbi;
 
+    if (m_id.isEmpty())
+        std::cerr << "No id given for tool chain." << std::endl;
+    if (m_displayName.isEmpty())
+        std::cerr << "No name given for tool chain." << std::endl;
+    if (m_path.isEmpty())
+        std::cerr << "No path given for tool chain." << std::endl;
+    if (m_targetAbi.isEmpty())
+        std::cerr << "No target abi given for tool chain." << std::endl;
+
     return !m_id.isEmpty() && !m_displayName.isEmpty() && !m_path.isEmpty() && !m_targetAbi.isEmpty();
 }
 

src/tools/sdktool/findkeyoperation.cpp

@@ -59,6 +59,11 @@ bool FindKeyOperation::setArguments(const QStringList &args)
         m_keys.append(current);
     }
 
+    if (m_file.isEmpty())
+        std::cerr << "No file given." << std::endl << std::endl;
+    if (m_keys.isEmpty())
+        std::cerr << "No keys given." << std::endl << std::endl;
+
     return (!m_file.isEmpty() && !m_keys.isEmpty());
 }
 

src/tools/sdktool/findvalueoperation.cpp

@@ -57,11 +57,18 @@ bool FindValueOperation::setArguments(const QStringList &args)
         }
 
         QVariant v = Operation::valueFromString(current);
-        if (!v.isValid())
+        if (!v.isValid()) {
+            std::cerr << "Value for key '" << qPrintable(current) << "' is not valid." << std::endl << std::endl;
             return false;
+        }
         m_values << v;
     }
 
+    if (m_file.isEmpty())
+        std::cerr << "No file given." << std::endl << std::endl;
+    if (m_values.isEmpty())
+        std::cerr << "No values given." << std::endl << std::endl;
+
     return (!m_file.isEmpty() && !m_values.isEmpty());
 }
 

src/tools/sdktool/getoperation.cpp

@@ -53,7 +53,13 @@ bool GetOperation::setArguments(const QStringList &args)
 
     m_keys = args;
     m_file = m_keys.takeFirst();
-    return true;
+
+    if (m_file.isEmpty())
+        std::cerr << "No file given." << std::endl << std::endl;
+    if (m_keys.isEmpty())
+        std::cerr << "No keys given." << std::endl << std::endl;
+
+    return !m_file.isEmpty() && !m_keys.isEmpty();
 }
 
 int GetOperation::execute() const

src/tools/sdktool/main.cpp

@@ -108,6 +108,11 @@ int parseArguments(const QStringList &args, Settings *s, const QList<Operation *
                 continue;
             }
             if (current == QLatin1String("-s")) {
+                if (next.isNull()) {
+                    std::cerr << "Missing argument to '-s'." << std::endl << std::endl;
+                    printHelp(operations);
+                    return -1;
+                }
                 s->sdkPath = Utils::FileName::fromString(next);
                 ++i; // skip next;
                 continue;
@@ -141,6 +146,7 @@ int parseArguments(const QStringList &args, Settings *s, const QList<Operation *
     if (!s->operation->setArguments(opArgs)) {
         std::cerr << "Argument parsing failed." << std::endl << std::endl;
         printHelp(s->operation);
+        s->operation = 0;
         return -1;
     }
 
@@ -175,11 +181,8 @@ int main(int argc, char *argv[])
 #endif
 
     int result = parseArguments(a.arguments(), &settings, operations);
-    if (result <= 0)
+    if (!settings.operation)
-        return result;
+        return result; // nothing to do:-)
 
-    Q_ASSERT(settings.operation);
+    return settings.operation->execute();
-    settings.operation->execute();
-
-    return result;
 }

src/tools/sdktool/rmkeysoperation.cpp

@@ -53,7 +53,13 @@ bool RmKeysOperation::setArguments(const QStringList &args)
 
     m_keys = args;
     m_file = m_keys.takeFirst();
-    return true;
+
+    if (m_file.isEmpty())
+        std::cerr << "No file given." << std::endl << std::endl;
+    if (m_keys.isEmpty())
+        std::cerr << "No keys given." << std::endl << std::endl;
+
+    return !m_file.isEmpty() && !m_keys.isEmpty();
 }
 
 int RmKeysOperation::execute() const

src/tools/sdktool/rmkitoperation.cpp

@@ -75,6 +75,9 @@ bool RmKitOperation::setArguments(const QStringList &args)
 
     m_id = args.at(1);
 
+    if (m_id.isEmpty())
+        std::cerr << "No id given." << std::endl << std::endl;
+
     return !m_id.isEmpty();
 }
 

src/tools/sdktool/rmqtoperation.cpp

@@ -66,14 +66,19 @@ bool RmQtOperation::setArguments(const QStringList &args)
         const QString next = ((i + 1) < args.count()) ? args.at(i + 1) : QString();
 
         if (current == QLatin1String("--id")) {
-            if (next.isNull())
+            if (next.isNull()) {
+                std::cerr << "No parameter for --id given." << std::endl << std::endl;
                 return false;
+            }
             ++i; // skip next;
             m_id = next;
             continue;
         }
     }
 
+    if (m_id.isEmpty())
+        std::cerr << "No id given." << std::endl << std::endl;
+
     return !m_id.isEmpty();
 }
 

src/tools/sdktool/rmtoolchainoperation.cpp

@@ -67,14 +67,19 @@ bool RmToolChainOperation::setArguments(const QStringList &args)
         const QString next = ((i + 1) < args.count()) ? args.at(i + 1) : QString();
 
         if (current == QLatin1String("--id")) {
-            if (next.isNull())
+            if (next.isNull()) {
+                std::cerr << "No parameter for --id given." << std::endl << std::endl;
                 return false;
+            }
             ++i; // skip next;
             m_id = next;
             continue;
         }
     }
 
+    if (m_id.isEmpty())
+        std::cerr << "No id given." << std::endl << std::endl;
+
     return !m_id.isEmpty();
 }