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

Feature: Improved command-line arguments parsing #83

Merged
merged 11 commits into from
Jan 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 122 additions & 50 deletions impl/command_line/arguments.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

using namespace std::literals::string_literals;
using namespace std::literals::string_view_literals;
using optionsVisited_t = substrate::commandLine::arguments_t::optionsVisited_t;

namespace substrate::commandLine
{
Expand Down Expand Up @@ -88,7 +89,7 @@
tokeniser_t lexer{argCount - 1U, argList + 1};
arguments_t result{};
// Try to parse all available arguments against the options tree for the program
if (!result.parseFrom(lexer, options))
if (!result.parseFrom(lexer, options, {}))
return std::nullopt;
return result;
}
Expand Down Expand Up @@ -129,7 +130,7 @@
[[nodiscard]] static auto collectRequiredOptions(const options_t &options) noexcept
{
// Build a set of all the required options defined in this options_t
std::set<internal::optionsItem_t> requiredOptions{};
optionsVisited_t requiredOptions{};
for (const auto &option : options)
{
// If the optionItem_t is a bad variant, ignore it
Expand All @@ -150,7 +151,7 @@
return requiredOptions;
}

static auto displayName(const optionsItem_t &item)
[[nodiscard]] static auto displayName(const optionsItem_t &item)
{
if (item.valueless_by_exception())
return ""s;
Expand All @@ -161,14 +162,44 @@
}, item);
}

[[nodiscard]] static size_t checkExclusivity(const optionsVisited_t &options) noexcept
{
std::set<option_t> exclusiveOptions{};
// Loop through all the visited options
for (const auto &option : options)
{
// Dispatch on the option type
std::visit(match_t
{
[&](const option_t &value)
{
if (value.isExclusive())
exclusiveOptions.insert(value);
},
[](const optionSet_t &) { },
}, option);
}
// Now we've collected all the options that are exclusive, display diagnostics
// if there is more than one in the set and then return how many there are
if (exclusiveOptions.size() > 1)
{
console.error("Multiple mutually exclusive options given together on command line, only one allowed."sv);
console.error("Mutually exclusive options given are:"sv);

Check warning on line 187 in impl/command_line/arguments.cxx

View check run for this annotation

Codecov / codecov/patch

impl/command_line/arguments.cxx#L186-L187

Added lines #L186 - L187 were not covered by tests
for (const auto &option : exclusiveOptions)
console.error(" "sv, option.displayName());
}

Check warning on line 190 in impl/command_line/arguments.cxx

View check run for this annotation

Codecov / codecov/patch

impl/command_line/arguments.cxx#L189-L190

Added lines #L189 - L190 were not covered by tests
return exclusiveOptions.size();
}

// NOLINTNEXTLINE(readability-convert-member-functions-to-static)
bool arguments_t::parseFrom(tokeniser_t &lexer, const options_t &options)
// NOLINTNEXTLINE(performance-unnecessary-value-param)
bool arguments_t::parseFrom(tokeniser_t &lexer, const options_t &options, const optionsVisited_t globalOptions)
amyspark marked this conversation as resolved.
Show resolved Hide resolved
{
const auto &token{lexer.token()};
optionsVisited_t optionsVisited{};
while (token.valid())
{
const auto result{parseArgument(lexer, options, optionsVisited)};
const auto result{parseArgument(lexer, options, globalOptions, optionsVisited)};
// If the result is a nullopt, we're unwinding an inner failure
if (!result)
return false;
Expand All @@ -181,6 +212,14 @@
return false;
}
}
// Having parsed as many options as we can, collect all exclusive options seen and
// perform the exclusivity check (there may not be more than 1 exclusive option given)
const auto exclusiveOptions{checkExclusivity(optionsVisited)};
if (exclusiveOptions > 1U)
return false;

Check warning on line 219 in impl/command_line/arguments.cxx

View check run for this annotation

Codecov / codecov/patch

