Skip to content
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- Option values are no longer mis-collected as positional arguments.
`parse_into` now walks argv in a single left-to-right pass with a shared
cursor, so a flag's value (e.g. the `foo.h` in `--include foo.h target.cpp`)
can't also be assigned to a positional.
- Unknown options and excess positionals are now reported as errors instead
of being silently ignored.

### Changed
- Parser rearchitected around a compile-time **parse plan**. `cli::build<T>()`
produces a `program_descriptor` (flat `option_descriptor` / `positional_descriptor`
Expand Down
232 changes: 106 additions & 126 deletions src/cli/parse.hpp
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,11 +12,9 @@
#include <cstdlib>
#include <meta>
#include <print>
#include <ranges>
#include <span>
#include <string>
#include <string_view>
#include <utility>
#include <vector>

namespace cli::detail {
Expand DownExpand Up@@ -63,151 +61,134 @@ inline auto extract_value(std::span<const std::string_view> args, std::size_t po
return {.value = {}, .extra_consumed = 0, .found = false};
}

// Find the option descriptor matching an option token, or nullptr.
// Handles long (`--name`, `--name=v`), short (`-x`), and clustered counter
// short forms (`-vvv`). Clustered different shorts (`-abc`) are not yet
// supported (see P2).
inline auto match_option(std::span<const option_descriptor> options, std::string_view arg)
-> const option_descriptor * {
for (const auto &opt : options) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
return &opt;
}
}
// clustered counter short form
if (std::size(arg) > 2 && arg[0] == '-' && arg[1] != '-') {
for (const auto &opt : options) {
if (opt.kind != option_kind::counter || opt.short_name != arg[1]) {
continue;
}
bool all_same = true;
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] != opt.short_name) {
all_same = false;
break;
}
}
if (all_same) {
return &opt;
}
}
}
return nullptr;
}

// single struct parser
template <class Opts>
auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_view> args)
-> void {
static constexpr auto plan = cli::build<Opts>();

// Built-in --help / -h wins from any position, regardless of other errors.
for (auto arg : args) {
if (arg == "--help" || arg == "-h") {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}

// collect anything that doesn't start with a '-'
std::vector<std::string_view> positionals{};
positionals.reserve(std::size(args));
for (auto arg : args) {
if (!arg.starts_with('-') || arg == "-") {
positionals.push_back(arg);
}
}

// classify tokens from left to right as an option, positional, or unknown.
std::vector<bool> option_seen(std::size(plan.options), false);
std::vector<bool> positional_filled(std::size(plan.positionals), false);
std::size_t positional_idx{0};

// switch over option_kind for matching and use the descriptor's invoke for parsing
static constexpr auto plan = cli::build<Opts>();
for (const auto &opt : plan.options) {
switch (opt.kind) {
case option_kind::help:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::version:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::bool_flag:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
opt.invoke(&opts, "");
break;
}
}
for (std::size_t i = 0; i < std::size(args); ++i) {
auto arg = args[i];
bool is_option = arg.starts_with('-') && arg != "-";

if (is_option) {
const option_descriptor *opt = match_option(plan.options, arg);
if (opt == nullptr) {
std::println(stderr, "Unknown option {}", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
option_seen[static_cast<std::size_t>(opt - std::data(plan.options))] = true;

switch (opt->kind) {
case option_kind::help:
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
case option_kind::version:
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
case option_kind::bool_flag:
opt->invoke(&opts, "");
break;
}
case option_kind::counter:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name)) {
opt.invoke(&opts, "");
} else if (arg.starts_with('-') && !arg.starts_with("--")) {
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] == opt.short_name) {
opt.invoke(&opts, "");
}
}
case option_kind::counter:
{
std::size_t times = matches_long(arg, opt->long_name) ? 1 : (std::size(arg) - 1);
for (std::size_t t = 0; t < times; ++t) {
opt->invoke(&opts, "");
}
break;
}
break;
}
case option_kind::scalar:
{
bool matched{false};
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
matched = true;
break;
case option_kind::scalar:
case option_kind::vector:
{
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", arg);
std::exit(EXIT_FAILURE);
}
}
if (!matched && opt.is_required) {
std::println(stderr, "Missing required option --{}", opt.long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
break;
}
case option_kind::vector:
{
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
if (!opt->invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", arg, vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
break;
}
break;
}
}
}

