Skip to content

Commit 158bc56

Browse files
committed
ArgParser: apply default values after dependency validation
validate_dependencies() incorrectly triggers on options that have default values but were not explicitly passed by the user. The root cause is that append_option_data() populates default values into the Arguments map before validate_dependencies() runs. When validate_dependencies() calls ret.get(key) for an option with a default, the lookup finds the entry and sets _is_called = true, making the option appear "used" even though the user never specified it on the command line. Fix by extracting the default-value loop into apply_option_defaults() and calling it after validate_dependencies() in parse().
1 parent 00604e1 commit 158bc56

3 files changed

Lines changed: 84 additions & 1 deletion

File tree

include/tscore/ArgParser.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,8 @@ class ArgParser
226226
void validate_mutex_groups(Arguments &ret) const;
227227
// Helper method to validate option dependencies
228228
void validate_dependencies(Arguments &ret) const;
229+
// Helper method to apply default values for options not explicitly set
230+
void apply_option_defaults(Arguments &ret) const;
229231
// The command name and help message
230232
std::string _name;
231233
std::string _description;

src/tscore/ArgParser.cc

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -725,7 +725,14 @@ ArgParser::Command::append_option_data(Arguments &ret, AP_StrVec &args, int inde
725725
help_message(std::to_string(_option_list.at(it.first).arg_num) + " arguments expected by " + it.first);
726726
}
727727
}
728-
// put in the default value of options
728+
}
729+
730+
// Apply default values for options not explicitly set by the user.
731+
// This must be called AFTER validate_dependencies() so that default values
732+
// (e.g. --timeout "0") don't falsely trigger dependency checks.
733+
void
734+
ArgParser::Command::apply_option_defaults(Arguments &ret) const
735+
{
729736
for (const auto &it : _option_list) {
730737
if (!it.second.default_value.empty() && ret.get(it.second.key).empty()) {
731738
std::istringstream ss(it.second.default_value);
@@ -771,6 +778,11 @@ ArgParser::Command::parse(Arguments &ret, AP_StrVec &args)
771778

772779
// Validate option dependencies
773780
validate_dependencies(ret);
781+
782+
// Apply default values after validation so that defaults don't
783+
// trigger dependency checks (e.g. --timeout with default "0"
784+
// should not require --monitor when not explicitly used).
785+
apply_option_defaults(ret);
774786
}
775787

776788
if (command_called) {

src/tscore/unit_tests/test_ArgParser.cc

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,3 +148,72 @@ TEST_CASE("Invoke test", "[invoke]")
148148
parsed_data.invoke();
149149
REQUIRE(global == 2);
150150
}
151+
152+
TEST_CASE("Case sensitive short options", "[parse]")
153+
{
154+
ts::ArgParser cs_parser;
155+
cs_parser.add_global_usage("test_prog [--SWITCH]");
156+
157+
// Add a command with two options that differ only in case: -t and -T
158+
ts::ArgParser::Command &cmd = cs_parser.add_command("process", "process data");
159+
cmd.add_option("--tag", "-t", "a label", "", 1, "");
160+
cmd.add_option("--threshold", "-T", "a numeric value", "", 1, "100");
161+
162+
ts::Arguments parsed;
163+
164+
// Use lowercase -t: should set "tag" only
165+
const char *argv1[] = {"test_prog", "process", "-t", "my_tag", nullptr};
166+
parsed = cs_parser.parse(argv1);
167+
REQUIRE(parsed.get("tag") == true);
168+
REQUIRE(parsed.get("tag").value() == "my_tag");
169+
// threshold should still have its default
170+
REQUIRE(parsed.get("threshold").value() == "100");
171+
172+
// Use uppercase -T: should set "threshold" only
173+
const char *argv2[] = {"test_prog", "process", "-T", "200", nullptr};
174+
parsed = cs_parser.parse(argv2);
175+
REQUIRE(parsed.get("threshold") == true);
176+
REQUIRE(parsed.get("threshold").value() == "200");
177+
// tag should be empty (no default)
178+
REQUIRE(parsed.get("tag").value() == "");
179+
180+
// Use both -t and -T together
181+
const char *argv3[] = {"test_prog", "process", "-t", "foo", "-T", "500", nullptr};
182+
parsed = cs_parser.parse(argv3);
183+
REQUIRE(parsed.get("tag") == true);
184+
REQUIRE(parsed.get("tag").value() == "foo");
185+
REQUIRE(parsed.get("threshold") == true);
186+
REQUIRE(parsed.get("threshold").value() == "500");
187+
}
188+
189+
TEST_CASE("with_required does not trigger on default values", "[parse]")
190+
{
191+
ts::ArgParser parser;
192+
parser.add_global_usage("test_prog [OPTIONS]");
193+
194+
ts::ArgParser::Command &cmd = parser.add_command("scan", "scan targets");
195+
cmd.add_option("--tag", "-t", "a label", "", 1, "");
196+
cmd.add_option("--verbose", "-v", "enable verbose output");
197+
cmd.add_option("--threshold", "-T", "a numeric value", "", 1, "100").with_required("--verbose");
198+
199+
// -t alone should NOT trigger --threshold's dependency on --verbose.
200+
// The default value "100" for --threshold must not count as "explicitly used".
201+
const char *argv1[] = {"test_prog", "scan", "-t", "my_tag", nullptr};
202+
ts::Arguments parsed = parser.parse(argv1);
203+
REQUIRE(parsed.get("tag").value() == "my_tag");
204+
// threshold default should still be applied after validation
205+
REQUIRE(parsed.get("threshold").value() == "100");
206+
207+
// -T with -v should work fine
208+
const char *argv2[] = {"test_prog", "scan", "-T", "200", "-v", nullptr};
209+
parsed = parser.parse(argv2);
210+
REQUIRE(parsed.get("threshold").value() == "200");
211+
REQUIRE(parsed.get("verbose") == true);
212+
213+
// -t and -T together with -v should work
214+
const char *argv3[] = {"test_prog", "scan", "-t", "foo", "-T", "300", "-v", nullptr};
215+
parsed = parser.parse(argv3);
216+
REQUIRE(parsed.get("tag").value() == "foo");
217+
REQUIRE(parsed.get("threshold").value() == "300");
218+
REQUIRE(parsed.get("verbose") == true);
219+
}

0 commit comments

Comments
 (0)