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]

Reply via email to