Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow non standard option names like -option #1078

Merged
merged 15 commits into from
Oct 23, 2024
Merged
2 changes: 2 additions & 0 deletions CPPLINT.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ linelength=120 # As in .clang-format

# Unused filters
filter=-build/c++11 # Reports e.g. chrono and thread, which overlap with Chromium's API. Not applicable to general C++ projects.
filter=-build/c++17 # google only restrictions not relevant
filter=-build/include_order # Requires unusual include order that encourages creating not self-contained headers
filter=-build/include_subdir # Prevents including files in current directory for whatever reason
filter=-readability/nolint # Conflicts with clang-tidy
Expand All @@ -13,3 +14,4 @@ filter=-runtime/string # Requires not using static const strings which makes th
filter=-whitespace/blank_line # Unnecessarily strict with blank lines that otherwise help with readability
filter=-whitespace/indent # Requires strange 3-space indent of private/protected/public markers
filter=-whitespace/parens,-whitespace/braces # Conflict with clang-format
filter=-whitespace/newline # handled by clang-format
10 changes: 8 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,6 @@ installation fuss.
There are some other possible "features" that are intentionally not supported by
this library:

- Non-standard variations on syntax, like `-long` options. This is non-standard
and should be avoided, so that is enforced by this library.
- Completion of partial options, such as Python's `argparse` supplies for
incomplete arguments. It's better not to guess. Most third party command line
parsers for python actually reimplement command line parsing rather than using
Expand Down Expand Up @@ -904,6 +902,14 @@ option_groups. These are:
the form of `/s /long /file:file_name.ext` This option does not change how
options are specified in the `add_option` calls or the ability to process
options in the form of `-s --long --file=file_name.ext`.
- `.allow_non_standard_option_names()`:🚧 Allow specification of single `-` long
form option names. This is not recommended but is available to enable
reworking of existing interfaces. If this modifier is enabled on an app or
subcommand, options or flags can be specified like normal but instead of
throwing an exception, long form single dash option names will be allowed. It
is not allowed to have a single character short option starting with the same
character as a single dash long form name; for example, `-s` and `-single` are
not allowed in the same application.
- `.fallthrough()`: Allow extra unmatched options and positionals to "fall
through" and be matched on a parent option. Subcommands by default are allowed
to "fall through" as in they will first attempt to match on the current
Expand Down
2 changes: 1 addition & 1 deletion azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
- job: CppLint
pool:
vmImage: "ubuntu-latest"
container: sharaku/cpplint:latest
container: helics/buildenv:cpplint
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the container we had been using hasn't been updated in 5 years. Figured we needed something a little newer.

steps:
- bash: cpplint --counting=detailed --recursive examples include/CLI tests
displayName: Checking against google style guide
Expand Down
1 change: 1 addition & 0 deletions examples/custom_parse.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <CLI/CLI.hpp>
#include <iostream>
#include <sstream>
#include <string>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these adding string with no changes? IWYU?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the update to cpplint includes a lot more IWYU checks so this came up.


// example file to demonstrate a custom lexical cast function

Expand Down
1 change: 1 addition & 0 deletions examples/formatter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <CLI/CLI.hpp>
#include <iostream>
#include <memory>
#include <string>

