Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Parameters not ordered fix #11

Merged
merged 4 commits into from

2 participants

@yanick

I have two commits in this pull that take care of the two problems of issue #10.

The position of parameters is now recorded and taken into consideration as the parameters are munged.

More contentiously, I removed the @extras to be able to have parameters set after options. So that we can have:

foo bar --option X my-param-1 my-param-2

Please have a look and let me know what you think.

yanick added some commits
@yanick yanick parameters are ordered alphabetically
... which is not what one would expect. I would rather think that
parameters should be taken in the order in which they are
declared, or following a 'position' attribute.
7073ef7
@yanick yanick typo 'inavlid' 6920c3b
@yanick yanick parameter positions are recorded 32b09e9
@yanick yanick parameters can be after --options
To make that one happen, I had to remove the
extras in the parser. Which I don't think is
too horrible: they are nothing but trailing
parameters. We might want to do something smart
with a parameter that is an array and slurp them all.
421d34b
@maros maros merged commit 421d34b into maros:master
@maros
Owner

Thank you for this patch! I have just merged it with and made some minor amendments (eg. parameters defined in roles didn't work). The fixes will make it to CPAN soon.

@yanick

Excellent. Thanks a bunch!

@maros
Owner

Just released MooseX::App 1.20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 2, 2013
  1. @yanick

    parameters are ordered alphabetically

    yanick authored
    ... which is not what one would expect. I would rather think that
    parameters should be taken in the order in which they are
    declared, or following a 'position' attribute.
Commits on Apr 5, 2013
  1. @yanick

    typo 'inavlid'

    yanick authored
  2. @yanick
  3. @yanick

    parameters can be after --options

    yanick authored
    To make that one happen, I had to remove the
    extras in the parser. Which I don't think is
    too horrible: they are nothing but trailing
    parameters. We might want to do something smart
    with a parameter that is an array and slurp them all.
This page is out of date. Refresh to see the latest.
View
2  lib/MooseX/App.pm
@@ -77,7 +77,7 @@ sub new_with_command {
} elsif (scalar @args % 2 == 0) {
%args = @args;
} else {
- Moose->throw_error('new_with_command got inavlid extra arguments');
+ Moose->throw_error('new_with_command got invalid extra arguments');
}
# Get ARGV
View
18 lib/MooseX/App/Exporter.pm
@@ -11,6 +11,8 @@ use Moose::Exporter;
use MooseX::App::Utils;
use MooseX::App::ParsedArgv;
+use List::Util qw/ max /;
+
our %PLUGIN_SPEC;
sub import {
@@ -65,7 +67,21 @@ sub _handle_attribute {
|| 'MooseX::App::Meta::Role::Attribute::Option' ~~ $local_attributes{traits};
}
}
+
+ if ( $type eq 'parameter' ) {
+ no warnings 'uninitialized';
+ # if a 'position' is given, take it
+ # if not, get the next one in line
+ $local_attributes{cmd_position} ||=
+ 1 + max
+ map { $_->cmd_position }
+ grep { $_->cmd_type eq 'parameter' }
+ grep { $_->does('MooseX::App::Meta::Role::Attribute::Option') }
+ $meta->get_all_attributes;
+ }
+
$meta->add_attribute( $attr, %local_attributes );
+
}
return;
@@ -161,4 +177,4 @@ sub command_usage($) {
return $meta->command_usage($usage);
}
-1;
+1;
View
6 lib/MooseX/App/Meta/Role/Attribute/Option.pm
@@ -48,6 +48,12 @@ has 'cmd_aliases' => (
coerce => 1,
);
+has cmd_position => (
+ is => 'rw',
+ isa => 'Int',
+ default => 0,
+);
+
sub cmd_is_bool {
my ($self) = @_;
View
11 lib/MooseX/App/Meta/Role/Class/Base.pm
@@ -112,8 +112,10 @@ sub command_args {
}
# Process params
- my @attributes_parameter = $self->command_usage_attributes($metaclass,'parameter');
-
+ my @attributes_parameter = sort {
+ $a->cmd_position <=> $b->cmd_position
+ } $self->command_usage_attributes($metaclass,'parameter');
+
foreach my $attribute (@attributes_parameter) {
my $value = $parsed_argv->consume('parameters');
last
@@ -514,7 +516,10 @@ sub command_usage_parameters {
$metaclass ||= $self;
my @parameters;
- foreach my $attribute ($self->command_usage_attributes($metaclass,'parameter')) {
+ foreach my $attribute (
+ sort { $a->cmd_position <=> $b->cmd_position }
+ $self->command_usage_attributes($metaclass,'parameter')
+ ) {
push(@parameters,[
$attribute->cmd_usage_name(),
$attribute->cmd_usage_description()
View
5 lib/MooseX/App/ParsedArgv.pm
@@ -59,7 +59,8 @@ sub _parse {
foreach my $element (@{$self->argv}) {
if ($stopprocessing) {
- push(@extra,$element);
+ push @parameters, MooseX::App::ParsedArgv::Element->new( key =>
+ $element );
} else {
given ($element) {
# Flags
@@ -102,8 +103,6 @@ sub _parse {
if (defined $lastkey) {
$lastkey->add_value($element);
undef $lastkey;
- } elsif (scalar @options) {
- push(@extra,$element);
} else {
push(@parameters,MooseX::App::ParsedArgv::Element->new( key => $element ));
}
View
4 lib/MooseX/App/Simple.pm
@@ -69,7 +69,7 @@ sub new_with_options {
} elsif (scalar @args % 2 == 0) {
%args = @args;
} else {
- Moose->throw_error('new_with_command got inavlid extra arguments');
+ Moose->throw_error('new_with_command got invalid extra arguments');
}
# Get ARGV
@@ -162,4 +162,4 @@ MooseX::App command line application.
See L<MooseX::Getopt> and L<MooX::Options> for alternatives
-=cut
+=cut
View
10 t/03_utils.t
@@ -60,10 +60,10 @@ subtest 'Parser' => sub {
is($parser->parameters->[0]->key,'hello','Parameter parsed ok');
is($parser->parameters->[1]->key,'mellow','Parameter parsed ok');
- is(scalar @{$parser->parameters},'2','Two parameters');
- is($parser->extra->[0],'baer','Extra parsed ok');
- is($parser->extra->[1],'hase','Extra parsed ok');
- is(scalar @{$parser->extra},'2','Two extra values');
+ is(scalar @{$parser->parameters},4,'Four parameters');
+ is($parser->parameters->[2]->key,'baer','Extra parsed ok');
+ is($parser->parameters->[3]->key,'hase','Extra parsed ok');
+ is(scalar @{$parser->extra},0,'Two extra values');
is($parser->options->[0]->key,'h','Parsed -h flag');
is($parser->options->[0]->has_values,0,'-h is flag');
is($parser->options->[1]->key,'u','Parsed -u flag');
@@ -81,4 +81,4 @@ subtest 'Parser' => sub {
is($parser->options->[5]->get_value(0),'value1','--key first value ok');
is($parser->options->[5]->get_value(1),'value2','--key second value ok');
is(scalar @{$parser->options},6,'Has 6 options/flags');
-};
+};
View
27 t/05_extended.t
@@ -2,7 +2,7 @@
# t/05_extended.t - Extended tests
-use Test::Most tests => 19+1;
+use Test::Most tests => 21;
use Test::NoWarnings;
use FindBin qw();
@@ -150,7 +150,20 @@ subtest 'Test positional params' => sub {
local @ARGV = qw(extra hui --value baer);
my $test12 = Test03->new_with_command;
isa_ok($test12,'Test03::ExtraCommand');
- is($test12->extra1,'hui','Extra1 value is undef');
+ is($test12->extra1,'hui','Extra1 value is "hui"');
+ is($test12->extra2, undef,'Extra2 value is undef');
+ is($test12->alpha, undef,'alpha value is undef');
+ is($test12->value,'baer','Value is set');
+};
+
+subtest 'Test positional params' => sub {
+ local @ARGV = qw(extra --value baer hui);
+ $MooseX::App::ParsedArgv::StopIt = 1;
+ my $test12 = Test03->new_with_command;
+ isa_ok($test12,'Test03::ExtraCommand');
+ is($test12->extra1,'hui','Extra1 value is "hui"');
+ is($test12->extra2, undef,'Extra2 value is undef');
+ is($test12->alpha, undef,'alpha value is undef');
is($test12->value,'baer','Value is set');
};
@@ -158,8 +171,9 @@ subtest 'Test optional positional params' => sub {
local @ARGV = qw(extra hui 11 --value baer);
my $test12 = Test03->new_with_command;
isa_ok($test12,'Test03::ExtraCommand');
- is($test12->extra1,'hui','Extra1 value is undef');
- is($test12->extra2,11,'Extra2 value is undef');
+ is($test12->extra1,'hui','Extra1 value is "hui"');
+ is($test12->extra2,11,'Extra2 value is "11"');
+ is($test12->alpha, undef,'alpha value is undef');
is($test12->value,'baer','Value is set');
};
@@ -170,7 +184,8 @@ subtest 'Test wrong positional params' => sub {
is($test13->blocks->[0]->header,"Invalid value for 'extra2'","Error message ok");
is($test13->blocks->[2]->header,"parameters:","Usage header ok");
is($test13->blocks->[2]->body," extra1 Important extra parameter [Required]
- extra2 [Integer]","Usage body ok");
+ extra2 [Integer]
+ alpha [Integer]","Usage body ok");
};
subtest 'Test missing positional params' => sub {
@@ -178,4 +193,4 @@ subtest 'Test missing positional params' => sub {
my $test13 = Test03->new_with_command;
isa_ok($test13,'MooseX::App::Message::Envelope');
is($test13->blocks->[0]->header,"Required parameter 'extra1' missing","Message ok");
-};
+};
View
4 t/09_classes.t
@@ -23,7 +23,7 @@ subtest 'Wrong usage' => sub {
throws_ok { Test03->new->new_with_command } qr/new_with_command is a class method/, 'Only callable as class method';
use Test03::SomeCommand;
throws_ok { Test03::SomeCommand->new_with_command } qr/new_with_command may only be called from the application base package/, 'new_with_command may only be called from the application base package';
- throws_ok { Test03->new_with_command(1,2,3) } qr/new_with_command got inavlid extra arguments/, 'Wrong default args';
+ throws_ok { Test03->new_with_command(1,2,3) } qr/new_with_command got invalid extra arguments/, 'Wrong default args';
};
@@ -32,4 +32,4 @@ subtest 'Conflicts' => sub {
throws_ok {
Test03->new_with_command;
} qr/Command line option conflict/, 'Conflict detected';
-};
+};
View
7 t/testlib/Test03/ExtraCommand.pm
@@ -14,6 +14,11 @@ parameter 'extra2' => (
isa => 'Int',
);
+parameter 'alpha' => (
+ is => 'rw',
+ isa => 'Int',
+);
+
option 'value' => (
is => 'rw',
isa => 'Str',
@@ -30,4 +35,4 @@ sub run {
}
}
-1;
+1;
Something went wrong with that request. Please try again.