impl/command_line/arguments.cxx#L219

Added line #L219 was not covered by tests
// If there was just one exclusive option, return true to short-circuit the required options check
if (exclusiveOptions == 1U)
return true;

Check warning on line 222 in impl/command_line/arguments.cxx

View check run for this annotation

Codecov / codecov/patch

impl/command_line/arguments.cxx#L222

Added line #L222 was not covered by tests
// Having parsed as many options as we can, collect all the required options into a set
const auto requiredOptions{collectRequiredOptions(options)};
optionsVisited_t missingOptions{};
Expand All @@ -200,14 +239,15 @@
static std::optional<optionMatch_t> matchOption(tokeniser_t &lexer, const option_t &option,
const std::string_view &argument) noexcept;
static std::optional<optionMatch_t> matchOptionSet(tokeniser_t &lexer, const optionSet_t &option,
const std::string_view &argument) noexcept;
const std::string_view &argument, const options_t &options,
const optionsVisited_t &globalOptions) noexcept;
template<typename set_t> static bool checkMatchValid(const optionsItem_t &option, set_t &optionsVisited) noexcept;
template<typename set_t> static std::optional<bool> handleResult(arguments_t &arguments, const optionsItem_t &option,
set_t &optionsVisited, const std::string_view &argument, const optionMatch_t &match) noexcept;
static void handleUnrecognised(tokeniser_t &lexer, const std::string_view &argument) noexcept;

std::optional<bool> arguments_t::parseArgument(tokeniser_t &lexer, const options_t &options,
optionsVisited_t &optionsVisited) noexcept
const optionsVisited_t &globalOptions, optionsVisited_t &optionsVisited) noexcept
{
// Start by checking we're in a suitable state
const auto &token{lexer.token()};
Expand All @@ -220,34 +260,50 @@
const auto argument{token.value()};
// Initialise look-aside for optionValue_t{} options
std::optional<option_t> valueOption{};
for (const auto &option : options)
const auto matchOptions
{
// Check if this option is an option_t that is valueOnly() (optionValue_t{})
if (std::holds_alternative<option_t>(option))
[&](const auto &optionsSet) -> std::variant<std::monostate, std::optional<bool>>
{
const auto &value{std::get<option_t>(option)};
if (value.valueOnly())
for (const auto &option : optionsSet)
{
valueOption = value;
continue;
// Check if this option is an option_t that is valueOnly() (optionValue_t{})
if (std::holds_alternative<option_t>(option))
{
const auto &value{std::get<option_t>(option)};
if (value.valueOnly())
{
valueOption = value;
continue;
}
}

// Otherwise, process the option normally
const auto match
{
// Dispatch based on the option type
std::visit(match_t
{
[&](const option_t &value) { return matchOption(lexer, value, argument); },
[&](const optionSet_t &value)
{ return matchOptionSet(lexer, value, argument, options, globalOptions); },
}, option)
};

// If we got a valid match, use the result
if (match)
return handleResult(*this, option, optionsVisited, argument, *match);
}
return std::monostate{};
}

// Otherwise, process the option normally
const auto match
{
// Dispatch based on the option type
std::visit(match_t
{
[&](const option_t &value) { return matchOption(lexer, value, argument); },
[&](const optionSet_t &value) { return matchOptionSet(lexer, value, argument); },
}, option)
};

// If we got a valid match, use the result
if (match)
return handleResult(*this, option, optionsVisited, argument, *match);
}
};
// Try matching on the arguments from this level of the recursion
const auto localsMatch{matchOptions(options)};
if (std::holds_alternative<std::optional<bool>>(localsMatch))
return std::get<std::optional<bool>>(localsMatch);
// If that fails, now try matching on the global options
const auto globalsMatch{matchOptions(globalOptions)};
if (std::holds_alternative<std::optional<bool>>(globalsMatch))
return std::get<std::optional<bool>>(globalsMatch);
// If there's an optionValue_t{} and we got no match so far, try matching on it
if (valueOption)
{
Expand Down Expand Up @@ -307,8 +363,36 @@
return flag_t{option.metaName(), std::move(*value)};
}

