From 68db3075d7ac4a48377967ad54e45b95b9ee6c61 Mon Sep 17 00:00:00 2001 From: David Steele Date: Mon, 22 Jan 2024 12:00:13 -0300 Subject: [PATCH] Allow --version and --help for version and help. It is a bit confusing that --help and --version do not work like most command-line programs. For example, git allows either --help or help. Make these work by making them shortcuts (not actual options) to the applicable commands. The user will still need to use help (not --help) to get help on specific commands/options, but at least they can get to the main help (which will tell them this) via --help. --- doc/xml/release/2024/2.51.xml | 15 ++++++++ src/config/parse.c | 56 +++++++++++++++++++++++------ test/src/module/config/parseTest.c | 58 ++++++++++++++++++++++++++++++ 3 files changed, 118 insertions(+), 11 deletions(-) diff --git a/doc/xml/release/2024/2.51.xml b/doc/xml/release/2024/2.51.xml index 1d07ead4d..88ee05ca4 100644 --- a/doc/xml/release/2024/2.51.xml +++ b/doc/xml/release/2024/2.51.xml @@ -1,2 +1,17 @@ + + + + + + + + + + + +

Allow --version and --help for version and help.

+ + + diff --git a/src/config/parse.c b/src/config/parse.c index d51e9b93f..a78ca57e7 100644 --- a/src/config/parse.c +++ b/src/config/parse.c @@ -1556,6 +1556,9 @@ cfgParse(const Storage *const storage, const unsigned int argListSize, const cha // Only the first non-option parameter should be treated as a command so track if the command has been set bool commandSet = false; + unsigned int argIgnore = 0; + bool help = false; + bool version = false; for (unsigned int argListIdx = 1; argListIdx < argListSize; argListIdx++) { @@ -1589,7 +1592,25 @@ cfgParse(const Storage *const storage, const unsigned int argListSize, const cha CfgParseOptionResult option = cfgParseOptionP(optionName, .prefixMatch = true); if (!option.found) - THROW_FMT(OptionInvalidError, "invalid option '--%s'", arg); + { + // Check for --help shortcut to the help command + if (strcmp(arg, CFGCMD_HELP) == 0) + { + help = true; + argIgnore++; + continue; + } + // Else check for --version shortcut to the version command + else if (strcmp(arg, CFGCMD_VERSION) == 0) + { + version = true; + argIgnore++; + continue; + } + // Else invalid option + else + THROW_FMT(OptionInvalidError, "invalid option '--%s'", arg); + } // If the option may have an argument (arguments are optional for boolean options) if (!option.negate && !option.reset) @@ -1749,13 +1770,6 @@ cfgParse(const Storage *const storage, const unsigned int argListSize, const cha config->help = true; else commandSet = true; - - // Set command options - config->lockRequired = parseRuleCommand[config->command].lockRequired; - config->lockRemoteRequired = parseRuleCommand[config->command].lockRemoteRequired; - config->lockType = (LockType)parseRuleCommand[config->command].lockType; - config->logFile = parseRuleCommand[config->command].logFile; - config->logLevelDefault = (LogLevel)parseRuleCommand[config->command].logLevelDefault; } // Additional arguments are command arguments else @@ -1778,11 +1792,27 @@ cfgParse(const Storage *const storage, const unsigned int argListSize, const cha if (!commandSet && !config->help) { // If there are args then error - if (argListSize > 1) + if (argListSize - argIgnore > 1) THROW_FMT(CommandRequiredError, "no command found"); - // Otherwise set the command to help - config->help = true; + // Output help if --help specified or --version not specified + if (help || !version) + { + config->help = true; + } + // Else output version + else + config->command = cfgCmdVersion; + } + + // Set command options + if (config->command != cfgCmdNone) + { + config->lockRequired = parseRuleCommand[config->command].lockRequired; + config->lockRemoteRequired = parseRuleCommand[config->command].lockRemoteRequired; + config->lockType = (LockType)parseRuleCommand[config->command].lockType; + config->logFile = parseRuleCommand[config->command].logFile; + config->logLevelDefault = (LogLevel)parseRuleCommand[config->command].logLevelDefault; } // Error when parameters found but the command does not allow parameters @@ -1797,6 +1827,10 @@ cfgParse(const Storage *const storage, const unsigned int argListSize, const cha // specific command and would like to display actual option values in the help. if (config->command != cfgCmdNone && config->command != cfgCmdVersion && config->command != cfgCmdHelp) { + // Error if --help or --version passed to command + if (help || version) + THROW_FMT(OptionInvalidError, "invalid option '--%s'", help ? CFGCMD_HELP : CFGCMD_VERSION); + // Phase 2: parse environment variables // --------------------------------------------------------------------------------------------------------------------- unsigned int environIdx = 0; diff --git a/test/src/module/config/parseTest.c b/test/src/module/config/parseTest.c index 3821e665b..e1e215e1d 100644 --- a/test/src/module/config/parseTest.c +++ b/test/src/module/config/parseTest.c @@ -1542,6 +1542,64 @@ testRun(void) TEST_RESULT_BOOL(cfgCommandHelp(), true, "help is set"); TEST_RESULT_INT(cfgCommand(), cfgCmdVersion, "command is version"); + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("help option"); + + argList = strLstNew(); + strLstAddZ(argList, TEST_BACKREST_EXE); + strLstAddZ(argList, "--help"); + + TEST_RESULT_VOID(cfgParseP(storageTest, strLstSize(argList), strLstPtr(argList), .noResetLogLevel = true), "load config"); + TEST_RESULT_BOOL(cfgCommandHelp(), true, "help is set"); + TEST_RESULT_INT(cfgCommand(), cfgCmdNone, "command is none"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("version option"); + + argList = strLstNew(); + strLstAddZ(argList, TEST_BACKREST_EXE); + strLstAddZ(argList, "--version"); + + TEST_RESULT_VOID(cfgParseP(storageTest, strLstSize(argList), strLstPtr(argList), .noResetLogLevel = true), "load config"); + TEST_RESULT_BOOL(cfgCommandHelp(), false, "help is not set"); + TEST_RESULT_INT(cfgCommand(), cfgCmdVersion, "command is version"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("help and version options"); + + argList = strLstNew(); + strLstAddZ(argList, TEST_BACKREST_EXE); + strLstAddZ(argList, "--help"); + strLstAddZ(argList, "--version"); + + TEST_RESULT_VOID(cfgParseP(storageTest, strLstSize(argList), strLstPtr(argList), .noResetLogLevel = true), "load config"); + TEST_RESULT_BOOL(cfgCommandHelp(), true, "help is not set"); + TEST_RESULT_INT(cfgCommand(), cfgCmdNone, "command is none"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("error on command with --help option"); + + argList = strLstNew(); + strLstAddZ(argList, TEST_BACKREST_EXE); + strLstAddZ(argList, "--help"); + strLstAddZ(argList, "backup"); + + TEST_ERROR( + cfgParseP(storageTest, strLstSize(argList), strLstPtr(argList), .noResetLogLevel = true), OptionInvalidError, + "invalid option '--help'"); + + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("error on command with --version option"); + + argList = strLstNew(); + strLstAddZ(argList, TEST_BACKREST_EXE); + strLstAddZ(argList, "--version"); + strLstAddZ(argList, "backup"); + + TEST_ERROR( + cfgParseP(storageTest, strLstSize(argList), strLstPtr(argList), .noResetLogLevel = true), OptionInvalidError, + "invalid option '--version'"); + // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("help command - should not fail on missing options");