Merge pull request #7585 from afishbeck/cliUsuageOnError

HPCC-12485 show ecl command usage after any parameter or option errors

Reviewed-By: Richard Chapman <rchapman@hpccsystems.com>

ecl/ecl-bundle/ecl-bundle.cpp

@@ -949,7 +949,6 @@ public:
         if (optBundle.isEmpty() && bundleCompulsory)
         {
             printf("Missing bundle name\n");
-            usage();
             return false;
         }
         if (!EclCmdCommon::finalizeOptions(globals))
@@ -1527,7 +1526,6 @@ public:
         if (optVersion.isEmpty())
         {
             printf("Version must be specified\n");
-            usage();
             return false;
         }
         return EclCmdBundleBaseWithVersion::finalizeOptions(globals);

ecl/ecl-package/ecl-package.cpp

@@ -55,10 +55,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -89,14 +86,12 @@ public:
             return false;
         if (optTarget.isEmpty())
         {
-            fprintf(stdout, "\n ... Missing target name\n\n");
-            usage();
+            fprintf(stdout, "\n ... Missing target name\n");
             return false;
         }
         if (optPackageMap.isEmpty())
         {
-            fprintf(stdout, "\n ... Missing package map name\n\n");
-            usage();
+            fprintf(stdout, "\n ... Missing package map name\n");
             return false;
         }
         if (optProcess.isEmpty())
@@ -151,10 +146,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -185,14 +177,12 @@ public:
             return false;
         if (optTarget.isEmpty())
         {
-            fprintf(stdout, "\n ... Missing target name\n\n");
-            usage();
+            fprintf(stdout, "\n ... Missing target name\n");
             return false;
         }
         if (optPackageMap.isEmpty())
         {
-            fprintf(stdout, "\n ... Missing package map name\n\n");
-            usage();
+            fprintf(stdout, "\n ... Missing package map name\n");
             return false;
         }
         if (optProcess.isEmpty())
@@ -332,10 +322,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -402,10 +389,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -439,14 +423,13 @@ public:
         }
         StringBuffer err;
         if (optPackageMap.isEmpty())
-            err.append("\n ... Missing package map name\n\n");
+            err.append("\n ... Missing package map name\n");
         else if (optTarget.isEmpty())
-            err.append("\n ... Specify a target cluster name\n\n");
+            err.append("\n ... Specify a target cluster name\n");
 
         if (err.length())
         {
             fprintf(stdout, "%s", err.str());
-            usage();
             return false;
         }
 
@@ -504,10 +487,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -555,14 +535,13 @@ public:
         }
         StringBuffer err;
         if (optFileName.isEmpty())
-            err.append("\n ... Missing package file name\n\n");
+            err.append("\n ... Missing package file name\n");
         else if (optTarget.isEmpty())
-            err.append("\n ... Specify a cluster name\n\n");
+            err.append("\n ... Specify a cluster name\n");
 
         if (err.length())
         {
             fprintf(stdout, "%s", err.str());
-            usage();
             return false;
         }
 
@@ -661,10 +640,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -717,16 +693,15 @@ public:
         if (optValidateActive)
             pcount++;
         if (pcount==0)
-            err.append("\n ... Package file name, --pmid, or --active required\n\n");
+            err.append("\n ... Package file name, --pmid, or --active required\n");
         else if (pcount > 1)
-            err.append("\n ... Package file name, --pmid, and --active are mutually exclusive\n\n");
+            err.append("\n ... Package file name, --pmid, and --active are mutually exclusive\n");
         if (optTarget.isEmpty())
             err.append("\n ... Specify a cluster name\n\n");
 
         if (err.length())
         {
             fprintf(stdout, "%s", err.str());
-            usage();
             return false;
         }
         return true;
@@ -854,10 +829,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -893,14 +865,13 @@ public:
         }
         StringBuffer err;
         if (optTarget.isEmpty())
-            err.append("\n ... A target cluster must be specified\n\n");
+            err.append("\n ... A target cluster must be specified\n");
         if (optQueryId.isEmpty())
