cmcfarlen commented on code in PR #12621:
URL: https://github.com/apache/trafficserver/pull/12621#discussion_r2490840873
##########
include/tscore/ArgParser.h:
##########
@@ -271,6 +327,7 @@ class ArgParser
friend class Command;
friend class Arguments;
+ friend class TestArgParser;
Review Comment:
I don't think this needs to be a friend to work
##########
src/tscore/ArgParser.cc:
##########
@@ -287,6 +306,55 @@ ArgParser::Command::add_option(std::string const
&long_option, std::string const
return *this;
}
+// Create a mutually exclusive group
+ArgParser::Command &
+ArgParser::Command::add_mutex_group(std::string const &group_name, bool
required, std::string const &description)
+{
+ if (group_name.empty()) {
+ std::cerr << "Error: Mutex group name cannot be empty" << std::endl;
+ ArgParser::do_exit(1);
+ }
+
+ if (_mutex_groups.find(group_name) != _mutex_groups.end()) {
+ std::cerr << "Error: Mutex group '" << group_name << "' already exists" <<
std::endl;
+ ArgParser::do_exit(1);
+ }
+ _mutex_groups.emplace(group_name, MutexGroup(group_name, required,
description));
+ return *this;
+}
+
+// Add an option to a mutually exclusive group
+ArgParser::Command &
+ArgParser::Command::add_option_to_group(std::string const &group_name,
std::string const &long_option,
+ std::string const &short_option,
std::string const &description, std::string const &envvar,
+ unsigned arg_num, std::string const
&default_value, std::string const &key)
+{
+ if (group_name.empty()) {
+ std::cerr << "Error: Mutex group name cannot be empty" << std::endl;
+ ArgParser::do_exit(1);
+ }
+
+ auto it_mutex_group = _mutex_groups.find(group_name);
+ if (it_mutex_group == _mutex_groups.end()) {
+ // Auto-create group if it doesn't exist. No need to call add_mutex_group
first.
+ auto ret = _mutex_groups.emplace(group_name, MutexGroup(group_name, false
/** required */));
+ if (!ret.second) {
+ std::cerr << "Error: Failed to auto-create mutex group '" << group_name
<< "'" << std::endl;
+ exit(1);
Review Comment:
do_exit
##########
src/tscore/ArgParser.cc:
##########
@@ -287,6 +306,55 @@ ArgParser::Command::add_option(std::string const
&long_option, std::string const
return *this;
}
+// Create a mutually exclusive group
+ArgParser::Command &
+ArgParser::Command::add_mutex_group(std::string const &group_name, bool
required, std::string const &description)
+{
+ if (group_name.empty()) {
+ std::cerr << "Error: Mutex group name cannot be empty" << std::endl;
+ ArgParser::do_exit(1);
+ }
+
+ if (_mutex_groups.find(group_name) != _mutex_groups.end()) {
+ std::cerr << "Error: Mutex group '" << group_name << "' already exists" <<
std::endl;
+ ArgParser::do_exit(1);
+ }
+ _mutex_groups.emplace(group_name, MutexGroup(group_name, required,
description));
+ return *this;
+}
+
+// Add an option to a mutually exclusive group
+ArgParser::Command &
+ArgParser::Command::add_option_to_group(std::string const &group_name,
std::string const &long_option,
+ std::string const &short_option,
std::string const &description, std::string const &envvar,
+ unsigned arg_num, std::string const
&default_value, std::string const &key)
+{
+ if (group_name.empty()) {
+ std::cerr << "Error: Mutex group name cannot be empty" << std::endl;
+ ArgParser::do_exit(1);
+ }
+
+ auto it_mutex_group = _mutex_groups.find(group_name);
+ if (it_mutex_group == _mutex_groups.end()) {
+ // Auto-create group if it doesn't exist. No need to call add_mutex_group
first.
Review Comment:
Is this a good idea? I'd rather just fail and have the programmer create the
group.
##########
include/tscore/ArgParser.h:
##########
@@ -262,6 +310,14 @@ class ArgParser
void add_description(std::string const &descr);
protected:
+ // Exit handler - throws in test mode, calls exit() otherwise
+ static void do_exit(int code);
+ // Enable test mode - makes do_exit() throw instead of calling exit() for
unit testing
+ static void set_test_mode(bool test = true);
+
+ // When true, do_exit() throws instead of calling exit()
+ static thread_local bool _test_mode;
Review Comment:
Does this need to be thread_local? Could it not just be a member variable on
this instance?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]