Allow non standard option names like -option (#1078)

This has been bounced around for a couple years now 

#474 and a few others have expressed desire to work with non-standard
option names. We have been somewhat resistant to that but I think it can
be done now. This PR adds a modifier `allow_non_standard_option_names()`
It is purposely long, it is purposely off by default. But what it does
is allow option names with a single `-` to act like a short option name.
With this modifier enabled no single letter short option names are
allowed to start with the same letter as a non-standard names. For
example `-s` and `-single` would not be allowed.

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <[email protected]>

Author: 3 people

CPPLINT.cfg

@@ -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
@@ -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

README.md

@@ -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
@@ -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

azure-pipelines.yml

@@ -22,7 +22,7 @@ jobs:
   - job: CppLint
     pool:
       vmImage: "ubuntu-latest"
-    container: sharaku/cpplint:latest
+    container: helics/buildenv:cpplint
     steps:
       - bash: cpplint --counting=detailed --recursive examples include/CLI tests
         displayName: Checking against google style guide

examples/custom_parse.cpp

@@ -9,6 +9,7 @@
 #include <CLI/CLI.hpp>
 #include <iostream>
 #include <sstream>
+#include <string>
 
 // example file to demonstrate a custom lexical cast function
 

examples/formatter.cpp

@@ -7,6 +7,7 @@
 #include <CLI/CLI.hpp>
 #include <iostream>
 #include <memory>
+#include <string>
 
 class MyFormatter : public CLI::Formatter {
   public:

examples/inter_argument_order.cpp

@@ -7,6 +7,7 @@
 #include <CLI/CLI.hpp>
 #include <algorithm>
 #include <iostream>
+#include <string>
 #include <tuple>
 #include <vector>
 

include/CLI/App.hpp

@@ -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};
 
@@ -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) {
@@ -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_; }
 

include/CLI/Option.hpp

@@ -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:

include/CLI/Split.hpp

@@ -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]

include/CLI/Timer.hpp

@@ -18,6 +18,7 @@
 
 #include <array>
 #include <chrono>
+#include <cstdio>
 #include <functional>
 #include <iostream>
 #include <string>

include/CLI/impl/App_inl.hpp

@@ -16,6 +16,7 @@
 
 // [CLI11:public_includes:set]
 #include <algorithm>
+#include <iostream>
 #include <memory>
 #include <string>
 #include <utility>
@@ -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_)) {
@@ -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);
@@ -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)) {
@@ -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()) {

include/CLI/impl/Split_inl.hpp

@@ -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;
@@ -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 {

tests/AppTest.cpp

@@ -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");
@@ -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]") {
@@ -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);