-            err.append("\n ... A query must be specified\n\n");
+            err.append("\n ... A query must be specified\n");
 
         if (err.length())
         {
             fprintf(stdout, "%s", err.str());
-            usage();
             return false;
         }
         return true;

ecl/eclcmd/eclcmd_common.cpp

@@ -743,10 +743,7 @@ bool EclCmdWithQueryTarget::finalizeOptions(IProperties *globals)
 bool EclCmdWithQueryTarget::parseCommandLineOptions(ArgvIterator &iter)
 {
     if (iter.done())
-    {
-        usage();
         return false;
-    }
 
     for (; !iter.done(); iter.next())
     {

ecl/eclcmd/eclcmd_core.cpp

@@ -222,10 +222,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -292,10 +289,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -360,7 +354,7 @@ public:
         }
         if (optNoActivate && (optSuspendPrevious || optDeletePrevious))
         {
-            fputs("invalid --suspend-prev and --delete-prev require activation.\n\n", stderr);
+            fputs("invalid --suspend-prev and --delete-prev require activation.\n", stderr);
             return false;
         }
         if (!optSuspendPrevious && !optDeletePrevious)
@@ -371,17 +365,17 @@ public:
         }
         if (optSuspendPrevious && optDeletePrevious)
         {
-            fputs("invalid --suspend-prev and --delete-prev are mutually exclusive options.\n\n", stderr);
+            fputs("invalid --suspend-prev and --delete-prev are mutually exclusive options.\n", stderr);
             return false;
         }
         if (optMemoryLimit.length() && !isValidMemoryValue(optMemoryLimit))
         {
-            fprintf(stderr, "invalid --memoryLimit value of %s.\n\n", optMemoryLimit.get());
+            fprintf(stderr, "invalid --memoryLimit value of %s.\n", optMemoryLimit.get());
             return false;
         }
         if (optPriority.length() && !isValidPriorityValue(optPriority))
         {
-            fprintf(stderr, "invalid --priority value of %s.\n\n", optPriority.get());
+            fprintf(stderr, "invalid --priority value of %s.\n", optPriority.get());
             return false;
         }
         return true;
