fix:cli: clearer unknown command & flag error; accept addon flags [#2388] [#458]

When given an unknown command and an unknown flag, we now prioritise
showing the unknown command error. [#2388]

This fix, on top of prior work, also solves a long standing CLI/UX
problem we've had with addon commands: now you can freely mix in
addon-specific options in a hledger command line, and hledger won't
complain. Ie, writing -- before addon flags is no longer needed. \o/

hledger/Hledger/Cli.hs

@@ -273,7 +273,7 @@ main = handleExit $ withGhcDebug' $ do
       else getConf' cliconfrawopts
 
   ---------------------------------------------------------------
-  dbgio "\n3. Identify a command name from config file or command line" ()
+  dbgio "\n3. Identify a command name if possible; handle version/help flags" ()
 
   -- Try to identify the subcommand name,
   -- from the first non-flag general argument in the config file,
@@ -296,12 +296,14 @@ main = handleExit $ withGhcDebug' $ do
     -- the command line contains a bad flag or wrongly present/missing flag value,
     -- cmdname will be "".
     args = [confcmdarg | not $ null confcmdarg] <> cliargswithcmdfirstwithoutclispecific
-    cmdname = stringopt "command" $ cmdargsParse "for command name" (mainmode addons) args
+    -- Actually, only scan the first non-flag argument, to avoid flag errors at this stage.
+    possiblecmdarg = take 1 $ dropWhile isFlagArg args
+    cmdname = stringopt "command" $ cmdargsParse "for command name" (mainmode addons) possiblecmdarg
 
     badcmdprovided = null cmdname && not nocmdprovided
     isaddoncmd     = not (null cmdname) && cmdname `elem` addons
 
-    -- And get the builtin command's mode and action, if any.
+    -- If it's a builtin command, get its mode and action.
     mbuiltincmdaction = findBuiltinCommand cmdname
     effectivemode = maybe (mainmode []) fst mbuiltincmdaction
 
@@ -314,6 +316,10 @@ main = handleExit $ withGhcDebug' $ do
   dbgio "bad command provided" badcmdprovided
   dbgio "is addon command" isaddoncmd
 
+  -- If a bad command was provided, show that error now, before the full cmdargsParse attempt.
+  when badcmdprovided $
+    error' $ "command "++clicmdarg++" is not recognized. Run with no command to see a list."
+
   ---------------------------------------------------------------
   dbgio "\n4. Get applicable options/arguments from config file" ()
 
@@ -398,44 +404,41 @@ main = handleExit $ withGhcDebug' $ do
     -- 6.2. --version flag found and none of these other conditions - show version
     | versionFlag && not (isaddoncmd || helpFlag || tldrFlag || infoFlag || manFlag) -> putStrLn prognameandversion
 
-    -- 6.3. there's a command argument, but it's bad - show error
+    -- 6.3. no command found, nothing else to do - show the commands list
-    | badcmdprovided -> error' $ "command "++clicmdarg++" is not recognized, run with no command to see a list"
-
-    -- 6.4. no command found, nothing else to do - show the commands list
     | nocmdprovided -> do
         dbg1IO "no command, showing commands list" ()
         commands opts (ignoredjournal "commands")
 
-    -- 6.5. builtin command found
+    -- 6.4. builtin command found
     | Just (cmdmode, cmdaction) <- mbuiltincmdaction -> do
       let mmodecmdname = headMay $ modeNames cmdmode
       dbg1IO "running builtin command mode" $ fromMaybe "" mmodecmdname
 
       -- run the builtin command according to its type
       if
-        -- 6.5.1. help/doc flag - show command help/docs
+        -- 6.4.1. help/doc flag - show command help/docs
         | helpFlag  -> runPager $ showModeUsage cmdmode ++ "\n"
         | tldrFlag  -> runTldrForPage $ maybe "hledger" (("hledger-"<>)) mmodecmdname
         | infoFlag  -> runInfoForTopic "hledger" mmodecmdname
         | manFlag   -> runManForTopic "hledger"  mmodecmdname
 
-        -- 6.5.2. builtin command which should not require or read the journal - run it
+        -- 6.4.2. builtin command which should not require or read the journal - run it
         | cmdname `elem` ["commands","demo","help","setup","test"] ->
           cmdaction opts (ignoredjournal cmdname)
 
-        -- 6.5.3. builtin command which should create the journal if missing - do that and run it
+        -- 6.4.3. builtin command which should create the journal if missing - do that and run it
         | cmdname `elem` ["add","import"] -> do
           ensureJournalFileExists . NE.head =<< journalFilePathFromOpts opts
           withJournalDo opts (cmdaction opts)
 
-        -- 6.5.4. "run" and "repl" need findBuiltinCommands passed to it to avoid circular dependency in the code
+        -- 6.4.4. "run" and "repl" need findBuiltinCommands passed to it to avoid circular dependency in the code
         | cmdname == "run"  -> Hledger.Cli.Commands.Run.run Nothing findBuiltinCommand addons opts
         | cmdname == "repl" -> Hledger.Cli.Commands.Run.repl findBuiltinCommand addons opts
 
-        -- 6.5.5. all other builtin commands - read the journal and if successful run the command with it
+        -- 6.4.5. all other builtin commands - read the journal and if successful run the command with it
         | otherwise -> withJournalDo opts $ cmdaction opts
 
-    -- 6.6. external addon command found - run it,
+    -- 6.5. external addon command found - run it,
     -- passing any cli arguments written after the command name
     -- and any command-specific opts from the config file.
     -- Any "--" arguments, which sometimes must be used in the command line
@@ -458,7 +461,7 @@ main = handleExit $ withGhcDebug' $ do
     -- deprecated command found
     -- cmdname == "convert" = error' (modeHelp oldconvertmode)
 
-    -- 6.7. something else (shouldn't happen) - show an error
+    -- 6.6. something else (shouldn't happen) - show an error
     | otherwise -> usageError $
         "could not understand the arguments "++show finalargs
         <> if null confothergenargs then "" else "\ngeneral arguments added from config file: "++show confothergenargs

hledger/hledger.m4.md

@@ -221,13 +221,11 @@ Eg: `hledger bal -h`.
 
 ## Add-on commands
 
-In addition to the built-in commands, you can install *add-on commands*:
+In addition to the built-in commands, you can install *add-on commands*, which will also appear in hledger's commands list.
-programs or scripts named "hledger-SOMETHING", which will also appear in hledger's commands list.
+Some of these can be installed as separate packages;
-If you used the [hledger-install script](https://hledger.org/install.html#build-methods),
+others can be found in hledger's bin/ directory, documented at <https://hledger.org/scripts.html>.
-you will have several add-ons installed already.
-Some more can be found in hledger's bin/ directory, documented at <https://hledger.org/scripts.html>.
 
-More precisely, add-on commands are programs or scripts in your shell's PATH,
+Add-on commands are programs or scripts in your shell's PATH,
 whose name starts with "hledger-"
 and ends with no extension or a recognised extension
 (".bat", ".com", ".exe", ".hs", ".js", ".lhs", ".lua", ".php", ".pl", ".py", ".rb", ".rkt", or ".sh"),
@@ -238,12 +236,9 @@ m4_dnl call hledger's code directly, which means they can do anything built-in c
 m4_dnl Scripts/programs in other languages can't do this, but they can use hledger's
 m4_dnl command-line interface, or output formats like CSV or JSON.
 
-You can run add-on commands using hledger, much like built-in commands:
+You can run add-on commands using hledger, much like built-in commands. Eg `hledger ui --watch`.
-`hledger ADDONCMD [-- ADDONCMDOPTS] [ADDONCMDARGS]`.
+(Before hledger 1.50, an `--` argument was needed before addon-specific options; this is no longer needed.)
-But note the double hyphen argument, required before add-on-specific options.
+You can also run add-ons directly: `hledger-ui --watch`.
-Eg: `hledger ui -- --watch` or `hledger web -- --serve`. 
-If this causes difficulty, you can always run the add-on directly, without using `hledger`:
-`hledger-ui --watch` or `hledger-web --serve`.
 
 # Options
 
@@ -6619,8 +6614,8 @@ the sharp edges described in [OPTIONS](#options),
 here are some tips that might help:
 
 - command-specific options must go after the command (it's fine to put common options there too: `hledger CMD OPTS ARGS`)
-- running add-on executables directly simplifies command line parsing (`hledger-ui OPTS ARGS`)
+- you can run addon commands via hledger (`hledger ui [ARGS]`) or directly (`hledger-ui [ARGS]`)
-- enclose "problematic" args in single quotes
+- enclose "problematic" arguments in single quotes
 - if needed, also add a backslash to hide regular expression metacharacters from the shell
 - to see how a misbehaving command line is being parsed, add `--debug=2`.
 
@@ -7009,9 +7004,6 @@ _reportbugs_
 
 Some known issues and limitations:
 
-The need to precede add-on command options with `--` when invoked from hledger is awkward.
-(See Command options, Constructing command lines.)
-
 A system locale with a suitable text encoding must be configured to work with non-ascii data.
 (See Text encoding, Troubleshooting.)
 

hledger/test/cli/addons.test

@@ -10,12 +10,12 @@
 $  PATH=$PATH:addons hledger addon --help
 > /hledger-addon/
 
-# ** 2. addon-specific flags which are not also defined in the main executable are not accepted
+# ** 2. addon-specific flags are recognised and won't cause an error. (#458)
 $  PATH=$PATH:addons hledger addon --addonflag
->2 /Unknown flag: --addonflag/
+add-on: addons/hledger-addon
->=1
+args: --addonflag
 
-# ** 3. hledger main executable ignores anything after --, and hides the -- from the addon.
+# ** 3. If an old-style -- argument is used before addon flags, the addon won't see it.
 $  PATH=$PATH:addons hledger addon --help -- --addonflag
 > /args: --help --addonflag/