static auto gatherGlobals(const options_t &options,
const optionsVisited_t &globalOptions) noexcept
{
// Clone the existing set of global options
auto result{globalOptions};
// Loop through the current level's options and pull out any that are global
std::for_each
(
options.begin(),
options.end(),
[&](const internal::optionsItem_t &option)
{
std::visit(match_t
{
[&](const option_t &value)
{
if (value.isGlobal())
result.insert(value);
},
[&](const optionSet_t &) { },
}, option);
}
);
// Having gathered all of them up, return the new set
return result;
}

static std::optional<optionMatch_t> matchOptionSet(tokeniser_t &lexer, const optionSet_t &option,
const std::string_view &argument) noexcept
const std::string_view &argument, const options_t &options,
const optionsVisited_t &globalOptions) noexcept
{
// Check if we're parsing an alternation from a set
const auto match{option.matches(argument)};
Expand All @@ -321,7 +405,7 @@
lexer.next();
arguments_t subarguments{};
const auto &suboptions{alternation.suboptions()};
if (!suboptions.empty() && !subarguments.parseFrom(lexer, suboptions))
if (!suboptions.empty() && !subarguments.parseFrom(lexer, suboptions, gatherGlobals(options, globalOptions)))
// If the operation fails, use monostate to signal match-but-fail.
return std::monostate{};
return choice_t{option.metaName(), argument, std::move(subarguments)};
Expand Down Expand Up @@ -388,12 +472,12 @@
}

// Implementation of the innards of arguments_t as otherwise we get compile errors
// NOLINTNEXTLINE(modernize-use-equals-default)
arguments_t::arguments_t() noexcept : _arguments{} { }
arguments_t::arguments_t(const arguments_t &arguments) noexcept : _arguments{arguments._arguments} { }
arguments_t::arguments_t(arguments_t &&arguments) noexcept : _arguments{std::move(arguments._arguments)} { }
// NOLINTNEXTLINE(modernize-use-equals-default)
arguments_t::~arguments_t() noexcept { }
arguments_t::arguments_t() noexcept = default;
arguments_t::arguments_t(const arguments_t &arguments) = default;
arguments_t::arguments_t(arguments_t &&arguments) noexcept = default;
arguments_t::~arguments_t() noexcept = default;
arguments_t &arguments_t::operator =(const arguments_t &arguments) = default;
arguments_t &arguments_t::operator =(arguments_t &&arguments) noexcept = default;

Check warning on line 480 in impl/command_line/arguments.cxx

View check run for this annotation

Codecov / codecov/patch

impl/command_line/arguments.cxx#L479-L480

Added lines #L479 - L480 were not covered by tests
size_t arguments_t::count() const noexcept
{ return _arguments.size(); }
size_t arguments_t::countMatching(const std::string_view &option) const noexcept
Expand All @@ -405,18 +489,6 @@
arguments_t::iterator_t arguments_t::find(const std::string_view &option) const noexcept
{ return _arguments.find(option); }

arguments_t &arguments_t::operator =(const arguments_t &arguments) noexcept
{
_arguments = arguments._arguments;
return *this;
}

arguments_t &arguments_t::operator =(arguments_t &&arguments) noexcept
{
_arguments = std::move(arguments._arguments);
return *this;
}

// NOLINTNEXTLINE(readability-convert-member-functions-to-static)
std::vector<const item_t *> arguments_t::findAll(const std::string_view &option) const noexcept
{
Expand Down
6 changes: 3 additions & 3 deletions impl/command_line/options.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include <substrate/console>
#include <substrate/conversions>
#include <substrate/index_sequence>
#include <substrate/utility>

using namespace std::literals::string_literals;
using namespace std::literals::string_view_literals;
Expand Down Expand Up @@ -201,6 +202,7 @@ namespace substrate::commandLine
case optionValueType_t::userDefined:
return "VAL"sv;
}
substrate::unreachable();
}

