Skip to content

Commit 5904c71

Browse files
Gettyclaude
Getty
and
claude
committed
cli: accept options before and between positional arguments
Commands read positionals as $args_ref->[N], but MooX::Cmd echoes the raw argv back into execute() — so `archive --json 1` parsed "--json" as the id list and died opaquely, `handoff --claim tester 1` looked up task "--claim", and `show --last 2` crashed with zero positionals. Worse, the extra-args guard counted only the leading dash-free run, so `archive 1 --json 99` silently dropped the 99 while archiving task 1. A positional_args extractor in Role::BoardAccess now subtracts option tokens from argv using the command's own _options_data: value-taking options swallow their following token (even flag-shaped values like --append-body '--weird'), --opt=value forms and short aliases (-a, -t, -y) are resolved, unknown dash tokens are defensively non-consuming. The seven positional-taking commands read positionals from it, and check_positional_args counts real positionals, closing the silent-drop hole. Flags can sit anywhere, matching cobra's behavior in kanban-md. Known residual: Getopt::Long abbreviations of value-taking options (--cl tester) reject with a clean usage error instead of resolving — safe, side-effect-free, and no worse than before. Adds t/45-options-before-positionals.t. Closes karr task #13. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GLsGFe6Vt98jHpCJmBwmMi
1 parent 4218164 commit 5904c71

10 files changed

Lines changed: 407 additions & 21 deletions

File tree

‎Changes‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,15 @@
11
{{$NEXT}}
22

3+
- Options may now be placed before, between, or after positional
4+
arguments (`karr archive --json 1`, `karr handoff --claim tester 1`,
5+
`karr edit --title New 3`), matching how the kanban-md CLI behaves.
6+
Previously the raw option token was read as the task id, giving opaque
7+
errors like "Task --claim not found" — and `karr show --last N` crashed
8+
outright. The real positionals are now extracted from argv using each
9+
command's own option metadata (value-taking options swallow their
10+
value, `--opt=value` and short aliases like `-a` included), which also
11+
closes a hole in the surplus-argument guard: `karr archive 1 --json 99`
12+
used to silently drop the 99 while still archiving task 1.
313
- Fix `karr delete` crashing on every ref-backed task ("Can't call method
414
\"remove\" on an undefined value") — it tried to unlink the task's
515
on-disk file, which tasks loaded from `refs/karr/*` never have, and