class MyFormatter : public CLI::Formatter {
public:
Expand Down
1 change: 1 addition & 0 deletions examples/inter_argument_order.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <CLI/CLI.hpp>
#include <algorithm>
#include <iostream>
#include <string>
#include <tuple>
#include <vector>

Expand Down
12 changes: 12 additions & 0 deletions include/CLI/App.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,9 @@ class App {
/// This is potentially useful as a modifier subcommand
bool silent_{false};

/// indicator that the subcommand should allow non-standard option arguments, such as -single_dash_flag
bool allow_non_standard_options_{false};

/// Counts the number of times this command/subcommand was parsed
std::uint32_t parsed_{0U};

Expand Down Expand Up @@ -392,6 +395,12 @@ class App {
return this;
}

/// allow non standard option names
App *allow_non_standard_option_names(bool allowed = true) {
allow_non_standard_options_ = allowed;
return this;
}

/// Set the subcommand to be disabled by default, so on clear(), at the start of each parse it is disabled
App *disabled_by_default(bool disable = true) {
if(disable) {
Expand Down Expand Up @@ -1146,6 +1155,9 @@ class App {
/// Get the status of silence
CLI11_NODISCARD bool get_silent() const { return silent_; }

/// Get the status of silence
CLI11_NODISCARD bool get_allow_non_standard_option_names() const { return allow_non_standard_options_; }

/// Get the status of disabled
CLI11_NODISCARD bool get_immediate_callback() const { return immediate_callback_; }

Expand Down
8 changes: 6 additions & 2 deletions include/CLI/Option.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,9 +341,13 @@ class Option : public OptionBase<Option> {
///@}

/// Making an option by hand is not defined, it must be made by the App class
Option(std::string option_name, std::string option_description, callback_t callback, App *parent)
Option(std::string option_name,
std::string option_description,
callback_t callback,
App *parent,
bool allow_non_standard = false)
: description_(std::move(option_description)), parent_(parent), callback_(std::move(callback)) {
std::tie(snames_, lnames_, pname_) = detail::get_names(detail::split_names(option_name));
std::tie(snames_, lnames_, pname_) = detail::get_names(detail::split_names(option_name), allow_non_standard);
}

public:
Expand Down
2 changes: 1 addition & 1 deletion include/CLI/Split.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ CLI11_INLINE std::vector<std::pair<std::string, std::string>> get_default_flag_v

/// Get a vector of short names, one of long names, and a single name
CLI11_INLINE std::tuple<std::vector<std::string>, std::vector<std::string>, std::string>
get_names(const std::vector<std::string> &input);
get_names(const std::vector<std::string> &input, bool allow_non_standard = false);

} // namespace detail
// [CLI11:split_hpp:end]
Expand Down
1 change: 1 addition & 0 deletions include/CLI/Timer.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

#include <array>
#include <chrono>
#include <cstdio>
#include <functional>
#include <iostream>
#include <string>
Expand Down
47 changes: 44 additions & 3 deletions include/CLI/impl/App_inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

// [CLI11:public_includes:set]
#include <algorithm>
#include <iostream>
#include <memory>
#include <string>
#include <utility>
Expand Down Expand Up @@ -161,7 +162,7 @@ CLI11_INLINE Option *App::add_option(std::string option_name,
std::string option_description,
bool defaulted,
std::function<std::string()> func) {
Option myopt{option_name, option_description, option_callback, this};
Option myopt{option_name, option_description, option_callback, this, allow_non_standard_options_};

if(std::find_if(std::begin(options_), std::end(options_), [&myopt](const Option_p &v) { return *v == myopt; }) ==
std::end(options_)) {
Expand Down Expand Up @@ -191,9 +192,34 @@ CLI11_INLINE Option *App::add_option(std::string option_name,
}
}
}
if(allow_non_standard_options_ && !myopt.snames_.empty()) {
for(auto &sname : myopt.snames_) {
if(sname.length() > 1) {
std::string test_name;
test_name.push_back('-');
test_name.push_back(sname.front());
auto *op = get_option_no_throw(test_name);
if(op != nullptr) {
throw(OptionAlreadyAdded("added option interferes with existing short option: " + sname));
}
}
}
for(auto &opt : options_) {
for(const auto &osn : opt->snames_) {
if(osn.size() > 1) {
std::string test_name;
test_name.push_back(osn.front());
if(myopt.check_sname(test_name)) {
throw(OptionAlreadyAdded("added option interferes with existing non standard option: " +
osn));
}
}
}
}
}
options_.emplace_back();
Option_p &option = options_.back();
option.reset(new Option(option_name, option_description, option_callback, this));
option.reset(new Option(option_name, option_description, option_callback, this, allow_non_standard_options_));

// Set the default string capture function
option->default_function(func);
Expand Down Expand Up @@ -1888,7 +1914,8 @@ App::_parse_arg(std::vector<std::string> &args, detail::Classifier current_type,
});

// Option not found
if(op_ptr == std::end(options_)) {
while(op_ptr == std::end(options_)) {
// using while so we can break
for(auto &subc : subcommands_) {
if(subc->name_.empty() && !subc->disabled_) {
if(subc->_parse_arg(args, current_type, local_processing_only)) {
Expand All @@ -1899,6 +1926,20 @@ App::_parse_arg(std::vector<std::string> &args, detail::Classifier current_type,
}
}
}
if(allow_non_standard_options_ && current_type == detail::Classifier::SHORT && current.size() > 2) {
std::string narg_name;
std::string nvalue;
detail::split_long(std::string{'-'} + current, narg_name, nvalue);
op_ptr = std::find_if(std::begin(options_), std::end(options_), [narg_name](const Option_p &opt) {
return opt->check_sname(narg_name);
});
if(op_ptr != std::end(options_)) {
arg_name = narg_name;
value = nvalue;
rest.clear();
break;
}
}

// don't capture missing if this is a nameless subcommand and nameless subcommands can't fallthrough
if(parent_ != nullptr && name_.empty()) {
Expand Down
28 changes: 20 additions & 8 deletions include/CLI/impl/Split_inl.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ CLI11_INLINE std::vector<std::pair<std::string, std::string>> get_default_flag_v
}

CLI11_INLINE std::tuple<std::vector<std::string>, std::vector<std::string>, std::string>
get_names(const std::vector<std::string> &input) {
get_names(const std::vector<std::string> &input, bool allow_non_standard) {

std::vector<std::string> short_names;
std::vector<std::string> long_names;
Expand All @@ -113,23 +113,35 @@ get_names(const std::vector<std::string> &input) {
continue;
}
if(name.length() > 1 && name[0] == '-' && name[1] != '-') {
if(name.length() == 2 && valid_first_char(name[1]))
if(name.length() == 2 && valid_first_char(name[1])) {
short_names.emplace_back(1, name[1]);
else if(name.length() > 2)
throw BadNameString::MissingDash(name);
else
} else if(name.length() > 2) {
if(allow_non_standard) {
name = name.substr(1);
if(valid_name_string(name)) {
short_names.push_back(name);
} else {
throw BadNameString::BadLongName(name);
}
} else {
throw BadNameString::MissingDash(name);
}
} else {
throw BadNameString::OneCharName(name);
}
} else if(name.length() > 2 && name.substr(0, 2) == "--") {
name = name.substr(2);
if(valid_name_string(name))
if(valid_name_string(name)) {
long_names.push_back(name);
else
} else {
throw BadNameString::BadLongName(name);
}
} else if(name == "-" || name == "--" || name == "++") {
throw BadNameString::ReservedName(name);
} else {
if(!pos_name.empty())
if(!pos_name.empty()) {
throw BadNameString::MultiPositionalNames(name);
}
if(valid_name_string(name)) {
pos_name = name;
} else {
Expand Down
51 changes: 51 additions & 0 deletions tests/AppTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@
#include <cstdlib>
#include <limits>
#include <map>
#include <string>
#include <utility>
#include <vector>

TEST_CASE_METHOD(TApp, "OneFlagShort", "[app]") {
app.add_flag("-c,--count");
Expand Down Expand Up @@ -663,6 +666,15 @@ TEST_CASE_METHOD(TApp, "singledash", "[app]") {
} catch(...) {
CHECK(false);
}
app.allow_non_standard_option_names();
try {
app.add_option("-!I{am}bad");
} catch(const CLI::BadNameString &e) {
std::string str = e.what();
CHECK_THAT(str, Contains("!I{am}bad"));
} catch(...) {
CHECK(false);
}
}

TEST_CASE_METHOD(TApp, "FlagLikeOption", "[app]") {
Expand Down Expand Up @@ -2389,6 +2401,45 @@ TEST_CASE_METHOD(TApp, "OrderedModifyingTransforms", "[app]") {
CHECK(std::vector<std::string>({"one21", "two21"}) == val);
}

// non standard options
TEST_CASE_METHOD(TApp, "nonStandardOptions", "[app]") {
std::string string1;
CHECK_THROWS_AS(app.add_option("-single", string1), CLI::BadNameString);
app.allow_non_standard_option_names();
CHECK(app.get_allow_non_standard_option_names());
app.add_option("-single", string1);
args = {"-single", "string1"};

run();

CHECK(string1 == "string1");
}

TEST_CASE_METHOD(TApp, "nonStandardOptions2", "[app]") {
std::vector<std::string> strings;
app.allow_non_standard_option_names();
app.add_option("-single,--single,-m", strings);
args = {"-single", "string1", "--single", "string2"};

run();

CHECK(strings == std::vector<std::string>{"string1", "string2"});
}

TEST_CASE_METHOD(TApp, "nonStandardOptionsIntersect", "[app]") {
std::vector<std::string> strings;
app.allow_non_standard_option_names();
app.add_option("-s,-t");
CHECK_THROWS_AS(app.add_option("-single,--single", strings), CLI::OptionAlreadyAdded);
}

TEST_CASE_METHOD(TApp, "nonStandardOptionsIntersect2", "[app]") {
std::vector<std::string> strings;
app.allow_non_standard_option_names();
app.add_option("-single,--single", strings);
CHECK_THROWS_AS(app.add_option("-s,-t"), CLI::OptionAlreadyAdded);
}

TEST_CASE_METHOD(TApp, "ThrowingTransform", "[app]") {
std::string val;
auto *m = app.add_option("-m,--mess", val);
Expand Down
Loading
Loading