[[nodiscard]] std::string option_t::displayName() const noexcept
Expand All @@ -220,9 +222,7 @@ namespace substrate::commandLine
{
[&](const std::string_view &option) { return std::string{option} + typeValue; },
[&](const optionFlagPair_t &option)
{
return std::string{option._shortFlag} + ", "s + std::string{option._longFlag} + typeValue;
},
{ return std::string{option._shortFlag} + ", "s + std::string{option._longFlag} + typeValue; },
[&](const optionValue_t &option) { return std::string{option.metaName()} + (isRepeatable() ? "..."s : ""s); }
}, _option);
}
Expand Down
15 changes: 9 additions & 6 deletions substrate/command_line/arguments
Original file line number Diff line number Diff line change
Expand Up @@ -40,25 +40,28 @@ namespace substrate::commandLine

struct SUBSTRATE_CLS_API arguments_t
{
public:
using optionsVisited_t = std::set<internal::optionsItem_t>;

private:
using storage_t = std::multiset<item_t, std::less<void>>;
using iterator_t = typename storage_t::const_iterator;
using optionsVisited_t = std::set<internal::optionsItem_t>;

storage_t _arguments;
storage_t _arguments{};

[[nodiscard]] std::optional<bool> parseArgument(internal::tokeniser_t &lexer, const options_t &options,
optionsVisited_t &optionsVisited) noexcept;
const optionsVisited_t &globalOptions, optionsVisited_t &optionsVisited) noexcept;

public:
arguments_t() noexcept;
arguments_t(const arguments_t &arguments) noexcept;
arguments_t(const arguments_t &arguments);
arguments_t(arguments_t &&arguments) noexcept;
~arguments_t() noexcept;
arguments_t &operator =(const arguments_t &arguments) noexcept;
arguments_t &operator =(const arguments_t &arguments);
arguments_t &operator =(arguments_t &&arguments) noexcept;

[[nodiscard]] bool parseFrom(internal::tokeniser_t &lexer, const options_t &options);
[[nodiscard]] bool parseFrom(internal::tokeniser_t &lexer, const options_t &options,
optionsVisited_t globalOptions);
amyspark marked this conversation as resolved.
Show resolved Hide resolved
[[nodiscard]] bool add(item_t argument) noexcept;

[[nodiscard]] size_t count() const noexcept;
Expand Down
18 changes: 18 additions & 0 deletions substrate/command_line/options
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ namespace substrate::commandLine
repeatable,
takesParameter,
required,
global,
exclusive,
};

enum class optionValueType_t
Expand Down Expand Up @@ -125,6 +127,18 @@ namespace substrate::commandLine
return *this;
}

[[nodiscard]] constexpr option_t &global() noexcept
{
_flags.set(optionFlags_t::global);
return *this;
}

[[nodiscard]] constexpr option_t &exclusive() noexcept
{
_flags.set(optionFlags_t::exclusive);
return *this;
}

template<typename T> [[nodiscard]] constexpr option_t &valueRange(T min, T max) noexcept
{
// These perform casts only to silence conversion warnings
Expand All @@ -144,6 +158,10 @@ namespace substrate::commandLine
{ return _flags.includes(optionFlags_t::repeatable); }
[[nodiscard]] constexpr bool isRequired() const noexcept
{ return _flags.includes(optionFlags_t::required); }
[[nodiscard]] constexpr bool isGlobal() const noexcept
{ return _flags.includes(optionFlags_t::global); }
[[nodiscard]] constexpr bool isExclusive() const noexcept
{ return _flags.includes(optionFlags_t::exclusive); }
[[nodiscard]] constexpr bool valueOnly() const noexcept
{ return std::holds_alternative<optionValue_t>(_option); }

Expand Down
Loading