‎lib/App/karr/Cmd/Archive.pm‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ sub execute {
3838

3939
$self->sync_before;
4040

41-
my $id_str = $args_ref->[0] or die "Usage: karr archive ID[,ID,...]\n";
41+
my @pos = $self->positional_args($args_ref);
42+
my $id_str = $pos[0] or die "Usage: karr archive ID[,ID,...]\n";
4243

4344
my @ids = $self->parse_ids($id_str);
4445
my @results;

‎lib/App/karr/Cmd/Create.pm‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,8 @@ sub execute {
115115

116116
$self->sync_before;
117117

118-
my $title = $self->title // $args_ref->[0]
118+
my @pos = $self->positional_args($args_ref);
119+
my $title = $self->title // $pos[0]
119120
or die "Title is required. Use --title or pass as argument.\n";
120121

121122
my $ec = $self->store->effective_config;

‎lib/App/karr/Cmd/Delete.pm‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,8 @@ sub execute {
5555

5656
$self->sync_before;
5757

58-
my $id_str = $args_ref->[0] or die "Usage: karr delete ID[,ID,...] [--yes] [--json]\n";
58+
my @pos = $self->positional_args($args_ref);
59+
my $id_str = $pos[0] or die "Usage: karr delete ID[,ID,...] [--yes] [--json]\n";
5960
my @ids = $self->parse_ids($id_str);
6061

6162
my @results;

‎lib/App/karr/Cmd/Edit.pm‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,8 @@ sub execute {
143143

144144
$self->sync_before;
145145

146-
my $id_str = $args_ref->[0] or die "Usage: karr edit ID[,ID,...] [FLAGS]\n";
146+
my @pos = $self->positional_args($args_ref);
147+
my $id_str = $pos[0] or die "Usage: karr edit ID[,ID,...] [FLAGS]\n";
147148
my @ids = $self->parse_ids($id_str);
148149

149150
my @results;

‎lib/App/karr/Cmd/Handoff.pm‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,8 @@ sub execute {
9191

9292
$self->sync_before;
9393

94-
my $id = $args_ref->[0] or die "Usage: karr handoff ID --claim NAME [--note TEXT] [--block REASON] [--release]\n";
94+
my @pos = $self->positional_args($args_ref);
95+
my $id = $pos[0] or die "Usage: karr handoff ID --claim NAME [--note TEXT] [--block REASON] [--release]\n";
9596

9697
my $ec = $self->store->effective_config;
9798

‎lib/App/karr/Cmd/Move.pm‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,10 @@ sub execute {
7272

7373
$self->sync_before;
7474

75-
my $id_str = $args_ref->[0] or die "Usage: karr move ID[,ID,...] [STATUS]\n";
75+
my @pos = $self->positional_args($args_ref);
76+
my $id_str = $pos[0] or die "Usage: karr move ID[,ID,...] [STATUS]\n";
7677
my @ids = $self->parse_ids($id_str);
77-
my $new_status = $args_ref->[1];
78+
my $new_status = $pos[1];
7879

7980
my $ec = $self->store->effective_config;
8081
my @statuses = $self->store->all_status_names;

‎lib/App/karr/Cmd/Show.pm‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -139,7 +139,8 @@ sub execute {
139139

140140
$self->check_positional_args($args_ref, 1);
141141

142-
my @tasks = $self->_select_tasks($args_ref->[0]);
142+
my @pos = $self->positional_args($args_ref);
143+
my @tasks = $self->_select_tasks($pos[0]);
143144

144145
unless (@tasks) {
145146
print "No tasks found.\n" unless $self->json;

‎lib/App/karr/Role/BoardAccess.pm‎

Lines changed: 55 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,25 +48,67 @@ sub parse_ids {
4848
return split /,/, $id_str;
4949
}
5050

51+
# Extract the real positional arguments from the argv MooX::Cmd echoes back
52+
# into execute(). Because MooX::Options runs with protect_argv (its default),
53+
# the array handed to execute() still holds every original token in order:
54+
# recognised option flags, the values they consumed, and --opt=value forms
55+
# included (e.g. `move --claim tester 1 in-progress` arrives verbatim as
56+
# [--claim, tester, 1, in-progress]). This gives cobra-style freedom to place
57+
# flags before, between, or after positionals -- we just have to subtract the
58+
# option tokens back out.
59+
#
60+
# We do that by walking the argv against the command's own %{_options_data}:
61+
# a dash token is an option; if that option takes a value (has a 'format') and
62+
# is given in space form (no inline '='), it also swallows the following token
63+
# as its value -- even a flag-shaped value like `--append-body --weird`.
64+
# Everything not eaten as an option or an option value is a positional, in
65+
# order. Option-name matching mirrors how the token reaches us: leading dashes
66+
# stripped, '-' folded to '_' to hit the underscore keys in _options_data, plus
67+
# a reverse map of short aliases (e.g. -a => append_body, -t => timestamp).
68+
# An unrecognised dash token is treated defensively as non-consuming: a genuine
69+
# typo would already have been rejected upstream by MooX::Options, so the only
70+
# accepted-but-unmatched shape here is a Getopt::Long abbreviation, and karr's
71+
# abbreviatable flags (e.g. --jso for --json) consume nothing anyway.
72+
sub positional_args {
73+
my ($self, $args_ref) = @_;
74+
75+
my %options_data = $self->_options_data;
76+
my %by_name;
77+
for my $name (keys %options_data) {
78+
$by_name{$name} = $options_data{$name};
79+
my $short = $options_data{$name}{short};
80+
next unless defined $short;
81+
$by_name{$_} = $options_data{$name} for split /\|/, $short;
82+
}
83+
84+
my @positional;
85+
my @args = @$args_ref;
86+
while (@args) {
87+
my $arg = shift @args;
88+
if ($arg =~ /^-/) {
89+
(my $name = $arg) =~ s/^-+//; # drop leading dashes
90+
my $has_inline = $name =~ s/=.*//s; # --opt=value carries its value
91+
$name =~ tr/-/_/; # match underscore keys
92+
my $data = $by_name{$name};
93+
shift @args if $data && $data->{format} && !$has_inline && @args;
94+
next;
95+
}
96+
push @positional, $arg;
97+
}
98+
return @positional;
99+
}
100+
51101
# Reject surplus positional arguments before a command does any work, matching
52102
# kanban-md's cobra Args validators (ExactArgs/RangeArgs/MaximumNArgs) which
53103
# refuse extra positionals ahead of RunE. The comma list stays the one and only
54-
# batch syntax; there is no space-separated id batch.
55-
#
56-
# MooX::Cmd hands execute() the raw argv and echoes parsed option flags *and*
57-
# their values back into it (e.g. `move 1 --next --claim tester` arrives as
58-
# [1, --next, --claim, tester]). Positionals always precede options on the
59-
# command line, so the positional count is the leading run of non-dash tokens;
60-
# stopping at the first option flag ignores both the flags and their echoed
61-
# values.
104+
# batch syntax; there is no space-separated id batch. Counting is done against
105+
# positional_args (the real positionals with option tokens subtracted out), not
106+
# a leading run of non-dash tokens, so `archive 1 --json 99` correctly rejects
107+
# the trailing "99" instead of silently dropping it.
62108
sub check_positional_args {
63109
my ($self, $args_ref, $max) = @_;
64110

65-
my @positional;
66-
for my $arg (@$args_ref) {
67-
last if $arg =~ /^-/;
68-
push @positional, $arg;
69-
}
111+
my @positional = $self->positional_args($args_ref);
70112
return if @positional <= $max;
71113

72114
my @extra = @positional[$max .. $#positional];

0 commit comments

Comments
 (0)