From a822bc93e74578789a45decaed030b6ccc3c059d Mon Sep 17 00:00:00 2001 From: Chris Punches Date: Thu, 13 Mar 2025 21:30:02 -0400 Subject: [PATCH] continued improvement of cli parsing --- dpmdk/include/CommonModuleAPI.hpp | 52 ++++++----- modules/build/include/cli_parsers.hpp | 6 +- modules/build/include/commands.hpp | 13 +++ modules/build/src/cli_parsers.cpp | 120 +++++++++++++++++--------- modules/build/src/commands.cpp | 47 ++++++---- 5 files changed, 148 insertions(+), 90 deletions(-) diff --git a/dpmdk/include/CommonModuleAPI.hpp b/dpmdk/include/CommonModuleAPI.hpp index 5e3611c..dcf96ec 100644 --- a/dpmdk/include/CommonModuleAPI.hpp +++ b/dpmdk/include/CommonModuleAPI.hpp @@ -163,40 +163,36 @@ extern "C" { */ #define DPM_MODULE_STANDALONE_MAIN() \ extern "C" void dpm_log(int level, const char* message) { \ - const char* level_str; \ - switch (level) { \ - case 0: level_str = "FATAL"; break; \ - case 1: level_str = "ERROR"; break; \ - case 2: level_str = "WARN"; break; \ - case 3: level_str = "INFO"; break; \ - case 4: level_str = "DEBUG"; break; \ - default: level_str = "UNKNOWN"; break; \ - } \ - std::cout << "[" << level_str << "] " << message << std::endl; \ +const char* level_str; \ +switch (level) { \ +case 0: level_str = "FATAL"; break; \ +case 1: level_str = "ERROR"; break; \ +case 2: level_str = "WARN"; break; \ +case 3: level_str = "INFO"; break; \ +case 4: level_str = "DEBUG"; break; \ +default: level_str = "UNKNOWN"; break; \ +} \ +std::cout << "[" << level_str << "] " << message << std::endl; \ } \ extern "C" const char* dpm_get_config(const char* section, const char* key) { \ - return nullptr; \ +return nullptr; \ } \ extern "C" void dpm_set_logging_level(int level) { \ - std::cout << "[INFO] Verbosity level ignored, as all standalone executions have maximum verbosity" << std::endl; \ +std::cout << "[INFO] Verbosity level ignored, as all standalone executions have maximum verbosity" << std::endl; \ } \ int main(int argc, char** argv) { \ - std::cout << "Module version: " << dpm_module_get_version() << std::endl; \ - std::cout << "Description: " << dpm_get_description() << std::endl; \ - \ - /* Default to "help" if no command is provided */ \ - const char* command = "help"; \ - \ - /* If arguments are provided, use the first as command */ \ - if (argc > 1) { \ - command = argv[1]; \ - /* Shift arguments for the command handler but keep the original argc count */ \ - argv++; \ - argc--; \ - } \ - \ - std::cout << "Executing command: " << command << std::endl; \ - return dpm_module_execute(command, argc, argv); \ +/* Default to "help" if no command is provided */ \ +const char* command = "help"; \ +\ +/* If arguments are provided, use the first as command */ \ +if (argc > 1) { \ +command = argv[1]; \ +/* Shift arguments for the command handler but keep the original argc count */ \ +argv++; \ +argc--; \ +} \ +\ +return dpm_module_execute(command, argc, argv); \ } #endif // BUILD_STANDALONE \ No newline at end of file diff --git a/modules/build/include/cli_parsers.hpp b/modules/build/include/cli_parsers.hpp index ab5f7cf..479d97a 100644 --- a/modules/build/include/cli_parsers.hpp +++ b/modules/build/include/cli_parsers.hpp @@ -26,16 +26,18 @@ struct BuildOptions { std::string contents_dir; /**< Directory with package contents */ std::string hooks_dir; /**< Directory with package hooks */ std::string package_name; /**< Name of the package to build */ + std::string package_version; /**< Version of the package to build */ bool force; /**< Flag to force package creation even if warnings occur */ bool verbose; /**< Flag for verbose output */ bool show_help; /**< Flag to show help information */ - // Constructor with default values + // Constructor with only force and verbose defaulted BuildOptions() : - output_dir("."), + output_dir(""), contents_dir(""), hooks_dir(""), package_name(""), + package_version(""), force(false), verbose(false), show_help(false) {} diff --git a/modules/build/include/commands.hpp b/modules/build/include/commands.hpp index 4baf72a..b7962c8 100644 --- a/modules/build/include/commands.hpp +++ b/modules/build/include/commands.hpp @@ -26,6 +26,19 @@ int cmd_stage(int argc, char** argv); */ int cmd_help(int argc, char** argv); + +/** + * @brief Handler for the help command + * + * Displays information about available commands in the build module. + * + * @param argc Number of arguments + * @param argv Array of arguments + * @return 0 on success, non-zero on failure + */ +int cmd_stage_help(int argc, char** argv); + + /** * @brief Handler for unknown commands * diff --git a/modules/build/src/cli_parsers.cpp b/modules/build/src/cli_parsers.cpp index d3378bb..6b54e93 100644 --- a/modules/build/src/cli_parsers.cpp +++ b/modules/build/src/cli_parsers.cpp @@ -1,17 +1,15 @@ #include "cli_parsers.hpp" - int parse_create_options(int argc, char** argv, BuildOptions& options) { - // Extend BuildOptions to track which options were provided on command line - struct { - bool output_dir = false; - bool contents_dir = false; - bool hooks_dir = false; - bool package_name = false; - bool force = false; - bool verbose = false; - bool help = false; - } provided; + // Track which options were explicitly provided on command line + bool output_dir_provided = false; + bool contents_dir_provided = false; + bool hooks_dir_provided = false; + bool package_name_provided = false; + bool package_version_provided = false; + bool force_provided = false; + bool verbose_provided = false; + bool help_provided = false; // For debugging dpm_log(LOG_DEBUG, "Parsing command-line arguments"); @@ -34,25 +32,31 @@ int parse_create_options(int argc, char** argv, BuildOptions& options) { if (option == "--output-dir") { options.output_dir = value; - provided.output_dir = true; + output_dir_provided = true; } else if (option == "--contents") { options.contents_dir = value; - provided.contents_dir = true; + contents_dir_provided = true; } else if (option == "--hooks") { options.hooks_dir = value; - provided.hooks_dir = true; + hooks_dir_provided = true; } else if (option == "--name") { options.package_name = value; - provided.package_name = true; + package_name_provided = true; + } else if (option == "--version") { + options.package_version = value; + package_version_provided = true; } else if (option == "--force") { - options.force = true; - provided.force = true; + // Parse the boolean value + options.force = (value == "true" || value == "1" || value == "yes"); + force_provided = true; } else if (option == "--verbose") { - options.verbose = true; - provided.verbose = true; + // Parse the boolean value + options.verbose = (value == "true" || value == "1" || value == "yes"); + verbose_provided = true; } else if (option == "--help") { - options.show_help = true; - provided.help = true; + // Parse the boolean value + options.show_help = (value == "true" || value == "1" || value == "yes"); + help_provided = true; } // Convert this argument to a dummy to prevent getopt from processing it @@ -65,6 +69,7 @@ int parse_create_options(int argc, char** argv, BuildOptions& options) { {"contents", required_argument, 0, 'c'}, {"hooks", required_argument, 0, 'H'}, {"name", required_argument, 0, 'n'}, + {"version", required_argument, 0, 'V'}, {"force", no_argument, 0, 'f'}, {"verbose", no_argument, 0, 'v'}, {"help", no_argument, 0, 'h'}, @@ -79,35 +84,39 @@ int parse_create_options(int argc, char** argv, BuildOptions& options) { int opt; int option_index = 0; - while ((opt = getopt_long(argc, argv, "o:c:H:n:fvh", long_options, &option_index)) != -1) { + while ((opt = getopt_long(argc, argv, "o:c:H:n:V:fvh", long_options, &option_index)) != -1) { switch (opt) { case 'o': options.output_dir = optarg; - provided.output_dir = true; + output_dir_provided = true; break; case 'c': options.contents_dir = optarg; - provided.contents_dir = true; + contents_dir_provided = true; break; case 'H': options.hooks_dir = optarg; - provided.hooks_dir = true; + hooks_dir_provided = true; break; case 'n': options.package_name = optarg; - provided.package_name = true; + package_name_provided = true; + break; + case 'V': + options.package_version = optarg; + package_version_provided = true; break; case 'f': options.force = true; - provided.force = true; + force_provided = true; break; case 'v': options.verbose = true; - provided.verbose = true; + verbose_provided = true; break; case 'h': options.show_help = true; - provided.help = true; + help_provided = true; break; case '?': // Ignore errors as we handle equals-format options separately @@ -130,45 +139,58 @@ int parse_create_options(int argc, char** argv, BuildOptions& options) { options.hooks_dir = expand_path(options.hooks_dir); } - // Log the parsed options for debugging + // Log the parsed options - only include options explicitly provided by the user dpm_log(LOG_DEBUG, "Parsed options:"); + bool any_options_provided = false; - if (provided.output_dir) { + if (output_dir_provided) { dpm_log(LOG_DEBUG, (" output_dir=" + options.output_dir).c_str()); + any_options_provided = true; } - if (provided.contents_dir) { + if (contents_dir_provided) { dpm_log(LOG_DEBUG, (" contents_dir=" + options.contents_dir).c_str()); + any_options_provided = true; } - if (provided.hooks_dir) { + if (hooks_dir_provided) { dpm_log(LOG_DEBUG, (" hooks_dir=" + options.hooks_dir).c_str()); + any_options_provided = true; } - if (provided.package_name) { + if (package_name_provided) { dpm_log(LOG_DEBUG, (" package_name=" + options.package_name).c_str()); + any_options_provided = true; } - if (provided.force) { - dpm_log(LOG_DEBUG, " force=true"); + if (package_version_provided) { + dpm_log(LOG_DEBUG, (" package_version=" + options.package_version).c_str()); + any_options_provided = true; } - if (provided.verbose) { - dpm_log(LOG_DEBUG, " verbose=true"); + if (force_provided) { + dpm_log(LOG_DEBUG, (" force=" + std::string(options.force ? "true" : "false")).c_str()); + any_options_provided = true; } - if (provided.help) { - dpm_log(LOG_DEBUG, " help=true"); + if (verbose_provided) { + dpm_log(LOG_DEBUG, (" verbose=" + std::string(options.verbose ? "true" : "false")).c_str()); + any_options_provided = true; } - if (!provided.output_dir && !provided.contents_dir && !provided.hooks_dir && - !provided.package_name && !provided.force && !provided.verbose && !provided.help) { - dpm_log(LOG_DEBUG, " No options were provided"); + if (help_provided) { + dpm_log(LOG_DEBUG, (" help=" + std::string(options.show_help ? "true" : "false")).c_str()); + any_options_provided = true; + } + + if (!any_options_provided) { + dpm_log(LOG_DEBUG, " No options provided - using defaults"); } return 0; } + Command parse_command(const char* cmd_str) { if (cmd_str == nullptr || strlen(cmd_str) == 0) { return CMD_HELP; @@ -200,6 +222,18 @@ int validate_build_options(const BuildOptions& options) { return 1; } + // Check if package name is provided + if (options.package_name.empty()) { + dpm_log(LOG_ERROR, "Package name is required (--name)"); + return 1; + } + + // Check if package version is provided + if (options.package_version.empty()) { + dpm_log(LOG_ERROR, "Package version is required (--version)"); + return 1; + } + // Check if hooks directory exists if provided if (!options.hooks_dir.empty() && !std::filesystem::exists(options.hooks_dir)) { dpm_log(LOG_ERROR, ("Hooks directory does not exist: " + options.hooks_dir).c_str()); diff --git a/modules/build/src/commands.cpp b/modules/build/src/commands.cpp index 7ac4dfc..f0a2cfa 100644 --- a/modules/build/src/commands.cpp +++ b/modules/build/src/commands.cpp @@ -2,6 +2,9 @@ int cmd_stage(int argc, char** argv) { + // Announce that the stage step is being executed (debug level) + dpm_log(LOG_DEBUG, "Executing stage command"); + // create a container for commandline options BuildOptions options; @@ -13,7 +16,7 @@ int cmd_stage(int argc, char** argv) { // If help was requested, show it and return if (options.show_help) { - return cmd_help(argc, argv); + return cmd_stage_help(argc, argv); } // Set logging level to DEBUG when verbose is enabled @@ -24,6 +27,8 @@ int cmd_stage(int argc, char** argv) { // Validate options int validate_result = validate_build_options(options); if (validate_result != 0) { + // Show help when validation fails + //cmd_help(argc, argv); return validate_result; } @@ -31,13 +36,13 @@ int cmd_stage(int argc, char** argv) { dpm_log(LOG_DEBUG, "Staging DPM package with the following options:"); dpm_log(LOG_DEBUG, (" Output directory: " + options.output_dir).c_str()); dpm_log(LOG_DEBUG, (" Contents directory: " + options.contents_dir).c_str()); + dpm_log(LOG_DEBUG, (" Package name: " + options.package_name).c_str()); + dpm_log(LOG_DEBUG, (" Package version: " + options.package_version).c_str()); if (!options.hooks_dir.empty()) { dpm_log(LOG_DEBUG, (" Hooks directory: " + options.hooks_dir).c_str()); - } - - if (!options.package_name.empty()) { - dpm_log(LOG_DEBUG, (" Package name: " + options.package_name).c_str()); + } else { + dpm_log(LOG_DEBUG, " Hooks directory: N/A"); } if (options.force) { @@ -52,20 +57,16 @@ int cmd_stage(int argc, char** argv) { } int cmd_help(int argc, char** argv) { - dpm_log(LOG_INFO, "DPM Build Module - Creates DPM packages according to specification"); + dpm_log(LOG_INFO, "DPM Build Module - Creates DPM packages according to specification."); + dpm_log(LOG_INFO, ""); dpm_log(LOG_INFO, "Available commands:"); dpm_log(LOG_INFO, " stage - Stage a new DPM package directory"); dpm_log(LOG_INFO, " help - Display this help message"); dpm_log(LOG_INFO, ""); - dpm_log(LOG_INFO, "Usage: dpm build stage [options]"); - dpm_log(LOG_INFO, "Options:"); - dpm_log(LOG_INFO, " -o, --output-dir DIR Directory to save the staged package (default: current directory)"); - dpm_log(LOG_INFO, " -c, --contents DIR Directory with package contents (required)"); - dpm_log(LOG_INFO, " -H, --hooks DIR Directory with package hooks (optional)"); - dpm_log(LOG_INFO, " -n, --name NAME Package name (required if not in metadata)"); - dpm_log(LOG_INFO, " -f, --force Force package staging even if warnings occur"); - dpm_log(LOG_INFO, " -v, --verbose Enable verbose output"); - dpm_log(LOG_INFO, " -h, --help Display this help message"); + dpm_log(LOG_INFO, "Usage: dpm build "); + dpm_log(LOG_INFO, ""); + dpm_log(LOG_INFO, "For command-specific help, use: dpm build --help"); + return 0; } @@ -77,5 +78,17 @@ int cmd_unknown(const char* command, int argc, char** argv) { return 1; } - - +int cmd_stage_help(int argc, char** argv) { + dpm_log(LOG_INFO, "Usage: dpm build stage [options]"); + dpm_log(LOG_INFO, ""); + dpm_log(LOG_INFO, "Options:"); + dpm_log(LOG_INFO, " -o, --output-dir DIR Directory to save the staged package (required)"); + dpm_log(LOG_INFO, " -c, --contents DIR Directory with package contents (required)"); + dpm_log(LOG_INFO, " -H, --hooks DIR Directory with package hooks (optional)"); + dpm_log(LOG_INFO, " -n, --name NAME Package name (required)"); + dpm_log(LOG_INFO, " -V, --version VERSION Package version (required)"); + dpm_log(LOG_INFO, " -f, --force Force package staging even if warnings occur"); + dpm_log(LOG_INFO, " -v, --verbose Enable verbose output"); + dpm_log(LOG_INFO, " -h, --help Display this help message"); + return 0; +} \ No newline at end of file