Skip to content

Commit c99918e

Browse files
phlptppre-commit-ci[bot]
phlptp
and
pre-commit-ci[bot]
authored
add additional tests for coverage, (#928)
Add some additional tests to try to get test coverage back to 100% refactor the positional parsing to reduce code duplication --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent ff1ec84 commit c99918e

File tree

6 files changed

+159
-68
lines changed

6 files changed

+159
-68
lines changed

‎cmake/CodeCoverage.cmake

+1-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ elseif(NOT CMAKE_COMPILER_IS_GNUCXX)
8888
endif()
8989

9090
set(COVERAGE_COMPILER_FLAGS
91-
"-g -O0 --coverage -fprofile-arcs -ftest-coverage -fno-inline -fno-inline-small-functions -fno-default-inline"
91+
"-g -O0 --coverage -fprofile-arcs -ftest-coverage -fno-inline -fno-inline-small-functions -fno-default-inline -fno-elide-constructors"
9292
CACHE INTERNAL "")
9393

9494
set(CMAKE_CXX_FLAGS_COVERAGE

‎include/CLI/impl/App_inl.hpp

+42-50
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ CLI11_NODISCARD CLI11_INLINE char **App::ensure_utf8(char **argv) {
8282
CLI11_INLINE App *App::name(std::string app_name) {
8383

8484
if(parent_ != nullptr) {
85-
auto oname = name_;
85+
std::string oname = name_;
8686
name_ = app_name;
8787
const auto &res = _compare_subcommand_names(*this, *_get_fallthrough_parent());
8888
if(!res.empty()) {
@@ -832,7 +832,7 @@ CLI11_NODISCARD CLI11_INLINE bool App::check_name(std::string name_to_check) con
832832
if(local_name == name_to_check) {
833833
return true;
834834
}
835-
for(auto les : aliases_) { // NOLINT(performance-for-range-copy)
835+
for(std::string les : aliases_) { // NOLINT(performance-for-range-copy)
836836
if(ignore_underscore_) {
837837
les = detail::remove_underscore(les);
838838
}
@@ -1556,6 +1556,7 @@ CLI11_NODISCARD CLI11_INLINE bool App::_has_remaining_positionals() const {
15561556
CLI11_INLINE bool App::_parse_positional(std::vector<std::string> &args, bool haltOnSubcommand) {
15571557

15581558
const std::string &positional = args.back();
1559+
Option *posOpt{nullptr};
15591560

15601561
if(positionals_at_end_) {
15611562
// deal with the case of required arguments at the end which should take precedence over other arguments
@@ -1572,56 +1573,47 @@ CLI11_INLINE bool App::_parse_positional(std::vector<std::string> &args, bool ha
15721573
continue;
15731574
}
15741575
}
1575-
1576-
parse_order_.push_back(opt.get());
1577-
/// if we require a separator add it here
1578-
if(opt->get_inject_separator()) {
1579-
if(!opt->results().empty() && !opt->results().back().empty()) {
1580-
opt->add_result(std::string{});
1581-
}
1582-
}
1583-
if(opt->get_trigger_on_parse() &&
1584-
opt->current_option_state_ == Option::option_state::callback_run) {
1585-
opt->clear();
1586-
}
1587-
opt->add_result(positional);
1588-
if(opt->get_trigger_on_parse()) {
1589-
opt->run_callback();
1590-
}
1591-
args.pop_back();
1592-
return true;
1576+
posOpt = opt.get();
1577+
break;
15931578
}
15941579
}
15951580
}
15961581
}
15971582
}
1598-
for(const Option_p &opt : options_) {
1599-
// Eat options, one by one, until done
1600-
if(opt->get_positional() &&
1601-
(static_cast<int>(opt->count()) < opt->get_items_expected_min() || opt->get_allow_extra_args())) {
1602-
if(validate_positionals_) {
1603-
std::string pos = positional;
1604-
pos = opt->_validate(pos, 0);
1605-
if(!pos.empty()) {
1606-
continue;
1607-
}
1608-
}
1609-
if(opt->get_inject_separator()) {
1610-
if(!opt->results().empty() && !opt->results().back().empty()) {
1611-
opt->add_result(std::string{});
1583+
if(posOpt == nullptr) {
1584+
for(const Option_p &opt : options_) {
1585+
// Eat options, one by one, until done
1586+
if(opt->get_positional() &&
1587+
(static_cast<int>(opt->count()) < opt->get_items_expected_min() || opt->get_allow_extra_args())) {
1588+
if(validate_positionals_) {
1589+
std::string pos = positional;
1590+
pos = opt->_validate(pos, 0);
1591+
if(!pos.empty()) {
1592+
continue;
1593+
}
16121594
}
1595+
posOpt = opt.get();
1596+
break;
16131597
}
1614-
if(opt->get_trigger_on_parse() && opt->current_option_state_ == Option::option_state::callback_run) {
1615-
opt->clear();
1616-
}
1617-
opt->add_result(positional);
1618-
if(opt->get_trigger_on_parse()) {
1619-
opt->run_callback();
1598+
}
1599+
}
1600+
if(posOpt != nullptr) {
1601+
parse_order_.push_back(posOpt);
1602+
if(posOpt->get_inject_separator()) {
1603+
if(!posOpt->results().empty() && !posOpt->results().back().empty()) {
1604+
posOpt->add_result(std::string{});
16201605
}
1621-
parse_order_.push_back(opt.get());
1622-
args.pop_back();
1623-
return true;
16241606
}
1607+
if(posOpt->get_trigger_on_parse() && posOpt->current_option_state_ == Option::option_state::callback_run) {
1608+
posOpt->clear();
1609+
}
1610+
posOpt->add_result(positional);
1611+
if(posOpt->get_trigger_on_parse()) {
1612+
posOpt->run_callback();
1613+
}
1614+
1615+
args.pop_back();
1616+
return true;
16251617
}
16261618

16271619
for(auto &subc : subcommands_) {
@@ -1964,7 +1956,7 @@ CLI11_INLINE void App::_trigger_pre_parse(std::size_t remaining_args) {
19641956
} else if(immediate_callback_) {
19651957
if(!name_.empty()) {
19661958
auto pcnt = parsed_;
1967-
auto extras = std::move(missing_);
1959+
missing_t extras = std::move(missing_);
19681960
clear();
19691961
parsed_ = pcnt;
19701962
pre_parse_called_ = true;
@@ -2150,12 +2142,12 @@ CLI11_INLINE void retire_option(App *app, Option *opt) {
21502142
->allow_extra_args(opt->get_allow_extra_args());
21512143

21522144
app->remove_option(opt);
2153-
auto *opt2 = app->add_option(option_copy->get_name(false, true), "option has been retired and has no effect")
2154-
->type_name("RETIRED")
2155-
->default_str("RETIRED")
2156-
->type_size(option_copy->get_type_size_min(), option_copy->get_type_size_max())
2157-
->expected(option_copy->get_expected_min(), option_copy->get_expected_max())
2158-
->allow_extra_args(option_copy->get_allow_extra_args());
2145+
auto *opt2 = app->add_option(option_copy->get_name(false, true), "option has been retired and has no effect");
2146+
opt2->type_name("RETIRED")
2147+
->default_str("RETIRED")
2148+
->type_size(option_copy->get_type_size_min(), option_copy->get_type_size_max())
2149+
->expected(option_copy->get_expected_min(), option_copy->get_expected_max())
2150+
->allow_extra_args(option_copy->get_allow_extra_args());
21592151

21602152
Validator retired_warning{[opt2](std::string &) {
21612153
std::cout << "WARNING " << opt2->get_name() << " is retired and has no effect\n";

‎include/CLI/impl/Config_inl.hpp

+10-16
Original file line numberDiff line numberDiff line change
@@ -221,15 +221,15 @@ inline std::vector<ConfigItem> ConfigBase::from_config(std::istream &input) cons
221221
}
222222

223223
// Find = in string, split and recombine
224-
auto pos = line.find(valueDelimiter);
225-
if(pos != std::string::npos) {
226-
name = detail::trim_copy(line.substr(0, pos));
227-
std::string item = detail::trim_copy(line.substr(pos + 1));
228-
auto cloc = item.find(commentChar);
229-
if(cloc != std::string::npos) {
230-
item.erase(cloc, std::string::npos); // NOLINT(readability-suspicious-call-argument)
231-
detail::trim(item);
232-
}
224+
auto delimiter_pos = line.find_first_of(valueDelimiter);
225+
auto comment_pos = line.find_first_of(commentChar);
226+
if(comment_pos < delimiter_pos) {
227+
delimiter_pos = std::string::npos;
228+
}
229+
if(delimiter_pos != std::string::npos) {
230+
name = detail::trim_copy(line.substr(0, delimiter_pos));
231+
std::string item = detail::trim_copy(line.substr(delimiter_pos + 1, comment_pos - delimiter_pos - 1));
232+
233233
if(item.size() > 1 && item.front() == aStart) {
234234
for(std::string multiline; item.back() != aEnd && std::getline(input, multiline);) {
235235
detail::trim(multiline);
@@ -244,13 +244,7 @@ inline std::vector<ConfigItem> ConfigBase::from_config(std::istream &input) cons
244244
items_buffer = {item};
245245
}
246246
} else {
247-
name = detail::trim_copy(line);
248-
auto cloc = name.find(commentChar);
249-
if(cloc != std::string::npos) {
250-
name.erase(cloc, std::string::npos); // NOLINT(readability-suspicious-call-argument)
251-
detail::trim(name);
252-
}
253-
247+
name = detail::trim_copy(line.substr(0, comment_pos));
254248
items_buffer = {"true"};
255249
}
256250
if(name.find(parentSeparatorChar) == std::string::npos) {

‎tests/AppTest.cpp

+16-1
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ TEST_CASE_METHOD(TApp, "OneFlagShortValues", "[app]") {
2727
run();
2828
CHECK(app.count("-c") == 1u);
2929
CHECK(app.count("--count") == 1u);
30-
auto v = app["-c"]->results();
30+
const auto &v = app["-c"]->results();
3131
CHECK("v1" == v[0]);
3232

3333
CHECK_THROWS_AS(app["--invalid"], CLI::OptionNotFound);
@@ -1175,6 +1175,21 @@ TEST_CASE_METHOD(TApp, "PositionalAtEnd", "[app]") {
11751175
CHECK_THROWS_AS(run(), CLI::ExtrasError);
11761176
}
11771177

1178+
// Tests positionals at end
1179+
TEST_CASE_METHOD(TApp, "PositionalInjectSeparator", "[app]") {
1180+
std::string options;
1181+
std::vector<std::vector<std::string>> foo;
1182+
1183+
app.add_option("-O", options);
1184+
auto *fooopt = app.add_option("foo", foo);
1185+
fooopt->inject_separator();
1186+
args = {"test1", "-O", "Test", "test2"};
1187+
run();
1188+
1189+
CHECK("Test" == options);
1190+
CHECK(foo.size() == 2U);
1191+
}
1192+
11781193
// Tests positionals at end
11791194
TEST_CASE_METHOD(TApp, "RequiredPositionals", "[app]") {
11801195
std::vector<std::string> sources;

‎tests/ConfigFileTest.cpp

+74
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ TEST_CASE("StringBased: FirstWithComments", "[config]") {
8484
ofile << "one=three\n";
8585
ofile << "two=four\n";
8686
ofile << "; and another one\n";
87+
ofile << " ; and yet another one\n";
8788

8889
ofile.seekg(0, std::ios::beg);
8990

@@ -1804,6 +1805,8 @@ TEST_CASE_METHOD(TApp, "IniNotConfigurable", "[config]") {
18041805
}
18051806

18061807
CHECK_THROWS_AS(run(), CLI::ConfigError);
1808+
app.allow_config_extras(CLI::config_extras_mode::ignore_all);
1809+
CHECK_NOTHROW(run());
18071810
}
18081811

18091812
TEST_CASE_METHOD(TApp, "IniSubFailure", "[config]") {
@@ -2072,6 +2075,66 @@ TEST_CASE_METHOD(TApp, "IniFlags", "[config]") {
20722075
CHECK(five);
20732076
}
20742077

2078+
TEST_CASE_METHOD(TApp, "IniFlagsComment", "[config]") {
2079+
TempFile tmpini{"TestIniTmp.ini"};
2080+
app.set_config("--config", tmpini);
2081+
2082+
{
2083+
std::ofstream out{tmpini};
2084+
out << "[default]" << std::endl;
2085+
out << "two=2 # comment 1" << std::endl;
2086+
out << "three=true" << std::endl;
2087+
out << "four=on #comment 2" << std::endl;
2088+
out << "five #comment 3" << std::endl;
2089+
out << std::endl;
2090+
}
2091+
2092+
int two{0};
2093+
bool three{false}, four{false}, five{false};
2094+
app.add_flag("--two", two);
2095+
app.add_flag("--three", three);
2096+
app.add_flag("--four", four);
2097+
app.add_flag("--five", five);
2098+
2099+
run();
2100+
2101+
CHECK(two == 2);
2102+
CHECK(three);
2103+
CHECK(four);
2104+
CHECK(five);
2105+
}
2106+
2107+
TEST_CASE_METHOD(TApp, "IniFlagsAltComment", "[config]") {
2108+
TempFile tmpini{"TestIniTmp.ini"};
2109+
app.set_config("--config", tmpini);
2110+
2111+
{
2112+
std::ofstream out{tmpini};
2113+
out << "[default]" << std::endl;
2114+
out << "two=2 % comment 1" << std::endl;
2115+
out << "three=true" << std::endl;
2116+
out << "four=on %% comment 2" << std::endl;
2117+
out << "five %= 3" << std::endl;
2118+
out << std::endl;
2119+
}
2120+
2121+
auto config = app.get_config_formatter_base();
2122+
config->comment('%');
2123+
int two{0};
2124+
bool three{false}, four{false}, five{false};
2125+
app.add_flag("--two", two);
2126+
app.add_flag("--three", three);
2127+
app.add_flag("--four", four);
2128+
app.add_flag("--five", five);
2129+
2130+
run();
2131+
2132+
CHECK(two == 2);
2133+
CHECK(three);
2134+
CHECK(four);
2135+
CHECK(five);
2136+
}
2137+
20752138
TEST_CASE_METHOD(TApp, "IniFalseFlags", "[config]") {
20762139
TempFile tmpini{"TestIniTmp.ini"};
20772140
app.set_config("--config", tmpini);
@@ -2299,6 +2362,17 @@ TEST_CASE_METHOD(TApp, "TomlOutputShortSingleDescription", "[config]") {
22992362
CHECK_THAT(str, Contains("# " + description + "\n" + flag + "=false\n"));
23002363
}
23012364

2365+
TEST_CASE_METHOD(TApp, "TomlOutputdefaultOptionString", "[config]") {
2366+
std::string option = "some_option";
2367+
const std::string description = "Some short description.";
2368+
app.add_option("--" + option, description)->run_callback_for_default();
2369+
2370+
run();
2371+
2372+
std::string str = app.config_to_str(true, true);
2373+
CHECK_THAT(str, Contains("# " + description + "\n" + option + "=\"\"\n"));
2374+
}
2375+
23022376
TEST_CASE_METHOD(TApp, "TomlOutputShortDoubleDescription", "[config]") {
23032377
std::string flag1 = "flagnr1";
23042378
std::string flag2 = "flagnr2";

‎tests/HelpTest.cpp

+16
Original file line numberDiff line numberDiff line change
@@ -1336,3 +1336,19 @@ TEST_CASE("TVersion: parse_throw", "[help]") {
13361336
CHECK(1U == cptr->count());
13371337
}
13381338
}
1339+
1340+
TEST_CASE("TVersion: exit", "[help]") {
1341+
1342+
CLI::App app;
1343+
1344+
app.set_version_flag("--version", CLI11_VERSION);
1345+
1346+
try {
1347+
app.parse("--version");
1348+
} catch(const CLI::CallForVersion &v) {
1349+
std::ostringstream out;
1350+
auto ret = app.exit(v, out);
1351+
CHECK_THAT(out.str(), Contains(CLI11_VERSION));
1352+
CHECK(0 == ret);
1353+
}
1354+
}

0 commit comments

Comments
 (0)