// Plan-driven dispatch for positionals. Vector positionals drain all
// remaining slots; scalar positionals take the next slot in declaration
// order.
for (const auto &pos : plan.positionals) {
if (pos.is_vector) {
while (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
std::exit(EXIT_FAILURE);
}
++positional_idx;
}
} else if (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
} else {
// handle positional tokens
if (positional_idx >= std::size(plan.positionals)) {
std::println(stderr, "Unexpected argument \"{}\"", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
++positional_idx;
} else if (pos.is_required) {
std::println(stderr, "Missing required positional argument `{}`", pos.name);
const auto &pos = plan.positionals[positional_idx];
if (!pos.invoke(&opts, arg)) {
std::println(stderr, "Failed to parse positional `{}` from \"{}\"", pos.name, arg);
std::exit(EXIT_FAILURE);
}
positional_filled[positional_idx] = true;
if (!pos.is_vector) {
++positional_idx;
}
}
}

// required options/positionals
for (std::size_t o = 0; o < std::size(plan.options); ++o) {
if (plan.options[o].is_required && !option_seen[o]) {
std::println(stderr, "Missing required option --{}", plan.options[o].long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
}
for (std::size_t p = 0; p < std::size(plan.positionals); ++p) {
if (plan.positionals[p].is_required && !positional_filled[p]) {
std::println(stderr, "Missing required positional argument `{}`", plan.positionals[p].name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
Expand All@@ -216,7 +197,6 @@ auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_
// TODO: range / choices / validate_with enforcement.
// TODO: env-var fallback when not matched.
// TODO: on_match action invocation.
// TODO: report unknown options not matched by any member.
}

// handle subcommand
Expand Down
82 changes: 82 additions & 0 deletions tests/test_positionals.cpp
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
// Regression tests for argument consumption: a flag's value must never be
// mis-collected as a positional (B1), and positionals interleaved with flags
// must still land in the right members.

#include <cli/cli.hpp>
#include <gtest/gtest.h>

#include <array>
#include <span>
#include <string>
#include <string_view>
#include <vector>

namespace {

struct [[=cli::program("test")]] Opts {
[[=cli::desc("Source file"), =cli::positional, =cli::required]]
std::string source;

[[=cli::desc("Include dirs"), =cli::short_name('I'), =cli::long_name("include")]]
std::vector<std::string> includes;

[[=cli::desc("Port"), =cli::short_name('p')]]
int port = 0;

[[=cli::desc("Verbose"), =cli::short_name('v')]]
bool verbose = false;
};

template <std::size_t N>
auto parse(std::array<std::string_view, N> const &args) -> Opts {
return cli::parse<Opts>(std::string_view{"test"}, std::span{args.data(), args.size()});
}

} // namespace

// The core B1 bug: `--include foo.h target.cpp` must put target.cpp in the
// positional and foo.h in the vector option — not foo.h in both.
TEST(positionals, flag_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--include", "foo.h", "target.cpp"});
EXPECT_EQ(opts.source, "target.cpp");
ASSERT_EQ(opts.includes.size(), 1u);
EXPECT_EQ(opts.includes[0], "foo.h");
}

// Scalar flag value (a number) must not be grabbed as the positional.
TEST(positionals, scalar_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--port", "9090", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Positional before the flag.
TEST(positionals, positional_before_flag) {
auto opts = parse(std::array<std::string_view, 3>{"main.cpp", "--port", "9090"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Bool flag interleaved with positional — bool consumes no value.
TEST(positionals, bool_flag_does_not_eat_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--verbose", "main.cpp"});
EXPECT_TRUE(opts.verbose);
EXPECT_EQ(opts.source, "main.cpp");
}

// --name=value form leaves the following token as the positional.
TEST(positionals, equals_form_leaves_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--port=9090", "main.cpp"});
EXPECT_EQ(opts.port, 9090);
EXPECT_EQ(opts.source, "main.cpp");
}

// Multiple repeated vector flags plus a trailing positional.
TEST(positionals, repeated_vector_flag_then_positional) {
auto opts =
parse(std::array<std::string_view, 5>{"-I", "a", "--include", "b", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
ASSERT_EQ(opts.includes.size(), 2u);
EXPECT_EQ(opts.includes[0], "a");
EXPECT_EQ(opts.includes[1], "b");
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Better token classification. Moved required option/positional checks.… by aaronaranda · Pull Request #3 · aaronaranda/libcli · GitHub
Skip to content
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- Option values are no longer mis-collected as positional arguments.
`parse_into` now walks argv in a single left-to-right pass with a shared
cursor, so a flag's value (e.g. the `foo.h` in `--include foo.h target.cpp`)
can't also be assigned to a positional.
- Unknown options and excess positionals are now reported as errors instead
of being silently ignored.

### Changed
- Parser rearchitected around a compile-time **parse plan**. `cli::build<T>()`
produces a `program_descriptor` (flat `option_descriptor` / `positional_descriptor`
Expand Down
232 changes: 106 additions & 126 deletions src/cli/parse.hpp
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,11 +12,9 @@
#include <cstdlib>
#include <meta>
#include <print>
#include <ranges>
#include <span>
#include <string>
#include <string_view>
#include <utility>
#include <vector>

namespace cli::detail {
Expand DownExpand Up@@ -63,151 +61,134 @@ inline auto extract_value(std::span<const std::string_view> args, std::size_t po
return {.value = {}, .extra_consumed = 0, .found = false};
}

// Find the option descriptor matching an option token, or nullptr.
// Handles long (`--name`, `--name=v`), short (`-x`), and clustered counter
// short forms (`-vvv`). Clustered different shorts (`-abc`) are not yet
// supported (see P2).
inline auto match_option(std::span<const option_descriptor> options, std::string_view arg)
-> const option_descriptor * {
for (const auto &opt : options) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
return &opt;
}
}
// clustered counter short form
if (std::size(arg) > 2 && arg[0] == '-' && arg[1] != '-') {
for (const auto &opt : options) {
if (opt.kind != option_kind::counter || opt.short_name != arg[1]) {
continue;
}
bool all_same = true;
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] != opt.short_name) {
all_same = false;
break;
}
}
if (all_same) {
return &opt;
}
}
}
return nullptr;
}

// single struct parser
template <class Opts>
auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_view> args)
-> void {
static constexpr auto plan = cli::build<Opts>();

// Built-in --help / -h wins from any position, regardless of other errors.
for (auto arg : args) {
if (arg == "--help" || arg == "-h") {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}

// collect anything that doesn't start with a '-'
std::vector<std::string_view> positionals{};
positionals.reserve(std::size(args));
for (auto arg : args) {
if (!arg.starts_with('-') || arg == "-") {
positionals.push_back(arg);
}
}

// classify tokens from left to right as an option, positional, or unknown.
std::vector<bool> option_seen(std::size(plan.options), false);
std::vector<bool> positional_filled(std::size(plan.positionals), false);
std::size_t positional_idx{0};

// switch over option_kind for matching and use the descriptor's invoke for parsing
static constexpr auto plan = cli::build<Opts>();
for (const auto &opt : plan.options) {
switch (opt.kind) {
case option_kind::help:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::version:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::bool_flag:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
opt.invoke(&opts, "");
break;
}
}
for (std::size_t i = 0; i < std::size(args); ++i) {
auto arg = args[i];
bool is_option = arg.starts_with('-') && arg != "-";

if (is_option) {
const option_descriptor *opt = match_option(plan.options, arg);
if (opt == nullptr) {
std::println(stderr, "Unknown option {}", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
option_seen[static_cast<std::size_t>(opt - std::data(plan.options))] = true;

switch (opt->kind) {
case option_kind::help:
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
case option_kind::version:
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
case option_kind::bool_flag:
opt->invoke(&opts, "");
break;
}
case option_kind::counter:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name)) {
opt.invoke(&opts, "");
} else if (arg.starts_with('-') && !arg.starts_with("--")) {
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] == opt.short_name) {
opt.invoke(&opts, "");
}
}
case option_kind::counter:
{
std::size_t times = matches_long(arg, opt->long_name) ? 1 : (std::size(arg) - 1);
for (std::size_t t = 0; t < times; ++t) {
opt->invoke(&opts, "");
}
break;
}
break;
}
case option_kind::scalar:
{
bool matched{false};
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
matched = true;
break;
case option_kind::scalar:
case option_kind::vector:
{
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", arg);
std::exit(EXIT_FAILURE);
}
}
if (!matched && opt.is_required) {
std::println(stderr, "Missing required option --{}", opt.long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
break;
}
case option_kind::vector:
{
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
if (!opt->invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", arg, vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
break;
}
break;
}
}
}

// Plan-driven dispatch for positionals. Vector positionals drain all
// remaining slots; scalar positionals take the next slot in declaration
// order.
for (const auto &pos : plan.positionals) {
if (pos.is_vector) {
while (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
std::exit(EXIT_FAILURE);
}
++positional_idx;
}
} else if (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
} else {
// handle positional tokens
if (positional_idx >= std::size(plan.positionals)) {
std::println(stderr, "Unexpected argument \"{}\"", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
++positional_idx;
} else if (pos.is_required) {
std::println(stderr, "Missing required positional argument `{}`", pos.name);
const auto &pos = plan.positionals[positional_idx];
if (!pos.invoke(&opts, arg)) {
std::println(stderr, "Failed to parse positional `{}` from \"{}\"", pos.name, arg);
std::exit(EXIT_FAILURE);
}
positional_filled[positional_idx] = true;
if (!pos.is_vector) {
++positional_idx;
}
}
}

// required options/positionals
for (std::size_t o = 0; o < std::size(plan.options); ++o) {
if (plan.options[o].is_required && !option_seen[o]) {
std::println(stderr, "Missing required option --{}", plan.options[o].long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
}
for (std::size_t p = 0; p < std::size(plan.positionals); ++p) {
if (plan.positionals[p].is_required && !positional_filled[p]) {
std::println(stderr, "Missing required positional argument `{}`", plan.positionals[p].name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
Expand All@@ -216,7 +197,6 @@ auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_
// TODO: range / choices / validate_with enforcement.
// TODO: env-var fallback when not matched.
// TODO: on_match action invocation.
// TODO: report unknown options not matched by any member.
}

// handle subcommand
Expand Down
82 changes: 82 additions & 0 deletions tests/test_positionals.cpp
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
// Regression tests for argument consumption: a flag's value must never be
// mis-collected as a positional (B1), and positionals interleaved with flags
// must still land in the right members.

#include <cli/cli.hpp>
#include <gtest/gtest.h>

#include <array>
#include <span>
#include <string>
#include <string_view>
#include <vector>

namespace {

struct [[=cli::program("test")]] Opts {
[[=cli::desc("Source file"), =cli::positional, =cli::required]]
std::string source;

[[=cli::desc("Include dirs"), =cli::short_name('I'), =cli::long_name("include")]]
std::vector<std::string> includes;

[[=cli::desc("Port"), =cli::short_name('p')]]
int port = 0;

[[=cli::desc("Verbose"), =cli::short_name('v')]]
bool verbose = false;
};

template <std::size_t N>
auto parse(std::array<std::string_view, N> const &args) -> Opts {
return cli::parse<Opts>(std::string_view{"test"}, std::span{args.data(), args.size()});
}

} // namespace

// The core B1 bug: `--include foo.h target.cpp` must put target.cpp in the
// positional and foo.h in the vector option — not foo.h in both.
TEST(positionals, flag_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--include", "foo.h", "target.cpp"});
EXPECT_EQ(opts.source, "target.cpp");
ASSERT_EQ(opts.includes.size(), 1u);
EXPECT_EQ(opts.includes[0], "foo.h");
}

// Scalar flag value (a number) must not be grabbed as the positional.
TEST(positionals, scalar_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--port", "9090", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Positional before the flag.
TEST(positionals, positional_before_flag) {
auto opts = parse(std::array<std::string_view, 3>{"main.cpp", "--port", "9090"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Bool flag interleaved with positional — bool consumes no value.
TEST(positionals, bool_flag_does_not_eat_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--verbose", "main.cpp"});
EXPECT_TRUE(opts.verbose);
EXPECT_EQ(opts.source, "main.cpp");
}

// --name=value form leaves the following token as the positional.
TEST(positionals, equals_form_leaves_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--port=9090", "main.cpp"});
EXPECT_EQ(opts.port, 9090);
EXPECT_EQ(opts.source, "main.cpp");
}

// Multiple repeated vector flags plus a trailing positional.
TEST(positionals, repeated_vector_flag_then_positional) {
auto opts =
parse(std::array<std::string_view, 5>{"-I", "a", "--include", "b", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
ASSERT_EQ(opts.includes.size(), 2u);
EXPECT_EQ(opts.includes[0], "a");
EXPECT_EQ(opts.includes[1], "b");
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Better token classification. Moved required option/positional checks.… by aaronaranda · Pull Request #3 · aaronaranda/libcli · GitHub
Skip to content
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- Option values are no longer mis-collected as positional arguments.
`parse_into` now walks argv in a single left-to-right pass with a shared
cursor, so a flag's value (e.g. the `foo.h` in `--include foo.h target.cpp`)
can't also be assigned to a positional.
- Unknown options and excess positionals are now reported as errors instead
of being silently ignored.

### Changed
- Parser rearchitected around a compile-time **parse plan**. `cli::build<T>()`
produces a `program_descriptor` (flat `option_descriptor` / `positional_descriptor`
Expand Down
232 changes: 106 additions & 126 deletions src/cli/parse.hpp
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,11 +12,9 @@
#include <cstdlib>
#include <meta>
#include <print>
#include <ranges>
#include <span>
#include <string>
#include <string_view>
#include <utility>
#include <vector>

namespace cli::detail {
Expand DownExpand Up@@ -63,151 +61,134 @@ inline auto extract_value(std::span<const std::string_view> args, std::size_t po
return {.value = {}, .extra_consumed = 0, .found = false};
}

// Find the option descriptor matching an option token, or nullptr.
// Handles long (`--name`, `--name=v`), short (`-x`), and clustered counter
// short forms (`-vvv`). Clustered different shorts (`-abc`) are not yet
// supported (see P2).
inline auto match_option(std::span<const option_descriptor> options, std::string_view arg)
-> const option_descriptor * {
for (const auto &opt : options) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
return &opt;
}
}
// clustered counter short form
if (std::size(arg) > 2 && arg[0] == '-' && arg[1] != '-') {
for (const auto &opt : options) {
if (opt.kind != option_kind::counter || opt.short_name != arg[1]) {
continue;
}
bool all_same = true;
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] != opt.short_name) {
all_same = false;
break;
}
}
if (all_same) {
return &opt;
}
}
}
return nullptr;
}

// single struct parser
template <class Opts>
auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_view> args)
-> void {
static constexpr auto plan = cli::build<Opts>();

// Built-in --help / -h wins from any position, regardless of other errors.
for (auto arg : args) {
if (arg == "--help" || arg == "-h") {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}

// collect anything that doesn't start with a '-'
std::vector<std::string_view> positionals{};
positionals.reserve(std::size(args));
for (auto arg : args) {
if (!arg.starts_with('-') || arg == "-") {
positionals.push_back(arg);
}
}

// classify tokens from left to right as an option, positional, or unknown.
std::vector<bool> option_seen(std::size(plan.options), false);
std::vector<bool> positional_filled(std::size(plan.positionals), false);
std::size_t positional_idx{0};

// switch over option_kind for matching and use the descriptor's invoke for parsing
static constexpr auto plan = cli::build<Opts>();
for (const auto &opt : plan.options) {
switch (opt.kind) {
case option_kind::help:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::version:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::bool_flag:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
opt.invoke(&opts, "");
break;
}
}
for (std::size_t i = 0; i < std::size(args); ++i) {
auto arg = args[i];
bool is_option = arg.starts_with('-') && arg != "-";

if (is_option) {
const option_descriptor *opt = match_option(plan.options, arg);
if (opt == nullptr) {
std::println(stderr, "Unknown option {}", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
option_seen[static_cast<std::size_t>(opt - std::data(plan.options))] = true;

switch (opt->kind) {
case option_kind::help:
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
case option_kind::version:
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
case option_kind::bool_flag:
opt->invoke(&opts, "");
break;
}
case option_kind::counter:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name)) {
opt.invoke(&opts, "");
} else if (arg.starts_with('-') && !arg.starts_with("--")) {
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] == opt.short_name) {
opt.invoke(&opts, "");
}
}
case option_kind::counter:
{
std::size_t times = matches_long(arg, opt->long_name) ? 1 : (std::size(arg) - 1);
for (std::size_t t = 0; t < times; ++t) {
opt->invoke(&opts, "");
}
break;
}
break;
}
case option_kind::scalar:
{
bool matched{false};
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
matched = true;
break;
case option_kind::scalar:
case option_kind::vector:
{
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", arg);
std::exit(EXIT_FAILURE);
}
}
if (!matched && opt.is_required) {
std::println(stderr, "Missing required option --{}", opt.long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
break;
}
case option_kind::vector:
{
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
if (!opt->invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", arg, vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
break;
}
break;
}
}
}

// Plan-driven dispatch for positionals. Vector positionals drain all
// remaining slots; scalar positionals take the next slot in declaration
// order.
for (const auto &pos : plan.positionals) {
if (pos.is_vector) {
while (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
std::exit(EXIT_FAILURE);
}
++positional_idx;
}
} else if (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
} else {
// handle positional tokens
if (positional_idx >= std::size(plan.positionals)) {
std::println(stderr, "Unexpected argument \"{}\"", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
++positional_idx;
} else if (pos.is_required) {
std::println(stderr, "Missing required positional argument `{}`", pos.name);
const auto &pos = plan.positionals[positional_idx];
if (!pos.invoke(&opts, arg)) {
std::println(stderr, "Failed to parse positional `{}` from \"{}\"", pos.name, arg);
std::exit(EXIT_FAILURE);
}
positional_filled[positional_idx] = true;
if (!pos.is_vector) {
++positional_idx;
}
}
}

// required options/positionals
for (std::size_t o = 0; o < std::size(plan.options); ++o) {
if (plan.options[o].is_required && !option_seen[o]) {
std::println(stderr, "Missing required option --{}", plan.options[o].long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
}
for (std::size_t p = 0; p < std::size(plan.positionals); ++p) {
if (plan.positionals[p].is_required && !positional_filled[p]) {
std::println(stderr, "Missing required positional argument `{}`", plan.positionals[p].name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
Expand All@@ -216,7 +197,6 @@ auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_
// TODO: range / choices / validate_with enforcement.
// TODO: env-var fallback when not matched.
// TODO: on_match action invocation.
// TODO: report unknown options not matched by any member.
}

// handle subcommand
Expand Down
82 changes: 82 additions & 0 deletions tests/test_positionals.cpp
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
// Regression tests for argument consumption: a flag's value must never be
// mis-collected as a positional (B1), and positionals interleaved with flags
// must still land in the right members.

#include <cli/cli.hpp>
#include <gtest/gtest.h>

#include <array>
#include <span>
#include <string>
#include <string_view>
#include <vector>

namespace {

struct [[=cli::program("test")]] Opts {
[[=cli::desc("Source file"), =cli::positional, =cli::required]]
std::string source;

[[=cli::desc("Include dirs"), =cli::short_name('I'), =cli::long_name("include")]]
std::vector<std::string> includes;

[[=cli::desc("Port"), =cli::short_name('p')]]
int port = 0;

[[=cli::desc("Verbose"), =cli::short_name('v')]]
bool verbose = false;
};

template <std::size_t N>
auto parse(std::array<std::string_view, N> const &args) -> Opts {
return cli::parse<Opts>(std::string_view{"test"}, std::span{args.data(), args.size()});
}

} // namespace

// The core B1 bug: `--include foo.h target.cpp` must put target.cpp in the
// positional and foo.h in the vector option — not foo.h in both.
TEST(positionals, flag_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--include", "foo.h", "target.cpp"});
EXPECT_EQ(opts.source, "target.cpp");
ASSERT_EQ(opts.includes.size(), 1u);
EXPECT_EQ(opts.includes[0], "foo.h");
}

// Scalar flag value (a number) must not be grabbed as the positional.
TEST(positionals, scalar_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--port", "9090", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Positional before the flag.
TEST(positionals, positional_before_flag) {
auto opts = parse(std::array<std::string_view, 3>{"main.cpp", "--port", "9090"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Bool flag interleaved with positional — bool consumes no value.
TEST(positionals, bool_flag_does_not_eat_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--verbose", "main.cpp"});
EXPECT_TRUE(opts.verbose);
EXPECT_EQ(opts.source, "main.cpp");
}

// --name=value form leaves the following token as the positional.
TEST(positionals, equals_form_leaves_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--port=9090", "main.cpp"});
EXPECT_EQ(opts.port, 9090);
EXPECT_EQ(opts.source, "main.cpp");
}

// Multiple repeated vector flags plus a trailing positional.
TEST(positionals, repeated_vector_flag_then_positional) {
auto opts =
parse(std::array<std::string_view, 5>{"-I", "a", "--include", "b", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
ASSERT_EQ(opts.includes.size(), 2u);
EXPECT_EQ(opts.includes[0], "a");
EXPECT_EQ(opts.includes[1], "b");
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Better token classification. Moved required option/positional checks.… by aaronaranda · Pull Request #3 · aaronaranda/libcli · GitHub
Skip to content
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- Option values are no longer mis-collected as positional arguments.
`parse_into` now walks argv in a single left-to-right pass with a shared
cursor, so a flag's value (e.g. the `foo.h` in `--include foo.h target.cpp`)
can't also be assigned to a positional.
- Unknown options and excess positionals are now reported as errors instead
of being silently ignored.

### Changed
- Parser rearchitected around a compile-time **parse plan**. `cli::build<T>()`
produces a `program_descriptor` (flat `option_descriptor` / `positional_descriptor`
Expand Down
232 changes: 106 additions & 126 deletions src/cli/parse.hpp
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,11 +12,9 @@
#include <cstdlib>
#include <meta>
#include <print>
#include <ranges>
#include <span>
#include <string>
#include <string_view>
#include <utility>
#include <vector>

namespace cli::detail {
Expand DownExpand Up@@ -63,151 +61,134 @@ inline auto extract_value(std::span<const std::string_view> args, std::size_t po
return {.value = {}, .extra_consumed = 0, .found = false};
}

// Find the option descriptor matching an option token, or nullptr.
// Handles long (`--name`, `--name=v`), short (`-x`), and clustered counter
// short forms (`-vvv`). Clustered different shorts (`-abc`) are not yet
// supported (see P2).
inline auto match_option(std::span<const option_descriptor> options, std::string_view arg)
-> const option_descriptor * {
for (const auto &opt : options) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
return &opt;
}
}
// clustered counter short form
if (std::size(arg) > 2 && arg[0] == '-' && arg[1] != '-') {
for (const auto &opt : options) {
if (opt.kind != option_kind::counter || opt.short_name != arg[1]) {
continue;
}
bool all_same = true;
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] != opt.short_name) {
all_same = false;
break;
}
}
if (all_same) {
return &opt;
}
}
}
return nullptr;
}

// single struct parser
template <class Opts>
auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_view> args)
-> void {
static constexpr auto plan = cli::build<Opts>();

// Built-in --help / -h wins from any position, regardless of other errors.
for (auto arg : args) {
if (arg == "--help" || arg == "-h") {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}

// collect anything that doesn't start with a '-'
std::vector<std::string_view> positionals{};
positionals.reserve(std::size(args));
for (auto arg : args) {
if (!arg.starts_with('-') || arg == "-") {
positionals.push_back(arg);
}
}

// classify tokens from left to right as an option, positional, or unknown.
std::vector<bool> option_seen(std::size(plan.options), false);
std::vector<bool> positional_filled(std::size(plan.positionals), false);
std::size_t positional_idx{0};

// switch over option_kind for matching and use the descriptor's invoke for parsing
static constexpr auto plan = cli::build<Opts>();
for (const auto &opt : plan.options) {
switch (opt.kind) {
case option_kind::help:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::version:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::bool_flag:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
opt.invoke(&opts, "");
break;
}
}
for (std::size_t i = 0; i < std::size(args); ++i) {
auto arg = args[i];
bool is_option = arg.starts_with('-') && arg != "-";

if (is_option) {
const option_descriptor *opt = match_option(plan.options, arg);
if (opt == nullptr) {
std::println(stderr, "Unknown option {}", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
option_seen[static_cast<std::size_t>(opt - std::data(plan.options))] = true;

switch (opt->kind) {
case option_kind::help:
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
case option_kind::version:
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
case option_kind::bool_flag:
opt->invoke(&opts, "");
break;
}
case option_kind::counter:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name)) {
opt.invoke(&opts, "");
} else if (arg.starts_with('-') && !arg.starts_with("--")) {
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] == opt.short_name) {
opt.invoke(&opts, "");
}
}
case option_kind::counter:
{
std::size_t times = matches_long(arg, opt->long_name) ? 1 : (std::size(arg) - 1);
for (std::size_t t = 0; t < times; ++t) {
opt->invoke(&opts, "");
}
break;
}
break;
}
case option_kind::scalar:
{
bool matched{false};
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
matched = true;
break;
case option_kind::scalar:
case option_kind::vector:
{
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", arg);
std::exit(EXIT_FAILURE);
}
}
if (!matched && opt.is_required) {
std::println(stderr, "Missing required option --{}", opt.long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
break;
}
case option_kind::vector:
{
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
if (!opt->invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", arg, vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
break;
}
break;
}
}
}

// Plan-driven dispatch for positionals. Vector positionals drain all
// remaining slots; scalar positionals take the next slot in declaration
// order.
for (const auto &pos : plan.positionals) {
if (pos.is_vector) {
while (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
std::exit(EXIT_FAILURE);
}
++positional_idx;
}
} else if (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
} else {
// handle positional tokens
if (positional_idx >= std::size(plan.positionals)) {
std::println(stderr, "Unexpected argument \"{}\"", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
++positional_idx;
} else if (pos.is_required) {
std::println(stderr, "Missing required positional argument `{}`", pos.name);
const auto &pos = plan.positionals[positional_idx];
if (!pos.invoke(&opts, arg)) {
std::println(stderr, "Failed to parse positional `{}` from \"{}\"", pos.name, arg);
std::exit(EXIT_FAILURE);
}
positional_filled[positional_idx] = true;
if (!pos.is_vector) {
++positional_idx;
}
}
}

// required options/positionals
for (std::size_t o = 0; o < std::size(plan.options); ++o) {
if (plan.options[o].is_required && !option_seen[o]) {
std::println(stderr, "Missing required option --{}", plan.options[o].long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
}
for (std::size_t p = 0; p < std::size(plan.positionals); ++p) {
if (plan.positionals[p].is_required && !positional_filled[p]) {
std::println(stderr, "Missing required positional argument `{}`", plan.positionals[p].name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
Expand All@@ -216,7 +197,6 @@ auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_
// TODO: range / choices / validate_with enforcement.
// TODO: env-var fallback when not matched.
// TODO: on_match action invocation.
// TODO: report unknown options not matched by any member.
}

// handle subcommand
Expand Down
82 changes: 82 additions & 0 deletions tests/test_positionals.cpp
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
// Regression tests for argument consumption: a flag's value must never be
// mis-collected as a positional (B1), and positionals interleaved with flags
// must still land in the right members.

#include <cli/cli.hpp>
#include <gtest/gtest.h>

#include <array>
#include <span>
#include <string>
#include <string_view>
#include <vector>

namespace {

struct [[=cli::program("test")]] Opts {
[[=cli::desc("Source file"), =cli::positional, =cli::required]]
std::string source;

[[=cli::desc("Include dirs"), =cli::short_name('I'), =cli::long_name("include")]]
std::vector<std::string> includes;

[[=cli::desc("Port"), =cli::short_name('p')]]
int port = 0;

[[=cli::desc("Verbose"), =cli::short_name('v')]]
bool verbose = false;
};

template <std::size_t N>
auto parse(std::array<std::string_view, N> const &args) -> Opts {
return cli::parse<Opts>(std::string_view{"test"}, std::span{args.data(), args.size()});
}

} // namespace

// The core B1 bug: `--include foo.h target.cpp` must put target.cpp in the
// positional and foo.h in the vector option — not foo.h in both.
TEST(positionals, flag_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--include", "foo.h", "target.cpp"});
EXPECT_EQ(opts.source, "target.cpp");
ASSERT_EQ(opts.includes.size(), 1u);
EXPECT_EQ(opts.includes[0], "foo.h");
}

// Scalar flag value (a number) must not be grabbed as the positional.
TEST(positionals, scalar_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--port", "9090", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Positional before the flag.
TEST(positionals, positional_before_flag) {
auto opts = parse(std::array<std::string_view, 3>{"main.cpp", "--port", "9090"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Bool flag interleaved with positional — bool consumes no value.
TEST(positionals, bool_flag_does_not_eat_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--verbose", "main.cpp"});
EXPECT_TRUE(opts.verbose);
EXPECT_EQ(opts.source, "main.cpp");
}

// --name=value form leaves the following token as the positional.
TEST(positionals, equals_form_leaves_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--port=9090", "main.cpp"});
EXPECT_EQ(opts.port, 9090);
EXPECT_EQ(opts.source, "main.cpp");
}

// Multiple repeated vector flags plus a trailing positional.
TEST(positionals, repeated_vector_flag_then_positional) {
auto opts =
parse(std::array<std::string_view, 5>{"-I", "a", "--include", "b", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
ASSERT_EQ(opts.includes.size(), 2u);
EXPECT_EQ(opts.includes[0], "a");
EXPECT_EQ(opts.includes[1], "b");
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' Better token classification. Moved required option/positional checks.… by aaronaranda · Pull Request #3 · aaronaranda/libcli · GitHub
Skip to content
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- Option values are no longer mis-collected as positional arguments.
`parse_into` now walks argv in a single left-to-right pass with a shared
cursor, so a flag's value (e.g. the `foo.h` in `--include foo.h target.cpp`)
can't also be assigned to a positional.
- Unknown options and excess positionals are now reported as errors instead
of being silently ignored.

### Changed
- Parser rearchitected around a compile-time **parse plan**. `cli::build<T>()`
produces a `program_descriptor` (flat `option_descriptor` / `positional_descriptor`
Expand Down
232 changes: 106 additions & 126 deletions src/cli/parse.hpp
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,11 +12,9 @@
#include <cstdlib>
#include <meta>
#include <print>
#include <ranges>
#include <span>
#include <string>
#include <string_view>
#include <utility>
#include <vector>

namespace cli::detail {
Expand DownExpand Up@@ -63,151 +61,134 @@ inline auto extract_value(std::span<const std::string_view> args, std::size_t po
return {.value = {}, .extra_consumed = 0, .found = false};
}

// Find the option descriptor matching an option token, or nullptr.
// Handles long (`--name`, `--name=v`), short (`-x`), and clustered counter
// short forms (`-vvv`). Clustered different shorts (`-abc`) are not yet
// supported (see P2).
inline auto match_option(std::span<const option_descriptor> options, std::string_view arg)
-> const option_descriptor * {
for (const auto &opt : options) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
return &opt;
}
}
// clustered counter short form
if (std::size(arg) > 2 && arg[0] == '-' && arg[1] != '-') {
for (const auto &opt : options) {
if (opt.kind != option_kind::counter || opt.short_name != arg[1]) {
continue;
}
bool all_same = true;
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] != opt.short_name) {
all_same = false;
break;
}
}
if (all_same) {
return &opt;
}
}
}
return nullptr;
}

// single struct parser
template <class Opts>
auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_view> args)
-> void {
static constexpr auto plan = cli::build<Opts>();

// Built-in --help / -h wins from any position, regardless of other errors.
for (auto arg : args) {
if (arg == "--help" || arg == "-h") {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}

// collect anything that doesn't start with a '-'
std::vector<std::string_view> positionals{};
positionals.reserve(std::size(args));
for (auto arg : args) {
if (!arg.starts_with('-') || arg == "-") {
positionals.push_back(arg);
}
}

// classify tokens from left to right as an option, positional, or unknown.
std::vector<bool> option_seen(std::size(plan.options), false);
std::vector<bool> positional_filled(std::size(plan.positionals), false);
std::size_t positional_idx{0};

// switch over option_kind for matching and use the descriptor's invoke for parsing
static constexpr auto plan = cli::build<Opts>();
for (const auto &opt : plan.options) {
switch (opt.kind) {
case option_kind::help:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::version:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::bool_flag:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
opt.invoke(&opts, "");
break;
}
}
for (std::size_t i = 0; i < std::size(args); ++i) {
auto arg = args[i];
bool is_option = arg.starts_with('-') && arg != "-";

if (is_option) {
const option_descriptor *opt = match_option(plan.options, arg);
if (opt == nullptr) {
std::println(stderr, "Unknown option {}", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
option_seen[static_cast<std::size_t>(opt - std::data(plan.options))] = true;

switch (opt->kind) {
case option_kind::help:
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
case option_kind::version:
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
case option_kind::bool_flag:
opt->invoke(&opts, "");
break;
}
case option_kind::counter:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name)) {
opt.invoke(&opts, "");
} else if (arg.starts_with('-') && !arg.starts_with("--")) {
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] == opt.short_name) {
opt.invoke(&opts, "");
}
}
case option_kind::counter:
{
std::size_t times = matches_long(arg, opt->long_name) ? 1 : (std::size(arg) - 1);
for (std::size_t t = 0; t < times; ++t) {
opt->invoke(&opts, "");
}
break;
}
break;
}
case option_kind::scalar:
{
bool matched{false};
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
matched = true;
break;
case option_kind::scalar:
case option_kind::vector:
{
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", arg);
std::exit(EXIT_FAILURE);
}
}
if (!matched && opt.is_required) {
std::println(stderr, "Missing required option --{}", opt.long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
break;
}
case option_kind::vector:
{
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
if (!opt->invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", arg, vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
break;
}
break;
}
}
}

// Plan-driven dispatch for positionals. Vector positionals drain all
// remaining slots; scalar positionals take the next slot in declaration
// order.
for (const auto &pos : plan.positionals) {
if (pos.is_vector) {
while (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
std::exit(EXIT_FAILURE);
}
++positional_idx;
}
} else if (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
} else {
// handle positional tokens
if (positional_idx >= std::size(plan.positionals)) {
std::println(stderr, "Unexpected argument \"{}\"", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
++positional_idx;
} else if (pos.is_required) {
std::println(stderr, "Missing required positional argument `{}`", pos.name);
const auto &pos = plan.positionals[positional_idx];
if (!pos.invoke(&opts, arg)) {
std::println(stderr, "Failed to parse positional `{}` from \"{}\"", pos.name, arg);
std::exit(EXIT_FAILURE);
}
positional_filled[positional_idx] = true;
if (!pos.is_vector) {
++positional_idx;
}
}
}

// required options/positionals
for (std::size_t o = 0; o < std::size(plan.options); ++o) {
if (plan.options[o].is_required && !option_seen[o]) {
std::println(stderr, "Missing required option --{}", plan.options[o].long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
}
for (std::size_t p = 0; p < std::size(plan.positionals); ++p) {
if (plan.positionals[p].is_required && !positional_filled[p]) {
std::println(stderr, "Missing required positional argument `{}`", plan.positionals[p].name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
Expand All@@ -216,7 +197,6 @@ auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_
// TODO: range / choices / validate_with enforcement.
// TODO: env-var fallback when not matched.
// TODO: on_match action invocation.
// TODO: report unknown options not matched by any member.
}

// handle subcommand
Expand Down
82 changes: 82 additions & 0 deletions tests/test_positionals.cpp
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
// Regression tests for argument consumption: a flag's value must never be
// mis-collected as a positional (B1), and positionals interleaved with flags
// must still land in the right members.

#include <cli/cli.hpp>
#include <gtest/gtest.h>

#include <array>
#include <span>
#include <string>
#include <string_view>
#include <vector>

namespace {

struct [[=cli::program("test")]] Opts {
[[=cli::desc("Source file"), =cli::positional, =cli::required]]
std::string source;

[[=cli::desc("Include dirs"), =cli::short_name('I'), =cli::long_name("include")]]
std::vector<std::string> includes;

[[=cli::desc("Port"), =cli::short_name('p')]]
int port = 0;

[[=cli::desc("Verbose"), =cli::short_name('v')]]
bool verbose = false;
};

template <std::size_t N>
auto parse(std::array<std::string_view, N> const &args) -> Opts {
return cli::parse<Opts>(std::string_view{"test"}, std::span{args.data(), args.size()});
}

} // namespace

// The core B1 bug: `--include foo.h target.cpp` must put target.cpp in the
// positional and foo.h in the vector option — not foo.h in both.
TEST(positionals, flag_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--include", "foo.h", "target.cpp"});
EXPECT_EQ(opts.source, "target.cpp");
ASSERT_EQ(opts.includes.size(), 1u);
EXPECT_EQ(opts.includes[0], "foo.h");
}

// Scalar flag value (a number) must not be grabbed as the positional.
TEST(positionals, scalar_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--port", "9090", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Positional before the flag.
TEST(positionals, positional_before_flag) {
auto opts = parse(std::array<std::string_view, 3>{"main.cpp", "--port", "9090"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Bool flag interleaved with positional — bool consumes no value.
TEST(positionals, bool_flag_does_not_eat_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--verbose", "main.cpp"});
EXPECT_TRUE(opts.verbose);
EXPECT_EQ(opts.source, "main.cpp");
}

// --name=value form leaves the following token as the positional.
TEST(positionals, equals_form_leaves_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--port=9090", "main.cpp"});
EXPECT_EQ(opts.port, 9090);
EXPECT_EQ(opts.source, "main.cpp");
}

// Multiple repeated vector flags plus a trailing positional.
TEST(positionals, repeated_vector_flag_then_positional) {
auto opts =
parse(std::array<std::string_view, 5>{"-I", "a", "--include", "b", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
ASSERT_EQ(opts.includes.size(), 2u);
EXPECT_EQ(opts.includes[0], "a");
EXPECT_EQ(opts.includes[1], "b");
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Better token classification. Moved required option/positional checks.… by aaronaranda · Pull Request #3 · aaronaranda/libcli · GitHub
Skip to content
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- Option values are no longer mis-collected as positional arguments.
`parse_into` now walks argv in a single left-to-right pass with a shared
cursor, so a flag's value (e.g. the `foo.h` in `--include foo.h target.cpp`)
can't also be assigned to a positional.
- Unknown options and excess positionals are now reported as errors instead
of being silently ignored.

### Changed
- Parser rearchitected around a compile-time **parse plan**. `cli::build<T>()`
produces a `program_descriptor` (flat `option_descriptor` / `positional_descriptor`
Expand Down
232 changes: 106 additions & 126 deletions src/cli/parse.hpp
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,11 +12,9 @@
#include <cstdlib>
#include <meta>
#include <print>
#include <ranges>
#include <span>
#include <string>
#include <string_view>
#include <utility>
#include <vector>

namespace cli::detail {
Expand DownExpand Up@@ -63,151 +61,134 @@ inline auto extract_value(std::span<const std::string_view> args, std::size_t po
return {.value = {}, .extra_consumed = 0, .found = false};
}

// Find the option descriptor matching an option token, or nullptr.
// Handles long (`--name`, `--name=v`), short (`-x`), and clustered counter
// short forms (`-vvv`). Clustered different shorts (`-abc`) are not yet
// supported (see P2).
inline auto match_option(std::span<const option_descriptor> options, std::string_view arg)
-> const option_descriptor * {
for (const auto &opt : options) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
return &opt;
}
}
// clustered counter short form
if (std::size(arg) > 2 && arg[0] == '-' && arg[1] != '-') {
for (const auto &opt : options) {
if (opt.kind != option_kind::counter || opt.short_name != arg[1]) {
continue;
}
bool all_same = true;
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] != opt.short_name) {
all_same = false;
break;
}
}
if (all_same) {
return &opt;
}
}
}
return nullptr;
}

// single struct parser
template <class Opts>
auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_view> args)
-> void {
static constexpr auto plan = cli::build<Opts>();

// Built-in --help / -h wins from any position, regardless of other errors.
for (auto arg : args) {
if (arg == "--help" || arg == "-h") {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}

// collect anything that doesn't start with a '-'
std::vector<std::string_view> positionals{};
positionals.reserve(std::size(args));
for (auto arg : args) {
if (!arg.starts_with('-') || arg == "-") {
positionals.push_back(arg);
}
}

// classify tokens from left to right as an option, positional, or unknown.
std::vector<bool> option_seen(std::size(plan.options), false);
std::vector<bool> positional_filled(std::size(plan.positionals), false);
std::size_t positional_idx{0};

// switch over option_kind for matching and use the descriptor's invoke for parsing
static constexpr auto plan = cli::build<Opts>();
for (const auto &opt : plan.options) {
switch (opt.kind) {
case option_kind::help:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::version:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::bool_flag:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
opt.invoke(&opts, "");
break;
}
}
for (std::size_t i = 0; i < std::size(args); ++i) {
auto arg = args[i];
bool is_option = arg.starts_with('-') && arg != "-";

if (is_option) {
const option_descriptor *opt = match_option(plan.options, arg);
if (opt == nullptr) {
std::println(stderr, "Unknown option {}", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
option_seen[static_cast<std::size_t>(opt - std::data(plan.options))] = true;

switch (opt->kind) {
case option_kind::help:
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
case option_kind::version:
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
case option_kind::bool_flag:
opt->invoke(&opts, "");
break;
}
case option_kind::counter:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name)) {
opt.invoke(&opts, "");
} else if (arg.starts_with('-') && !arg.starts_with("--")) {
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] == opt.short_name) {
opt.invoke(&opts, "");
}
}
case option_kind::counter:
{
std::size_t times = matches_long(arg, opt->long_name) ? 1 : (std::size(arg) - 1);
for (std::size_t t = 0; t < times; ++t) {
opt->invoke(&opts, "");
}
break;
}
break;
}
case option_kind::scalar:
{
bool matched{false};
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
matched = true;
break;
case option_kind::scalar:
case option_kind::vector:
{
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", arg);
std::exit(EXIT_FAILURE);
}
}
if (!matched && opt.is_required) {
std::println(stderr, "Missing required option --{}", opt.long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
break;
}
case option_kind::vector:
{
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
if (!opt->invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", arg, vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
break;
}
break;
}
}
}

// Plan-driven dispatch for positionals. Vector positionals drain all
// remaining slots; scalar positionals take the next slot in declaration
// order.
for (const auto &pos : plan.positionals) {
if (pos.is_vector) {
while (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
std::exit(EXIT_FAILURE);
}
++positional_idx;
}
} else if (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
} else {
// handle positional tokens
if (positional_idx >= std::size(plan.positionals)) {
std::println(stderr, "Unexpected argument \"{}\"", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
++positional_idx;
} else if (pos.is_required) {
std::println(stderr, "Missing required positional argument `{}`", pos.name);
const auto &pos = plan.positionals[positional_idx];
if (!pos.invoke(&opts, arg)) {
std::println(stderr, "Failed to parse positional `{}` from \"{}\"", pos.name, arg);
std::exit(EXIT_FAILURE);
}
positional_filled[positional_idx] = true;
if (!pos.is_vector) {
++positional_idx;
}
}
}

// required options/positionals
for (std::size_t o = 0; o < std::size(plan.options); ++o) {
if (plan.options[o].is_required && !option_seen[o]) {
std::println(stderr, "Missing required option --{}", plan.options[o].long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
}
for (std::size_t p = 0; p < std::size(plan.positionals); ++p) {
if (plan.positionals[p].is_required && !positional_filled[p]) {
std::println(stderr, "Missing required positional argument `{}`", plan.positionals[p].name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
Expand All@@ -216,7 +197,6 @@ auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_
// TODO: range / choices / validate_with enforcement.
// TODO: env-var fallback when not matched.
// TODO: on_match action invocation.
// TODO: report unknown options not matched by any member.
}

// handle subcommand
Expand Down
82 changes: 82 additions & 0 deletions tests/test_positionals.cpp
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
// Regression tests for argument consumption: a flag's value must never be
// mis-collected as a positional (B1), and positionals interleaved with flags
// must still land in the right members.

#include <cli/cli.hpp>
#include <gtest/gtest.h>

#include <array>
#include <span>
#include <string>
#include <string_view>
#include <vector>

namespace {

struct [[=cli::program("test")]] Opts {
[[=cli::desc("Source file"), =cli::positional, =cli::required]]
std::string source;

[[=cli::desc("Include dirs"), =cli::short_name('I'), =cli::long_name("include")]]
std::vector<std::string> includes;

[[=cli::desc("Port"), =cli::short_name('p')]]
int port = 0;

[[=cli::desc("Verbose"), =cli::short_name('v')]]
bool verbose = false;
};

template <std::size_t N>
auto parse(std::array<std::string_view, N> const &args) -> Opts {
return cli::parse<Opts>(std::string_view{"test"}, std::span{args.data(), args.size()});
}

} // namespace

// The core B1 bug: `--include foo.h target.cpp` must put target.cpp in the
// positional and foo.h in the vector option — not foo.h in both.
TEST(positionals, flag_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--include", "foo.h", "target.cpp"});
EXPECT_EQ(opts.source, "target.cpp");
ASSERT_EQ(opts.includes.size(), 1u);
EXPECT_EQ(opts.includes[0], "foo.h");
}

// Scalar flag value (a number) must not be grabbed as the positional.
TEST(positionals, scalar_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--port", "9090", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Positional before the flag.
TEST(positionals, positional_before_flag) {
auto opts = parse(std::array<std::string_view, 3>{"main.cpp", "--port", "9090"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Bool flag interleaved with positional — bool consumes no value.
TEST(positionals, bool_flag_does_not_eat_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--verbose", "main.cpp"});
EXPECT_TRUE(opts.verbose);
EXPECT_EQ(opts.source, "main.cpp");
}

// --name=value form leaves the following token as the positional.
TEST(positionals, equals_form_leaves_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--port=9090", "main.cpp"});
EXPECT_EQ(opts.port, 9090);
EXPECT_EQ(opts.source, "main.cpp");
}

// Multiple repeated vector flags plus a trailing positional.
TEST(positionals, repeated_vector_flag_then_positional) {
auto opts =
parse(std::array<std::string_view, 5>{"-I", "a", "--include", "b", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
ASSERT_EQ(opts.includes.size(), 2u);
EXPECT_EQ(opts.includes[0], "a");
EXPECT_EQ(opts.includes[1], "b");
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' Better token classification. Moved required option/positional checks.… by aaronaranda · Pull Request #3 · aaronaranda/libcli · GitHub
Skip to content
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- Option values are no longer mis-collected as positional arguments.
`parse_into` now walks argv in a single left-to-right pass with a shared
cursor, so a flag's value (e.g. the `foo.h` in `--include foo.h target.cpp`)
can't also be assigned to a positional.
- Unknown options and excess positionals are now reported as errors instead
of being silently ignored.

### Changed
- Parser rearchitected around a compile-time **parse plan**. `cli::build<T>()`
produces a `program_descriptor` (flat `option_descriptor` / `positional_descriptor`
Expand Down
232 changes: 106 additions & 126 deletions src/cli/parse.hpp
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,11 +12,9 @@
#include <cstdlib>
#include <meta>
#include <print>
#include <ranges>
#include <span>
#include <string>
#include <string_view>
#include <utility>
#include <vector>

namespace cli::detail {
Expand DownExpand Up@@ -63,151 +61,134 @@ inline auto extract_value(std::span<const std::string_view> args, std::size_t po
return {.value = {}, .extra_consumed = 0, .found = false};
}

// Find the option descriptor matching an option token, or nullptr.
// Handles long (`--name`, `--name=v`), short (`-x`), and clustered counter
// short forms (`-vvv`). Clustered different shorts (`-abc`) are not yet
// supported (see P2).
inline auto match_option(std::span<const option_descriptor> options, std::string_view arg)
-> const option_descriptor * {
for (const auto &opt : options) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
return &opt;
}
}
// clustered counter short form
if (std::size(arg) > 2 && arg[0] == '-' && arg[1] != '-') {
for (const auto &opt : options) {
if (opt.kind != option_kind::counter || opt.short_name != arg[1]) {
continue;
}
bool all_same = true;
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] != opt.short_name) {
all_same = false;
break;
}
}
if (all_same) {
return &opt;
}
}
}
return nullptr;
}

// single struct parser
template <class Opts>
auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_view> args)
-> void {
static constexpr auto plan = cli::build<Opts>();

// Built-in --help / -h wins from any position, regardless of other errors.
for (auto arg : args) {
if (arg == "--help" || arg == "-h") {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}

// collect anything that doesn't start with a '-'
std::vector<std::string_view> positionals{};
positionals.reserve(std::size(args));
for (auto arg : args) {
if (!arg.starts_with('-') || arg == "-") {
positionals.push_back(arg);
}
}

// classify tokens from left to right as an option, positional, or unknown.
std::vector<bool> option_seen(std::size(plan.options), false);
std::vector<bool> positional_filled(std::size(plan.positionals), false);
std::size_t positional_idx{0};

// switch over option_kind for matching and use the descriptor's invoke for parsing
static constexpr auto plan = cli::build<Opts>();
for (const auto &opt : plan.options) {
switch (opt.kind) {
case option_kind::help:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::version:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::bool_flag:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
opt.invoke(&opts, "");
break;
}
}
for (std::size_t i = 0; i < std::size(args); ++i) {
auto arg = args[i];
bool is_option = arg.starts_with('-') && arg != "-";

if (is_option) {
const option_descriptor *opt = match_option(plan.options, arg);
if (opt == nullptr) {
std::println(stderr, "Unknown option {}", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
option_seen[static_cast<std::size_t>(opt - std::data(plan.options))] = true;

switch (opt->kind) {
case option_kind::help:
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
case option_kind::version:
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
case option_kind::bool_flag:
opt->invoke(&opts, "");
break;
}
case option_kind::counter:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name)) {
opt.invoke(&opts, "");
} else if (arg.starts_with('-') && !arg.starts_with("--")) {
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] == opt.short_name) {
opt.invoke(&opts, "");
}
}
case option_kind::counter:
{
std::size_t times = matches_long(arg, opt->long_name) ? 1 : (std::size(arg) - 1);
for (std::size_t t = 0; t < times; ++t) {
opt->invoke(&opts, "");
}
break;
}
break;
}
case option_kind::scalar:
{
bool matched{false};
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
matched = true;
break;
case option_kind::scalar:
case option_kind::vector:
{
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", arg);
std::exit(EXIT_FAILURE);
}
}
if (!matched && opt.is_required) {
std::println(stderr, "Missing required option --{}", opt.long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
break;
}
case option_kind::vector:
{
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
if (!opt->invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", arg, vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
break;
}
break;
}
}
}

// Plan-driven dispatch for positionals. Vector positionals drain all
// remaining slots; scalar positionals take the next slot in declaration
// order.
for (const auto &pos : plan.positionals) {
if (pos.is_vector) {
while (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
std::exit(EXIT_FAILURE);
}
++positional_idx;
}
} else if (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
} else {
// handle positional tokens
if (positional_idx >= std::size(plan.positionals)) {
std::println(stderr, "Unexpected argument \"{}\"", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
++positional_idx;
} else if (pos.is_required) {
std::println(stderr, "Missing required positional argument `{}`", pos.name);
const auto &pos = plan.positionals[positional_idx];
if (!pos.invoke(&opts, arg)) {
std::println(stderr, "Failed to parse positional `{}` from \"{}\"", pos.name, arg);
std::exit(EXIT_FAILURE);
}
positional_filled[positional_idx] = true;
if (!pos.is_vector) {
++positional_idx;
}
}
}

// required options/positionals
for (std::size_t o = 0; o < std::size(plan.options); ++o) {
if (plan.options[o].is_required && !option_seen[o]) {
std::println(stderr, "Missing required option --{}", plan.options[o].long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
}
for (std::size_t p = 0; p < std::size(plan.positionals); ++p) {
if (plan.positionals[p].is_required && !positional_filled[p]) {
std::println(stderr, "Missing required positional argument `{}`", plan.positionals[p].name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
Expand All@@ -216,7 +197,6 @@ auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_
// TODO: range / choices / validate_with enforcement.
// TODO: env-var fallback when not matched.
// TODO: on_match action invocation.
// TODO: report unknown options not matched by any member.
}

// handle subcommand
Expand Down
82 changes: 82 additions & 0 deletions tests/test_positionals.cpp
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
// Regression tests for argument consumption: a flag's value must never be
// mis-collected as a positional (B1), and positionals interleaved with flags
// must still land in the right members.

#include <cli/cli.hpp>
#include <gtest/gtest.h>

#include <array>
#include <span>
#include <string>
#include <string_view>
#include <vector>

namespace {

struct [[=cli::program("test")]] Opts {
[[=cli::desc("Source file"), =cli::positional, =cli::required]]
std::string source;

[[=cli::desc("Include dirs"), =cli::short_name('I'), =cli::long_name("include")]]
std::vector<std::string> includes;

[[=cli::desc("Port"), =cli::short_name('p')]]
int port = 0;

[[=cli::desc("Verbose"), =cli::short_name('v')]]
bool verbose = false;
};

template <std::size_t N>
auto parse(std::array<std::string_view, N> const &args) -> Opts {
return cli::parse<Opts>(std::string_view{"test"}, std::span{args.data(), args.size()});
}

} // namespace

// The core B1 bug: `--include foo.h target.cpp` must put target.cpp in the
// positional and foo.h in the vector option — not foo.h in both.
TEST(positionals, flag_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--include", "foo.h", "target.cpp"});
EXPECT_EQ(opts.source, "target.cpp");
ASSERT_EQ(opts.includes.size(), 1u);
EXPECT_EQ(opts.includes[0], "foo.h");
}

// Scalar flag value (a number) must not be grabbed as the positional.
TEST(positionals, scalar_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--port", "9090", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Positional before the flag.
TEST(positionals, positional_before_flag) {
auto opts = parse(std::array<std::string_view, 3>{"main.cpp", "--port", "9090"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Bool flag interleaved with positional — bool consumes no value.
TEST(positionals, bool_flag_does_not_eat_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--verbose", "main.cpp"});
EXPECT_TRUE(opts.verbose);
EXPECT_EQ(opts.source, "main.cpp");
}

// --name=value form leaves the following token as the positional.
TEST(positionals, equals_form_leaves_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--port=9090", "main.cpp"});
EXPECT_EQ(opts.port, 9090);
EXPECT_EQ(opts.source, "main.cpp");
}

// Multiple repeated vector flags plus a trailing positional.
TEST(positionals, repeated_vector_flag_then_positional) {
auto opts =
parse(std::array<std::string_view, 5>{"-I", "a", "--include", "b", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
ASSERT_EQ(opts.includes.size(), 2u);
EXPECT_EQ(opts.includes[0], "a");
EXPECT_EQ(opts.includes[1], "b");
}
Loading
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); Better token classification. Moved required option/positional checks.… by aaronaranda · Pull Request #3 · aaronaranda/libcli · GitHub
Skip to content
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
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Fixed
- Option values are no longer mis-collected as positional arguments.
`parse_into` now walks argv in a single left-to-right pass with a shared
cursor, so a flag's value (e.g. the `foo.h` in `--include foo.h target.cpp`)
can't also be assigned to a positional.
- Unknown options and excess positionals are now reported as errors instead
of being silently ignored.

### Changed
- Parser rearchitected around a compile-time **parse plan**. `cli::build<T>()`
produces a `program_descriptor` (flat `option_descriptor` / `positional_descriptor`
Expand Down
232 changes: 106 additions & 126 deletions src/cli/parse.hpp
Original file line numberDiff line numberDiff line change
Expand Up@@ -12,11 +12,9 @@
#include <cstdlib>
#include <meta>
#include <print>
#include <ranges>
#include <span>
#include <string>
#include <string_view>
#include <utility>
#include <vector>

namespace cli::detail {
Expand DownExpand Up@@ -63,151 +61,134 @@ inline auto extract_value(std::span<const std::string_view> args, std::size_t po
return {.value = {}, .extra_consumed = 0, .found = false};
}

// Find the option descriptor matching an option token, or nullptr.
// Handles long (`--name`, `--name=v`), short (`-x`), and clustered counter
// short forms (`-vvv`). Clustered different shorts (`-abc`) are not yet
// supported (see P2).
inline auto match_option(std::span<const option_descriptor> options, std::string_view arg)
-> const option_descriptor * {
for (const auto &opt : options) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
return &opt;
}
}
// clustered counter short form
if (std::size(arg) > 2 && arg[0] == '-' && arg[1] != '-') {
for (const auto &opt : options) {
if (opt.kind != option_kind::counter || opt.short_name != arg[1]) {
continue;
}
bool all_same = true;
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] != opt.short_name) {
all_same = false;
break;
}
}
if (all_same) {
return &opt;
}
}
}
return nullptr;
}

// single struct parser
template <class Opts>
auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_view> args)
-> void {
static constexpr auto plan = cli::build<Opts>();

// Built-in --help / -h wins from any position, regardless of other errors.
for (auto arg : args) {
if (arg == "--help" || arg == "-h") {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}

// collect anything that doesn't start with a '-'
std::vector<std::string_view> positionals{};
positionals.reserve(std::size(args));
for (auto arg : args) {
if (!arg.starts_with('-') || arg == "-") {
positionals.push_back(arg);
}
}

// classify tokens from left to right as an option, positional, or unknown.
std::vector<bool> option_seen(std::size(plan.options), false);
std::vector<bool> positional_filled(std::size(plan.positionals), false);
std::size_t positional_idx{0};

// switch over option_kind for matching and use the descriptor's invoke for parsing
static constexpr auto plan = cli::build<Opts>();
for (const auto &opt : plan.options) {
switch (opt.kind) {
case option_kind::help:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::version:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
}
}
break;
}
case option_kind::bool_flag:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name) || matches_short(arg, opt.short_name)) {
opt.invoke(&opts, "");
break;
}
}
for (std::size_t i = 0; i < std::size(args); ++i) {
auto arg = args[i];
bool is_option = arg.starts_with('-') && arg != "-";

if (is_option) {
const option_descriptor *opt = match_option(plan.options, arg);
if (opt == nullptr) {
std::println(stderr, "Unknown option {}", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
option_seen[static_cast<std::size_t>(opt - std::data(plan.options))] = true;

switch (opt->kind) {
case option_kind::help:
cli::print_help<Opts>(argv0);
std::exit(EXIT_SUCCESS);
case option_kind::version:
std::println("{}", plan.version != nullptr ? plan.version : "(no version set)");
std::exit(EXIT_SUCCESS);
case option_kind::bool_flag:
opt->invoke(&opts, "");
break;
}
case option_kind::counter:
{
for (auto arg : args) {
if (matches_long(arg, opt.long_name)) {
opt.invoke(&opts, "");
} else if (arg.starts_with('-') && !arg.starts_with("--")) {
for (std::size_t k = 1; k < std::size(arg); ++k) {
if (arg[k] == opt.short_name) {
opt.invoke(&opts, "");
}
}
case option_kind::counter:
{
std::size_t times = matches_long(arg, opt->long_name) ? 1 : (std::size(arg) - 1);
for (std::size_t t = 0; t < times; ++t) {
opt->invoke(&opts, "");
}
break;
}
break;
}
case option_kind::scalar:
{
bool matched{false};
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
matched = true;
break;
case option_kind::scalar:
case option_kind::vector:
{
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", arg);
std::exit(EXIT_FAILURE);
}
}
if (!matched && opt.is_required) {
std::println(stderr, "Missing required option --{}", opt.long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
break;
}
case option_kind::vector:
{
for (std::size_t i = 0; i < std::size(args); ++i) {
if (matches_long(args[i], opt.long_name) || matches_short(args[i], opt.short_name)) {
auto vl = extract_value(args, i);
if (!vl.found) {
std::println(stderr, "Option {} is missing a value", args[i]);
std::exit(EXIT_FAILURE);
}
if (!opt.invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", args[i], vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
if (!opt->invoke(&opts, vl.value)) {
std::println(stderr, "Failed to parse `{}` value \"{}\"", arg, vl.value);
std::exit(EXIT_FAILURE);
}
i += vl.extra_consumed;
break;
}
break;
}
}
}

// Plan-driven dispatch for positionals. Vector positionals drain all
// remaining slots; scalar positionals take the next slot in declaration
// order.
for (const auto &pos : plan.positionals) {
if (pos.is_vector) {
while (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
std::exit(EXIT_FAILURE);
}
++positional_idx;
}
} else if (positional_idx < std::size(positionals)) {
if (!pos.invoke(&opts, positionals[positional_idx])) {
std::println(
stderr,
"Failed to parse positional `{}` from \"{}\"",
pos.name,
positionals[positional_idx]);
} else {
// handle positional tokens
if (positional_idx >= std::size(plan.positionals)) {
std::println(stderr, "Unexpected argument \"{}\"", arg);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
++positional_idx;
} else if (pos.is_required) {
std::println(stderr, "Missing required positional argument `{}`", pos.name);
const auto &pos = plan.positionals[positional_idx];
if (!pos.invoke(&opts, arg)) {
std::println(stderr, "Failed to parse positional `{}` from \"{}\"", pos.name, arg);
std::exit(EXIT_FAILURE);
}
positional_filled[positional_idx] = true;
if (!pos.is_vector) {
++positional_idx;
}
}
}

// required options/positionals
for (std::size_t o = 0; o < std::size(plan.options); ++o) {
if (plan.options[o].is_required && !option_seen[o]) {
std::println(stderr, "Missing required option --{}", plan.options[o].long_name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
}
for (std::size_t p = 0; p < std::size(plan.positionals); ++p) {
if (plan.positionals[p].is_required && !positional_filled[p]) {
std::println(stderr, "Missing required positional argument `{}`", plan.positionals[p].name);
cli::print_help<Opts>(argv0);
std::exit(EXIT_FAILURE);
}
Expand All@@ -216,7 +197,6 @@ auto parse_into(Opts &opts, std::string_view argv0, std::span<const std::string_
// TODO: range / choices / validate_with enforcement.
// TODO: env-var fallback when not matched.
// TODO: on_match action invocation.
// TODO: report unknown options not matched by any member.
}

// handle subcommand
Expand Down
82 changes: 82 additions & 0 deletions tests/test_positionals.cpp
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
// Regression tests for argument consumption: a flag's value must never be
// mis-collected as a positional (B1), and positionals interleaved with flags
// must still land in the right members.

#include <cli/cli.hpp>
#include <gtest/gtest.h>

#include <array>
#include <span>
#include <string>
#include <string_view>
#include <vector>

namespace {

struct [[=cli::program("test")]] Opts {
[[=cli::desc("Source file"), =cli::positional, =cli::required]]
std::string source;

[[=cli::desc("Include dirs"), =cli::short_name('I'), =cli::long_name("include")]]
std::vector<std::string> includes;

[[=cli::desc("Port"), =cli::short_name('p')]]
int port = 0;

[[=cli::desc("Verbose"), =cli::short_name('v')]]
bool verbose = false;
};

template <std::size_t N>
auto parse(std::array<std::string_view, N> const &args) -> Opts {
return cli::parse<Opts>(std::string_view{"test"}, std::span{args.data(), args.size()});
}

} // namespace

// The core B1 bug: `--include foo.h target.cpp` must put target.cpp in the
// positional and foo.h in the vector option — not foo.h in both.
TEST(positionals, flag_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--include", "foo.h", "target.cpp"});
EXPECT_EQ(opts.source, "target.cpp");
ASSERT_EQ(opts.includes.size(), 1u);
EXPECT_EQ(opts.includes[0], "foo.h");
}

// Scalar flag value (a number) must not be grabbed as the positional.
TEST(positionals, scalar_value_not_collected_as_positional) {
auto opts = parse(std::array<std::string_view, 3>{"--port", "9090", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Positional before the flag.
TEST(positionals, positional_before_flag) {
auto opts = parse(std::array<std::string_view, 3>{"main.cpp", "--port", "9090"});
EXPECT_EQ(opts.source, "main.cpp");
EXPECT_EQ(opts.port, 9090);
}

// Bool flag interleaved with positional — bool consumes no value.
TEST(positionals, bool_flag_does_not_eat_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--verbose", "main.cpp"});
EXPECT_TRUE(opts.verbose);
EXPECT_EQ(opts.source, "main.cpp");
}

// --name=value form leaves the following token as the positional.
TEST(positionals, equals_form_leaves_positional) {
auto opts = parse(std::array<std::string_view, 2>{"--port=9090", "main.cpp"});
EXPECT_EQ(opts.port, 9090);
EXPECT_EQ(opts.source, "main.cpp");
}

// Multiple repeated vector flags plus a trailing positional.
TEST(positionals, repeated_vector_flag_then_positional) {
auto opts =
parse(std::array<std::string_view, 5>{"-I", "a", "--include", "b", "main.cpp"});
EXPECT_EQ(opts.source, "main.cpp");
ASSERT_EQ(opts.includes.size(), 2u);
EXPECT_EQ(opts.includes[0], "a");
EXPECT_EQ(opts.includes[1], "b");
}
Loading