Copilot commented on code in PR #12934:
URL: https://github.com/apache/trafficserver/pull/12934#discussion_r2875045532
##########
include/tscore/ArgParser.h:
##########
@@ -226,6 +226,8 @@ class ArgParser
void validate_mutex_groups(Arguments &ret) const;
// Helper method to validate option dependencies
void validate_dependencies(Arguments &ret) const;
+ // Helper method to apply default values for options not explicitly set
+ void apply_option_defaults(Arguments &ret);
Review Comment:
`apply_option_defaults()` can be declared `const` since it doesn't modify
the `Command` object; making it `const` aligns with `validate_mutex_groups()` /
`validate_dependencies()` and prevents accidental state changes later.
```suggestion
void apply_option_defaults(Arguments &ret);
void apply_option_defaults(Arguments &ret) const
{
const_cast<Command *>(this)->apply_option_defaults(ret);
}
```
##########
src/tscore/unit_tests/test_ArgParser.cc:
##########
@@ -148,3 +148,72 @@ TEST_CASE("Invoke test", "[invoke]")
parsed_data.invoke();
REQUIRE(global == 2);
}
+
+TEST_CASE("Case sensitive short options", "[parse]")
+{
+ ts::ArgParser cs_parser;
+ cs_parser.add_global_usage("test_prog [--SWITCH]");
+
+ // Add a command with two options that differ only in case: -t and -T
+ ts::ArgParser::Command &cmd = cs_parser.add_command("process", "process
data");
+ cmd.add_option("--tag", "-t", "a label", "", 1, "");
+ cmd.add_option("--threshold", "-T", "a numeric value", "", 1, "100");
+
+ ts::Arguments parsed;
+
+ // Use lowercase -t: should set "tag" only
+ const char *argv1[] = {"test_prog", "process", "-t", "my_tag", nullptr};
+ parsed = cs_parser.parse(argv1);
+ REQUIRE(parsed.get("tag") == true);
+ REQUIRE(parsed.get("tag").value() == "my_tag");
+ // threshold should still have its default
+ REQUIRE(parsed.get("threshold").value() == "100");
+
+ // Use uppercase -T: should set "threshold" only
+ const char *argv2[] = {"test_prog", "process", "-T", "200", nullptr};
+ parsed = cs_parser.parse(argv2);
+ REQUIRE(parsed.get("threshold") == true);
+ REQUIRE(parsed.get("threshold").value() == "200");
+ // tag should be empty (no default)
+ REQUIRE(parsed.get("tag").value() == "");
+
+ // Use both -t and -T together
+ const char *argv3[] = {"test_prog", "process", "-t", "foo", "-T", "500",
nullptr};
+ parsed = cs_parser.parse(argv3);
+ REQUIRE(parsed.get("tag") == true);
+ REQUIRE(parsed.get("tag").value() == "foo");
+ REQUIRE(parsed.get("threshold") == true);
+ REQUIRE(parsed.get("threshold").value() == "500");
+}
+
+TEST_CASE("with_required does not trigger on default values", "[parse]")
+{
+ ts::ArgParser parser;
+ parser.add_global_usage("test_prog [OPTIONS]");
+
+ ts::ArgParser::Command &cmd = parser.add_command("scan", "scan targets");
+ cmd.add_option("--tag", "-t", "a label", "", 1, "");
+ cmd.add_option("--verbose", "-v", "enable verbose output");
+ cmd.add_option("--threshold", "-T", "a numeric value", "", 1,
"100").with_required("--verbose");
Review Comment:
This test introduces a local variable named `parser` which shadows the
file-scope `ts::ArgParser parser` used by other test cases in this file.
Renaming the local variable (e.g., `local_parser`) would avoid confusion and
reduce the risk of accidental misuse during future edits.
--
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]
