Restructure CommandLineArgs::ParseCommandLine_Impl

CommandLineArgs::ParseCommandLine_Impl is restructured to remove
multiple redundant booleans and make its structure more clear.
Command line "events" (that specify following filenames treatment)
are grouped to an enum. (NB: "events" is not invented by me, this
is currently used term).

Normalized treatment of --display param: now it is always ignored.
Previously, if it happened to be a known switch like --writer, it
handled as it's not after --display.

Immediate parameters to switches --pt, --convert-to, --outdir,
--printer-name are now treated likewise. They are treated as
corresponding values, even if they happen to be known switches.

--language (without "=") is no more a recognized switch. Before
this it skipped all filenames until next file management switch.

After this gets approved, TODO is to tweak Office URIs to take
current event into account: e.g., to print opened file if it's
after -p.

Change-Id: Ic2f3980548f8906f695d297dc6677bd2cbd19c5b
Reviewed-on: https://gerrit.libreoffice.org/27316
Tested-by: Jenkins <ci@libreoffice.org>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

2 files changed, 88 insertions, 178 deletions

diff --git a/desktop/source/app/cmdlineargs.cxx b/desktop/source/app/cmdlineargs.cxx
index c4ae61a330f5..f13e01c7b82d 100644
--- a/desktop/source/app/cmdlineargs.cxx
+++ b/desktop/source/app/cmdlineargs.cxx
@@ -208,22 +208,9 @@ void CommandLineArgs::ParseCommandLine_Impl( Supplier& supplier )
 {
     m_cwdUrl = supplier.getCwdUrl();
 
-    // parse command line arguments
-    bool bOpenEvent(true);
-    bool bPrintEvent(false);
-    bool bViewEvent(false);
-    bool bStartEvent(false);
-    bool bPrintToEvent(false);
-    bool bPrinterName(false);
-    bool bForceOpenEvent(false);
-    bool bForceNewEvent(false);
-    bool bDisplaySpec(false);
-    bool bOpenDoc(false);
-    bool bConversionEvent(false);
-    bool bConversionParamsEvent(false);
-    bool bBatchPrintEvent(false);
-    bool bBatchPrinterNameEvent(false);
-    bool bConversionOutEvent(false);
+    enum class CommandLineEvent { Open, Print, View, Start, PrintTo,
+                                  ForceOpen, ForceNew, Conversion, BatchPrint };
+    CommandLineEvent eCurrentEvent = CommandLineEvent::Open;
 
     for (;;)
     {
@@ -270,8 +257,7 @@ void CommandLineArgs::ParseCommandLine_Impl( Supplier& supplier )
             {
                 m_textcat = true;
                 m_conversionparams = "txt:Text";
-                bOpenEvent = false;
-                bConversionEvent = true;
+                eCurrentEvent = CommandLineEvent::Conversion;
                 setHeadless();
             }
             else if ( oArg == "quickstart" )
@@ -345,8 +331,8 @@ void CommandLineArgs::ParseCommandLine_Impl( Supplier& supplier )
 
                 // We specifically need to consume the following 2 arguments
                 // for --protector
-                assert( supplier.next( &aArg ) );
-                assert( supplier.next( &aArg ) );
+                if ((!supplier.next(&aArg) || !supplier.next(&aArg)) && m_unknown.isEmpty())
+                    m_unknown = "--protector must be followed by two arguments";
             }
             else if ( oArg == "version" )
             {
@@ -446,120 +432,80 @@ void CommandLineArgs::ParseCommandLine_Impl( Supplier& supplier )
             else if ( aArg == "-n" )
             {
                 // force new documents based on the following documents
-                bForceNewEvent  = true;
-                bOpenEvent      = false;
-                bForceOpenEvent = false;
-                bPrintToEvent   = false;
-                bPrintEvent     = false;
-                bViewEvent      = false;
-                bStartEvent     = false;
-                bDisplaySpec    = false;
+                eCurrentEvent = CommandLineEvent::ForceNew;
             }
             else if ( aArg == "-o" )
             {
                 // force open documents regardless if they are templates or not
-                bForceOpenEvent = true;
-                bOpenEvent      = false;
-                bForceNewEvent  = false;
-                bPrintToEvent   = false;
-                bPrintEvent     = false;
-                bViewEvent      = false;
-                bStartEvent     = false;
-                bDisplaySpec    = false;
+                eCurrentEvent = CommandLineEvent::ForceOpen;
             }
             else if ( oArg == "pt" )
             {
                 // Print to special printer
-                bPrintToEvent   = true;
-                bPrinterName    = true;
-                bPrintEvent     = false;
-                bOpenEvent      = false;
-                bForceNewEvent  = false;
-                bViewEvent      = false;
-                bStartEvent     = false;
-                bDisplaySpec    = false;
-                bForceOpenEvent = false;
+                eCurrentEvent = CommandLineEvent::PrintTo;
+                // first argument after "-pt" must be the printer name
+                if (supplier.next(&aArg))
+                    m_printername = aArg;
+                else if (m_unknown.isEmpty())
+                    m_unknown = "--pt must be followed by printername";
             }
             else if ( aArg == "-p" )
             {
                 // Print to default printer
-                bPrintEvent     = true;
-                bPrintToEvent   = false;
-                bOpenEvent      = false;
-                bForceNewEvent  = false;
-                bForceOpenEvent = false;
-                bViewEvent      = false;
-                bStartEvent     = false;
-                bDisplaySpec    = false;
+                eCurrentEvent = CommandLineEvent::Print;
             }
             else if ( oArg == "view")
             {
                 // open in viewmode
-                bOpenEvent      = false;
-                bPrintEvent     = false;
-                bPrintToEvent   = false;
-                bForceNewEvent  = false;
-                bForceOpenEvent = false;
-                bViewEvent      = true;
-                bStartEvent     = false;
-                bDisplaySpec    = false;
+                eCurrentEvent = CommandLineEvent::View;
             }
             else if ( oArg == "show" )
             {
                 // open in viewmode
-                bOpenEvent      = false;
-                bViewEvent      = false;
-                bStartEvent     = true;
-                bPrintEvent     = false;
-                bPrintToEvent   = false;
-                bForceNewEvent  = false;
-                bForceOpenEvent = false;
-                bDisplaySpec    = false;
+                eCurrentEvent = CommandLineEvent::Start;
             }
             else if ( oArg == "display" )
             {
-                // set display
-                bOpenEvent      = false;
-                bPrintEvent     = false;
-                bForceOpenEvent = false;
-                bPrintToEvent   = false;
-                bForceNewEvent  = false;
-                bViewEvent      = false;
-                bStartEvent     = false;
-                bDisplaySpec    = true;
-            }
-            else if ( oArg == "language" )
-            {
-                bOpenEvent      = false;
-                bPrintEvent     = false;
-                bForceOpenEvent = false;
-                bPrintToEvent   = false;
-                bForceNewEvent  = false;
-                bViewEvent      = false;
-                bStartEvent     = false;
-                bDisplaySpec    = false;
+                // The command line argument following --display should
+                // always be treated as the argument of --display.
+                // --display and its argument are handled "out of line"
+                // in Unix-only desktop/unx/source/splashx.c and vcl/unx/*,
+                // and just ignored here
+                supplier.next(&aArg);
             }
             else if ( oArg == "convert-to" )
             {
-                bOpenEvent = false;
-                bConversionEvent = true;
-                bConversionParamsEvent = true;
-                // It doesn't make sense to use convert-to without headless.
-                setHeadless();
+                eCurrentEvent = CommandLineEvent::Conversion;
+                // first argument must be the params
+                if (supplier.next(&aArg))
+                {
+                    m_conversionparams = aArg;
+                    // It doesn't make sense to use convert-to without headless.
+                    setHeadless();
+                }
+                else if (m_unknown.isEmpty())
+                    m_unknown = "--convert-to must be followed by output_file_extension[:output_filter_name]";
             }
             else if ( oArg == "print-to-file" )
             {
-                bOpenEvent = false;
-                bBatchPrintEvent = true;
+                eCurrentEvent = CommandLineEvent::BatchPrint;
             }
-            else if ( oArg == "printer-name" && bBatchPrintEvent )
+            else if ( eCurrentEvent == CommandLineEvent::BatchPrint && oArg == "printer-name" )
             {
-                bBatchPrinterNameEvent = true;
+                // first argument is the printer name
+                if (supplier.next(&aArg))
+                    m_printername = aArg;
+                else if (m_unknown.isEmpty())
+                    m_unknown = "--printer-name must be followed by printername";
             }
-            else if ( oArg == "outdir" &&
-                      (bConversionEvent || bBatchPrintEvent) )
+            else if ( (eCurrentEvent == CommandLineEvent::Conversion ||
+                       eCurrentEvent == CommandLineEvent::BatchPrint)
+                      && oArg == "outdir" )
             {
-                bConversionOutEvent = true;
+                if (supplier.next(&aArg))
+                    m_conversionout = aArg;
+                else if (m_unknown.isEmpty())
+                    m_unknown = "--outdir must be followed by output directory path";
             }
             else if ( aArg.startsWith("-") )
             {
@@ -586,82 +532,49 @@ void CommandLineArgs::ParseCommandLine_Impl( Supplier& supplier )
             }
             else
             {
-                if ( bPrinterName && bPrintToEvent )
-                {
-                    // first argument after "-pt" this must be the printer name
-                    m_printername = aArg;
-                    bPrinterName = false;
-                }
-                else if ( bConversionParamsEvent && bConversionEvent )
-                {
-                    // first argument must be the params
-                    m_conversionparams = aArg;
-                    bConversionParamsEvent = false;
-                }
-                else if ( bBatchPrinterNameEvent && bBatchPrintEvent )
-                {
-                    // first argument is the printer name
-                    m_printername = aArg;
-                    bBatchPrinterNameEvent = false;
-                }
-                else if ( (bConversionEvent || bBatchPrintEvent) && bConversionOutEvent )
+                // handle this argument as a filename
+
+                // 1. Check if this is an Office URI
+                if (OfficeURISchemeCommandLineSupplier::IsOfficeURI(aArg))
                 {
-                    m_conversionout = aArg;
-                    bConversionOutEvent = false;
+                    OfficeURISchemeCommandLineSupplier OfficeURISupplier(getCwdUrl(), aArg);
+                    // Add the file according its command, ignore current event
+                    ParseCommandLine_Impl(OfficeURISupplier);
                 }
-                else
+                else switch (eCurrentEvent)
                 {
-                    // handle this argument as a filename
-
-                    // 1. Check if this is an Office URI
-                    if (!bDisplaySpec && OfficeURISchemeCommandLineSupplier::IsOfficeURI(aArg))
-                    {
-                        OfficeURISchemeCommandLineSupplier OfficeURISupplier(getCwdUrl(), aArg);
-                        // Add the file according its command, ignore current event
-                        ParseCommandLine_Impl(OfficeURISupplier);
-                    }
-                    else if ( bOpenEvent )
-                    {
-                        m_openlist.push_back(aArg);
-                        bOpenDoc = true;
-                    }
-                    else if ( bViewEvent )
-                    {
-                        m_viewlist.push_back(aArg);
-                        bOpenDoc = true;
-                    }
-                    else if ( bStartEvent )
-                    {
-                        m_startlist.push_back(aArg);
-                        bOpenDoc = true;
-                    }
-                    else if ( bPrintEvent )
-                    {
-                        m_printlist.push_back(aArg);
-                        bOpenDoc = true;
-                    }
-                    else if ( bPrintToEvent )
-                    {
-                        m_printtolist.push_back(aArg);
-                        bOpenDoc = true;
-                    }
-                    else if ( bForceNewEvent )
-                    {
-                        m_forcenewlist.push_back(aArg);
-                        bOpenDoc = true;
-                    }
-                    else if ( bForceOpenEvent )
-                    {
-                        m_forceopenlist.push_back(aArg);
-                        bOpenDoc = true;
-                    }
-                    else if ( bDisplaySpec )
-                    {
-                        bDisplaySpec = false; // only one display, not a list
-                        bOpenEvent = true;    // set back to standard
-                    }
-                    else if ( bConversionEvent || bBatchPrintEvent )
-                        m_conversionlist.push_back(aArg);
+                case CommandLineEvent::Open:
+                    m_openlist.push_back(aArg);
+                    m_bDocumentArgs = true;
+                    break;
+                case CommandLineEvent::View:
+                    m_viewlist.push_back(aArg);
+                    m_bDocumentArgs = true;
+                    break;
+                case CommandLineEvent::Start:
+                    m_startlist.push_back(aArg);
+                    m_bDocumentArgs = true;
+                    break;
+                case CommandLineEvent::Print:
+                    m_printlist.push_back(aArg);
+                    m_bDocumentArgs = true;
+                    break;
+                case CommandLineEvent::PrintTo:
+                    m_printtolist.push_back(aArg);
+                    m_bDocumentArgs = true;
+                    break;
+                case CommandLineEvent::ForceNew:
+                    m_forcenewlist.push_back(aArg);
+                    m_bDocumentArgs = true;
+                    break;
+                case CommandLineEvent::ForceOpen:
+                    m_forceopenlist.push_back(aArg);
+                    m_bDocumentArgs = true;
+                    break;
+                case CommandLineEvent::Conversion:
+                case CommandLineEvent::BatchPrint:
+                    m_conversionlist.push_back(aArg);
+                    break;
                 }
             }
 
@@ -672,9 +585,6 @@ void CommandLineArgs::ParseCommandLine_Impl( Supplier& supplier )
             }
         }
     }
-
-    if ( bOpenDoc )
-        m_bDocumentArgs = true;
 }
 
 void CommandLineArgs::InitParamValues()
diff --git a/desktop/source/app/cmdlinehelp.cxx b/desktop/source/app/cmdlinehelp.cxx
index 8cbcf58b741f..15377473bae2 100644
--- a/desktop/source/app/cmdlinehelp.cxx
+++ b/desktop/source/app/cmdlinehelp.cxx
@@ -148,7 +148,7 @@ namespace desktop
         aHelpMessage_head = aHelpMessage_head.replaceFirst( "%CMDNAME", "soffice" );
         if (!unknown.isEmpty())
         {
-            aHelpMessage_head = "Unknown option: " + unknown + "\n\n"
+            aHelpMessage_head = "Error in option: " + unknown + "\n\n"
                 + aHelpMessage_head;
         }
 #ifdef UNX