From 877ab3e5b0aeef98e5a62ded1ebddc42c61c3a48 Mon Sep 17 00:00:00 2001 From: Simon Michael Date: Sun, 15 Jun 2025 12:14:17 -1000 Subject: [PATCH] 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 | 33 ++++++++++++++++++--------------- hledger/hledger.m4.md | 26 +++++++++----------------- hledger/test/cli/addons.test | 8 ++++---- 3 files changed, 31 insertions(+), 36 deletions(-) diff --git a/hledger/Hledger/Cli.hs b/hledger/Hledger/Cli.hs index b97214fb4..d70798242 100644 --- a/hledger/Hledger/Cli.hs +++ b/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 - | 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 + -- 6.3. 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 diff --git a/hledger/hledger.m4.md b/hledger/hledger.m4.md index 580f19168..5d08d48dc 100644 --- a/hledger/hledger.m4.md +++ b/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*: -programs or scripts named "hledger-SOMETHING", which will also appear in hledger's commands list. -If you used the [hledger-install script](https://hledger.org/install.html#build-methods), -you will have several add-ons installed already. -Some more can be found in hledger's bin/ directory, documented at . +In addition to the built-in commands, you can install *add-on commands*, which will also appear in hledger's commands list. +Some of these can be installed as separate packages; +others can be found in hledger's bin/ directory, documented at . -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: -`hledger ADDONCMD [-- ADDONCMDOPTS] [ADDONCMDARGS]`. -But note the double hyphen argument, required before add-on-specific options. -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`. +You can run add-on commands using hledger, much like built-in commands. Eg `hledger ui --watch`. +(Before hledger 1.50, an `--` argument was needed before addon-specific options; this is no longer needed.) +You can also run add-ons directly: `hledger-ui --watch`. # 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`) -- enclose "problematic" args in single quotes +- you can run addon commands via hledger (`hledger ui [ARGS]`) or directly (`hledger-ui [ARGS]`) +- 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.) diff --git a/hledger/test/cli/addons.test b/hledger/test/cli/addons.test index 1317e1d87..10b345f30 100644 --- a/hledger/test/cli/addons.test +++ b/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/ ->=1 +add-on: addons/hledger-addon +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/