Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions libs/argparse/argparse.h
Original file line number Diff line number Diff line change
Expand Up @@ -440,11 +440,15 @@ namespace argparse {
std::vector<std::shared_ptr<Entry>> arg_entries;
std::map<std::string, std::shared_ptr<SubcommandEntry>> subcommand_entries;
bool& _help = flag("?,help", "print help");
bool& _version = flag("version", "print version");

public:
std::string program_name;
bool is_valid = false;

bool help_requested() const { return _help; }
bool version_requested() const { return _version; }

virtual ~Args() = default;

/* Add a positional argument, the order in which it is defined equals the order in which they are being read.
Expand Down Expand Up @@ -693,8 +697,7 @@ namespace argparse {
}
}

if (_help) {
// help();
if (_help || _version) {
return true;
}
Comment on lines +700 to 702

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it's over-engineering. But fine, we can do it.


Expand Down
5 changes: 5 additions & 0 deletions src/green/params/except.h
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,11 @@ namespace green::params {
public:
explicit params_empty_name_error(const std::string& string) : runtime_error(string) {}
};

class params_reserved_name_error : public std::runtime_error {
public:
explicit params_reserved_name_error(const std::string& string) : runtime_error(string) {}
};
} // namespace green::params

#endif // GREEN_PARAMS_EXCEPT_H
54 changes: 47 additions & 7 deletions src/green/params/params.h
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,8 @@ namespace green::params {
*
* @param description - name of the parameters (used for printing)
*/
params(const std::string& description = "") : parsed_(false), built_(false), description_(description), inifile_(nullptr) {
params(const std::string& description = "", const std::string& version = "")
: parsed_(false), built_(false), description_(description), version_(version), inifile_(nullptr) {
Comment thread
gauravharsha marked this conversation as resolved.
inifile_ = &args_.arg_t<std::string>("Parameters INI File").set_default("");
}

Expand All @@ -190,6 +191,10 @@ namespace green::params {
if (name.empty()) {
throw params_empty_name_error("Can not define parameter with an empty name");
}
for (const std::string& n : argparse::split(name)) {
if (n == "help" || n == "?" || n == "version")
throw params_reserved_name_error("Parameter name '" + n + "' is reserved for a built-in flag (--help/--version)");
}
built_ = false;
auto [names, redefinied, old_entry] = check_redefiniton<T>(argparse::split(name));
argparse::Entry* entry = redefinied ? old_entry : &args_.kwarg_t<T>(name, descr);
Expand Down Expand Up @@ -223,6 +228,21 @@ namespace green::params {
*/
std::string description() const { return description_; }

/**
* @return the release version string reported by the `--version` flag
*/
std::string version() const { return version_; }

Comment thread
gauravharsha marked this conversation as resolved.
/**
* @return true if --help was passed on the command line. Meaningful after parse()/build().
*/
bool help_requested() const { return args_.help_requested(); }

/**
* @return true if --version was passed on the command line. Meaningful after parse()/build().
*/
bool version_requested() const { return args_.version_requested(); }

/**
* Subscript operator to access parameter by name. Can be assigned to any type that can accomodate the parameter value.
*
Expand Down Expand Up @@ -287,7 +307,8 @@ namespace green::params {
/**
* Parse command line arguments represented as a string. As usual, the first parameter should be program name.
* @param s - string with command line arguments
* @return false if help requested, true otherwise
* @return false if the caller should stop — an informational flag (--help/--version) was
* requested, or arguments were supplied before any parameters were defined; true otherwise
*/
Comment thread
gauravharsha marked this conversation as resolved.
bool parse(const std::string& s) {
std::string to_parse = s;
Expand All @@ -300,15 +321,16 @@ namespace green::params {
*
* @param argc - number of command line arguments
* @param argv - values of command line arguments
* @return false if help requested, true otherwise
* @return false if the caller should stop — an informational flag (--help/--version) was
* requested, or arguments were supplied before any parameters were defined; true otherwise
*/
bool parse(int argc, char* argv[]) {
args_.parse(argc, argv, false);
parsed_ = true;
if (parameters_map_.empty() && argc > 2)
return false; // we provided command line parameters but haven't defined any them yet
bool help_requested = build();
return !help_requested;
build();
return !help_requested() && !version_requested();
Comment on lines 329 to +333
}

/**
Expand All @@ -326,6 +348,7 @@ namespace green::params {
#endif
if (!built_) build();
std::cout << description_ << std::endl;
if (!version_.empty()) std::cout << "Version: " << version_ << std::endl;
args_.print();
}

Expand All @@ -338,9 +361,23 @@ namespace green::params {
#endif
if (!built_) build();
std::cout << description_ << std::endl;
if (!version_.empty()) std::cout << "Version: " << version_ << std::endl;
args_.help();
}

/**
* Print the informational message that made parse() return false: the release string for
* --version, otherwise the help text. Call this on the rank that owns console output. Future
* informational flags should be handled here so callers can stay agnostic to the specific flag.
*/
void help_or_version() {
if (version_requested()) {
std::cout << version_ << std::endl;
return;
Comment on lines +368 to +376
}
help();
}
Comment thread
gauravharsha marked this conversation as resolved.

[[nodiscard]] const std::unordered_set<std::shared_ptr<params_item>>& params_set() const { return params_set_; }

private:
Expand All @@ -351,10 +388,13 @@ namespace green::params {
std::unordered_set<std::shared_ptr<params_item>> params_set_;
std::string description_;
argparse::Entry* inifile_;
std::string version_ = "";

inline bool build_internal() {
bool help_requested = args_.build(false);
if (help_requested) return true;
bool info = args_.build(false); // true on --help or --version
// Short-circuit before INI handling so informational flags are honored regardless of other
// (possibly invalid) arguments.
if (info) return true;
if (inifile_->has_value() && !inifile_->string_value().value().empty() &&
std::filesystem::exists(inifile_->string_value().value())) {
INI::File ft;
Expand Down
63 changes: 63 additions & 0 deletions test/params_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@

#include <catch2/catch_test_macros.hpp>

#include <sstream>

using namespace std::string_literals;

enum myenum { GREEN, BLACK, YELLOW };
Expand All @@ -16,6 +18,49 @@ TEST_CASE("Params") {
REQUIRE(p.description() == "DESCR");
}

SECTION("Version string from constructor") {
auto p_default = green::params::params("DESCR");
REQUIRE(p_default.version().empty());
auto p = green::params::params("DESCR", "v1.2.3");
REQUIRE(p.version() == "v1.2.3");
}

SECTION("Version flag is reported as a stop signal") {
auto p = green::params::params("DESCR", "v1.2.3");
REQUIRE_FALSE(p.parse("test --version"s)); // false => caller should stop
REQUIRE(p.version_requested());
REQUIRE_FALSE(p.help_requested());

std::ostringstream out;
std::streambuf* old = std::cout.rdbuf(out.rdbuf());
p.help_or_version();
std::cout.rdbuf(old);
Comment thread
gauravharsha marked this conversation as resolved.
REQUIRE(out.str() == "v1.2.3\n");
}

SECTION("Help flag is reported as a stop signal, not version") {
auto p = green::params::params("DESCR", "v1.2.3");
REQUIRE_FALSE(p.parse("test --help"s));
REQUIRE(p.help_requested());
REQUIRE_FALSE(p.version_requested());
}

SECTION("Version flag works with parameters defined and extra args") {
auto p = green::params::params("DESCR", "v1.2.3");
p.define<int>("alpha", "an integer parameter", 0);
REQUIRE_FALSE(p.parse("test --alpha 5 --version"s));
REQUIRE(p.version_requested());
}

SECTION("Reserved flag names cannot be defined as parameters") {
auto p = green::params::params("DESCR", "v1.2.3");
REQUIRE_THROWS_AS(p.define<int>("version", "collides with built-in"), green::params::params_reserved_name_error);
REQUIRE_THROWS_AS(p.define<int>("help", "collides with built-in"), green::params::params_reserved_name_error);
REQUIRE_THROWS_AS(p.define<int>("?", "collides with built-in"), green::params::params_reserved_name_error);
// a reserved name as an alias in a comma-separated definition is also rejected
REQUIRE_THROWS_AS(p.define<int>("vsn,version", "collides via alias"), green::params::params_reserved_name_error);
}

SECTION("String parser") {
SECTION("SIMPLE") {
std::string args = "test --a 33";
Expand Down Expand Up @@ -337,4 +382,22 @@ TEST_CASE("Params") {
std::vector<myenum> a;
REQUIRE_THROWS_AS(a = p["a"], green::params::params_value_error);
}

SECTION("print emits Version line when set, omits when empty") {
auto p = green::params::params("DESCR", "v1.2.3");
p.parse("test"s);
std::ostringstream out;
std::streambuf* old = std::cout.rdbuf(out.rdbuf());
p.print();
std::cout.rdbuf(old);
Comment thread
gauravharsha marked this conversation as resolved.
REQUIRE(out.str().find("Version: v1.2.3") != std::string::npos);

auto p_empty = green::params::params("DESCR");
p_empty.parse("test"s);
std::ostringstream out2;
old = std::cout.rdbuf(out2.rdbuf());
p_empty.print();
std::cout.rdbuf(old);
Comment thread
gauravharsha marked this conversation as resolved.
REQUIRE(out2.str().find("Version:") == std::string::npos);
}
}
Loading