@@ -519,10 +513,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -837,10 +828,7 @@ public:
     {
         bool retVal = false;
         if (iter.done())
-        {
-            usage();
-            return retVal;
-        }
+            return false;
 
         for (; !iter.done(); iter.next())
         {
@@ -970,10 +958,7 @@ public:
     {
         bool retVal = false;
         if (iter.done())
-        {
-            usage();
-            return retVal;
-        }
+            return false;
 
         for (; !iter.done(); iter.next())
         {
@@ -1056,10 +1041,7 @@ public:
     {
         bool retVal = false;
         if (iter.done())
-        {
-            usage();
-            return retVal;
-        }
+            return false;
 
         for (; !iter.done(); iter.next())
         {
@@ -1142,10 +1124,7 @@ public:
     {
         bool retVal = false;
         if (iter.done())
-        {
-            usage();
-            return retVal;
-        }
+            return false;
 
         for (; !iter.done(); iter.next())
         {

ecl/eclcmd/eclcmd_shell.cpp

@@ -97,10 +97,15 @@ int EclCMDShell::processCMD(ArgvIterator &iter)
         return 0;
     }
     if (!c->parseCommandLineOptions(iter))
+    {
+        c->usage();
         return 0;
-
+    }
     if (!c->finalizeOptions(globals))
+    {
+        c->usage();
         return 0;
+    }
 
     return c->processCMD();
 }

ecl/eclcmd/queries/ecl-queries.cpp

@@ -95,10 +95,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -134,7 +131,7 @@ public:
                         flags |= QUERYLIST_SHOW_UNFLAGGED;
                         break;
                     default:
-                        fprintf(stderr, "Unrecognized --show flag = %c", *ch);
+                        fprintf(stderr, "Unrecognized --show flag = %c\n", *ch);
                         return false;
                     }
                 }
@@ -151,7 +148,7 @@ public:
         {
             if (flags)
             {
-                fputs("--show and --inactive should not be used together.\n\n", stderr);
+                fputs("--show and --inactive should not be used together.\n", stderr);
                 return false;
             }
 
@@ -309,10 +306,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -339,12 +333,12 @@ public:
     {
         if (optTarget.isEmpty())
         {
-            fputs("Target must be specified.\n\n", stderr);
+            fputs("Target must be specified.\n", stderr);
             return false;
         }
         if (optTarget.isEmpty())
         {
-            fputs("Query must be specified.\n\n", stderr);
+            fputs("Query must be specified.\n", stderr);
             return false;
         }
         if (!EclCmdCommon::finalizeOptions(globals))
@@ -413,10 +407,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -477,17 +468,17 @@ public:
             return false;
         if (optSourceQueryPath.isEmpty() && optTargetCluster.isEmpty())
         {
-            fputs("source and target must both be specified.\n\n", stderr);
+            fputs("source and target must both be specified.\n", stderr);
             return false;
         }
         if (optMemoryLimit.length() && !isValidMemoryValue(optMemoryLimit))
         {
-            fprintf(stderr, "invalid --memoryLimit value of %s.\n\n", optMemoryLimit.get());
+            fprintf(stderr, "invalid --memoryLimit value of %s.\n", optMemoryLimit.get());
             return false;
         }
         if (optPriority.length() && !isValidPriorityValue(optPriority))
         {
-            fprintf(stderr, "invalid --priority value of %s.\n\n", optPriority.get());
+            fprintf(stderr, "invalid --priority value of %s.\n", optPriority.get());
             return false;
         }
 
@@ -600,10 +591,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -646,7 +634,7 @@ public:
             return false;
         if (optSourceQuerySet.isEmpty() || optDestQuerySet.isEmpty())
         {
-            fputs("source and destination querysets must both be specified.\n\n", stderr);
+            fputs("source and destination querysets must both be specified.\n", stderr);
             return false;
         }
         return true;
@@ -742,10 +730,7 @@ public:
     virtual bool parseCommandLineOptions(ArgvIterator &iter)
     {
         if (iter.done())
-        {
-            usage();
             return false;
-        }
 
         for (; !iter.done(); iter.next())
         {
@@ -788,17 +773,17 @@ public:
             return false;
         if (optTargetCluster.isEmpty() || optQueryId.isEmpty())
         {
-            fputs("Target and QueryId must both be specified.\n\n", stderr);
+            fputs("Target and QueryId must both be specified.\n", stderr);
             return false;
         }
         if (optMemoryLimit.length() && !isValidMemoryValue(optMemoryLimit))
         {
-            fprintf(stderr, "invalid --memoryLimit value of %s.\n\n", optMemoryLimit.get());
+            fprintf(stderr, "invalid --memoryLimit value of %s.\n", optMemoryLimit.get());
             return false;
         }
         if (optPriority.length() && !isValidPriorityValue(optPriority))
         {
-            fprintf(stderr, "invalid --priority value of %s.\n\n", optPriority.get());
+            fprintf(stderr, "invalid --priority value of %s.\n", optPriority.get());
             return false;
         }
         return true;

ecl/eclcmd/roxie/ecl-roxie.cpp

@@ -177,7 +177,7 @@ public:
             return false;
         if (optProcess.isEmpty())
         {
-            fputs("process cluster must be specified.\n\n", stderr);
+            fputs("process cluster must be specified.\n", stderr);
             return false;
         }
         return true;
@@ -283,7 +283,7 @@ public:
             return false;
         if (optProcess.isEmpty())
         {
-            fputs("process cluster must be specified.\n\n", stderr);
+            fputs("process cluster must be specified.\n", stderr);
             return false;
         }
         return true;
@@ -401,7 +401,7 @@ public:
             return false;
         if (optProcess.isEmpty())
         {
-            fputs("process cluster must be specified.\n\n", stderr);
+            fputs("process cluster must be specified.\n", stderr);
             return false;
         }
         if (optDeleteRecursive)