* MooseX::Getopt: _get_options_from_configfile renamed to get_options_from_configfile.

* MooseX::Getopt: get_options_from_configfile and get_options_from_argv returns hashref that hash.
* MooseX::Getopt::Session, MooseX::Getopt::Parser::*: Removed params attribute.
* t/008_configfromfile.t: Tests for different parsers.

diff --git a/lib/MooseX/Getopt.pm b/lib/MooseX/Getopt.pm
index 27c17c8..0e4a268 100644 (file)
--- a/lib/MooseX/Getopt.pm
+++ b/lib/MooseX/Getopt.pm
@@ -40,30 +40,29 @@ sub get_options_from_argv {
     Moose->throw_error("Single parameters to get_options_from_argv() must be a HASH ref")
         if ref $_[0] and ref $_ ne 'HASH';
 
-    my %params = ( $class->_get_options_from_configfile, @_ == 1 ? %{ $_[0] } : @_ );
+    my $options = { %{ $class->get_options_from_configfile }, @_ == 1 ? %{ $_[0] } : @_ };
 
-    my $getopt = defined $params{getopt}
-                 ? $params{getopt}
+    my $getopt = defined $options->{getopt}
+                 ? $options->{getopt}
                  : $class->_default_getopt_session->new(
                        classes_filter => sub { $_ eq $class },
-                       params => \%params,
+                       options => $options,
                    );
 
-    my %new_params = (
-        %{ $getopt->params },                   # params from session object
-        %params,                                # explicit params to ->new
-        %{ $getopt->options },                  # params from CLI
+    my $new_options = {
+        %{ $options },                          # explicit options to ->new
+        %{ $getopt->options },                  # options from CLI
         getopt => $getopt,
-    );
+    };
 
-    return %new_params;
+    return $new_options;
 };
 
 
-sub _get_options_from_configfile {
+sub get_options_from_configfile {
     my $class = shift;
 
-    my %params = ();
+    my $options = {};
 
     if ($class->meta->does_role('MooseX::ConfigFromFile')) {
         local @ARGV = @ARGV;
@@ -78,11 +77,11 @@ sub _get_options_from_configfile {
         };
 
         if (defined $configfile) {
-            %params = %{ $class->get_config_from_file($configfile) };
+            $options = $class->get_config_from_file($configfile);
         };
     };
 
-    return %params;
+    return $options;
 };
 
 
@@ -339,6 +338,11 @@ C<new>.
 This method returns the list of parameters collected from command line
 without creating the new object.
 
+=item B<get_options_from_configfile>
+
+This method returns the list of parameters collected with
+L<MooseX::ConfigFromFile> mechanism.
+
 =item B<ARGV>
 
 This accessor contains a reference to a copy of the C<@ARGV> array as it

diff --git a/lib/MooseX/Getopt/Parser/Descriptive.pm b/lib/MooseX/Getopt/Parser/Descriptive.pm
index c60839c..c212eb1 100644 (file)
--- a/lib/MooseX/Getopt/Parser/Descriptive.pm
+++ b/lib/MooseX/Getopt/Parser/Descriptive.pm
@@ -32,7 +32,9 @@ sub build_options {
     Moose->throw_error('First argument is not a MooseX::Getopt::Session')
         unless $getopt->isa('MooseX::Getopt::Session');
 
-    my $options = {};
+    my $options = $getopt->options;
+    my $new_options = {};
+
     my $usage;
     my (@opts, %cmd_flags_to_names);
 
@@ -51,7 +53,7 @@ sub build_options {
         $doc = $attr->documentation if $attr->has_documentation;
         $doc = ' ' unless $doc;
 
-        my $is_required = !exists $getopt->params->{$name}
+        my $is_required = !exists $options->{$name}
                           && $attr->is_required
                           && !$attr->has_default
                           && !$attr->has_builder;
@@ -60,7 +62,7 @@ sub build_options {
             $opt_string => $doc,
             {
                 ( $is_required ? ( required => $attr->is_required ) : () ),
-            }
+            },
         ];
     };
 
@@ -75,7 +77,7 @@ sub build_options {
         };
 
         eval {
-            ($options, $usage) = Getopt::Long::Descriptive::describe_options(
+            ($new_options, $usage) = Getopt::Long::Descriptive::describe_options(
                 $self->format, @opts, { getopt_conf => [ $self->config ] }
             );
         };
@@ -87,13 +89,16 @@ sub build_options {
     };
 
     # Convert cmd_flags back to names in options hashref
-    $options = { map { $cmd_flags_to_names{$_} => $options->{$_} } keys %$options };
+    $new_options = { map { $cmd_flags_to_names{$_} => $new_options->{$_} } keys %$new_options };
+
+    # Include old options and usage object
+    $new_options = { usage => $usage, %$options, %$new_options };
 
-    $getopt->options( $options );
+    $getopt->options( $new_options );
 
     die $warnings if $warnings;
 
-    return $options;
+    return $new_options;
 };
 
 

diff --git a/lib/MooseX/Getopt/Parser/Long.pm b/lib/MooseX/Getopt/Parser/Long.pm
index d676e1b..fb369ce 100644 (file)
--- a/lib/MooseX/Getopt/Parser/Long.pm
+++ b/lib/MooseX/Getopt/Parser/Long.pm
@@ -25,7 +25,7 @@ sub build_options {
     Moose->throw_error('First argument is not a MooseX::Getopt::Session')
         unless $getopt->isa('MooseX::Getopt::Session');
 
-    my $options = {};
+    my $options = $getopt->options;
     my @opts;
 
     foreach my $attr (@attrs) {

diff --git a/lib/MooseX/Getopt/Session.pm b/lib/MooseX/Getopt/Session.pm
index 3359f9a..bdb0417 100644 (file)
--- a/lib/MooseX/Getopt/Session.pm
+++ b/lib/MooseX/Getopt/Session.pm
@@ -23,13 +23,6 @@ has classes_filter => (
     default => sub { sub { 1 } },
 );
 
-# Explicite parameters for new_with_options
-has params => (
-    is => 'rw',
-    isa => 'HashRef',
-    default => sub { {} },
-);
-
 # Original @ARGV values
 has ARGV => (
     is => 'rw',
@@ -171,11 +164,6 @@ searching proper classes to create options list.  The filter passes any
 class by default but L<MooseX::Getopt> will search the attributes only
 in own class, if new session is created implicity.
 
-=item B<params>
-
-This accessor contains the parameters which will be included to each
-L<MooseX::Getopt>->new_with_options call.
-
 =item B<ARGV>
 
 This accessor contains a reference to a copy of the C<@ARGV> array as it
@@ -189,8 +177,9 @@ un-mangled.
 
 =item B<options>
 
-This accessor contains an arrayref of options parsed from command line
-by L<MooseX::Getopt::Parser>.
+This accessor contains an arrayref of options parsed from command line by
+L<MooseX::Getopt::Parser>.  If the options list are not empty before parsing
+the command line, the old list will be included to new list.
 
 =item B<BUILD>
 

diff --git a/t/008_configfromfile.t b/t/008_configfromfile.t
index 93700b9..8fc724c 100644 (file)
--- a/t/008_configfromfile.t
+++ b/t/008_configfromfile.t
@@ -12,7 +12,7 @@ if ( !eval { require MooseX::ConfigFromFile } )
 }
 else
 {
-    plan tests => 25;
+    plan tests => 119;
 }
 
 {
@@ -74,112 +74,183 @@ else
     );
 }
 
-# No config specified
-{
-    local @ARGV = qw( --required_from_argv 1 );
+foreach my $parser_name (qw(MooseX::Getopt::Parser::Long MooseX::Getopt::Parser::Descriptive MooseX::Getopt::Parser::Default)) {
+    SKIP: {
+        if ($parser_name eq 'MooseX::Getopt::Parser::Descriptive') {
+            eval { require Getopt::Long::Descriptive };
+            skip "Getopt::Long::Descriptive not installed", 39 if $@;
+        }
 
-    throws_ok { App->new_with_options } qr/Required option missing: required_from_config/;
+        # No config specified
+        {
+            local @ARGV = qw( --required_from_argv 1 );
 
-    {
-        my $app = App::DefaultConfigFile->new_with_options;
-        isa_ok( $app, 'App::DefaultConfigFile' );
-        app_ok( $app );
+            throws_ok {
+                my $parser = $parser_name->new;
+                ok(ref($parser) =~ /^MooseX::Getopt::Parser::/, '... parser object is created');
 
-        ok(  !$app->config_from_override,
-            '... config_from_override false as expected' );
+                my $options = App->get_options_from_configfile;
+                my $getopt = MooseX::Getopt::Session->new( parser => $parser, options => $options, classes_filter => sub { $_ eq 'App' } );
+                isa_ok($getopt, 'MooseX::Getopt::Session');
 
-        is( $app->configfile, '/notused/default',
-            '... configfile is /notused/default as expected' );
-    }
-}
+                App->new_with_options( getopt => $getopt );
+            } qr/required_from_config/;
 
-# Config specified
-{
-    local @ARGV = qw( --configfile /notused --required_from_argv 1 );
+            {
+                my $parser = $parser_name->new;
+                ok(ref($parser) =~ /^MooseX::Getopt::Parser::/, '... parser object is created');
 
-    {
-        my $app = App->new_with_options;
-        isa_ok( $app, 'App' );
-        app_ok( $app );
-    }
+                my $options = App::DefaultConfigFile->get_options_from_configfile;
+                my $getopt = MooseX::Getopt::Session->new( parser => $parser, options => $options, classes_filter => sub { $_ eq 'App::DefaultConfigFile' } );
+                isa_ok($getopt, 'MooseX::Getopt::Session');
 
-    {
-        my $app = App::DefaultConfigFile->new_with_options;
-        isa_ok( $app, 'App::DefaultConfigFile' );
-        app_ok( $app );
+                my $app = App::DefaultConfigFile->new_with_options( getopt => $getopt );
+                isa_ok( $app, 'App::DefaultConfigFile' );
+                app_ok( $app );
 
-        ok( $app->config_from_override,
-             '... config_from_override true as expected' );
+                ok(  !$app->config_from_override,
+                    '... config_from_override false as expected' );
 
-        is( $app->configfile, '/notused',
-            '... configfile is /notused as expected' );
-    }
-}
+                is( $app->configfile, '/notused/default',
+                    '... configfile is /notused/default as expected' );
+            }
+        }
 
-# Required arg not supplied from cmdline
-{
-    local @ARGV = qw( --configfile /notused );
-    throws_ok { App->new_with_options } qr/Required option missing: required_from_argv/;
-}
+        # Config specified
+        {
+            local @ARGV = qw( --configfile /notused --required_from_argv 1 );
 
-# Config file value overriden from cmdline
-{
-    local @ARGV = qw( --configfile /notused --required_from_argv 1 --required_from_config override );
+            {
+                my $parser = $parser_name->new;
+                ok(ref($parser) =~ /^MooseX::Getopt::Parser::/, '... parser object is created');
 
-    my $app = App->new_with_options;
-    isa_ok( $app, 'App' );
+                my $options = App->get_options_from_configfile;
+                my $getopt = MooseX::Getopt::Session->new( parser => $parser, options => $options, classes_filter => sub { $_ eq 'App' } );
+                isa_ok($getopt, 'MooseX::Getopt::Session');
 
-    is( $app->required_from_config, 'override',
-        '... required_from_config is override as expected' );
+                my $app = App->new_with_options( getopt => $getopt );
+                isa_ok( $app, 'App' );
+                app_ok( $app );
+            }
 
-    is( $app->optional_from_config, 'from_config_2',
-        '... optional_from_config is from_config_2 as expected' );
-}
+            {
+                my $parser = $parser_name->new;
+                ok(ref($parser) =~ /^MooseX::Getopt::Parser::/, '... parser object is created');
 
-# No config file
-{
-    local @ARGV = qw( --required_from_argv 1 --required_from_config noconfig );
+                my $options = App::DefaultConfigFile->get_options_from_configfile;
+                my $getopt = MooseX::Getopt::Session->new( parser => $parser, options => $options, classes_filter => sub { $_ eq 'App::DefaultConfigFile' } );
+                isa_ok($getopt, 'MooseX::Getopt::Session');
 
-    my $app = App->new_with_options;
-    isa_ok( $app, 'App' );
+                my $app = App::DefaultConfigFile->new_with_options( getopt => $getopt );
+                isa_ok( $app, 'App::DefaultConfigFile' );
+                app_ok( $app );
 
-    is( $app->required_from_config, 'noconfig',
-        '... required_from_config is noconfig as expected' );
+                ok( $app->config_from_override,
+                     '... config_from_override true as expected' );
 
-    ok( !defined $app->optional_from_config,
-        '... optional_from_config is undef as expected' );
-}
+                is( $app->configfile, '/notused',
+                    '... configfile is /notused as expected' );
+            }
+        }
 
-{
-    package BaseApp::WithConfig;
-    use Moose;
-    with 'MooseX::ConfigFromFile';
+        # Required arg not supplied from cmdline
+        {
+            local @ARGV = qw( --configfile /notused );
+            throws_ok {
+                my $parser = $parser_name->new;
+                ok(ref($parser) =~ /^MooseX::Getopt::Parser::/, '... parser object is created');
 
-    sub get_config_from_file { return {}; }
-}
+                my $options = App->get_options_from_configfile;
+                my $getopt = MooseX::Getopt::Session->new( parser => $parser, options => $options, classes_filter => sub { $_ eq 'App' } );
+                isa_ok($getopt, 'MooseX::Getopt::Session');
 
-{
-    package DerivedApp::Getopt;
-    use Moose;
-    extends 'BaseApp::WithConfig';
-    with 'MooseX::Getopt';
-}
+                App->new_with_options( getopt => $getopt );
+            } qr/required_from_argv/;
+        }
 
-# With DerivedApp, the Getopt role was applied at a different level
-# than the ConfigFromFile role
-{
-    lives_ok { DerivedApp::Getopt->new_with_options } 'Can create DerivedApp';
-}
+        # Config file value overriden from cmdline
+        {
+            local @ARGV = qw( --configfile /notused --required_from_argv 1 --required_from_config override );
+
+            my $parser = $parser_name->new;
+            ok(ref($parser) =~ /^MooseX::Getopt::Parser::/, '... parser object is created');
+
+            my $options = App->get_options_from_configfile;
+            my $getopt = MooseX::Getopt::Session->new( parser => $parser, options => $options, classes_filter => sub { $_ eq 'App' } );
+            isa_ok($getopt, 'MooseX::Getopt::Session');
+
+            my $app = App->new_with_options( getopt => $getopt );
+            isa_ok( $app, 'App' );
+
+            is( $app->required_from_config, 'override',
+                '... required_from_config is override as expected' );
+
+            is( $app->optional_from_config, 'from_config_2',
+                '... optional_from_config is from_config_2 as expected' );
+        }
+
+        # No config file
+        {
+            local @ARGV = qw( --required_from_argv 1 --required_from_config noconfig );
 
-sub app_ok {
-    my $app = shift;
+            my $parser = $parser_name->new;
+            ok(ref($parser) =~ /^MooseX::Getopt::Parser::/, '... parser object is created');
 
-    is( $app->required_from_config, 'from_config_1',
-        '... required_from_config is from_config_1 as expected' );
+            my $options = App->get_options_from_configfile;
+            my $getopt = MooseX::Getopt::Session->new( parser => $parser, options => $options, classes_filter => sub { $_ eq 'App' } );
+            isa_ok($getopt, 'MooseX::Getopt::Session');
 
-    is( $app->optional_from_config, 'from_config_2',
-        '... optional_from_config is from_config_2 as expected' );
+            my $app = App->new_with_options( getopt => $getopt );
+            isa_ok( $app, 'App' );
 
-    is( $app->required_from_argv, '1',
-        '... required_from_argv is 1 as expected' );
+            is( $app->required_from_config, 'noconfig',
+                '... required_from_config is noconfig as expected' );
+
+            ok( !defined $app->optional_from_config,
+                '... optional_from_config is undef as expected' );
+        }
+
+        {
+            package BaseApp::WithConfig;
+            use Moose;
+            with 'MooseX::ConfigFromFile';
+
+            sub get_config_from_file { return {}; }
+        }
+
+        {
+            package DerivedApp::Getopt;
+            use Moose;
+            extends 'BaseApp::WithConfig';
+            with 'MooseX::Getopt';
+        }
+
+        # With DerivedApp, the Getopt role was applied at a different level
+        # than the ConfigFromFile role
+        {
+            lives_ok {
+                my $parser = $parser_name->new;
+                ok(ref($parser) =~ /^MooseX::Getopt::Parser::/, '... parser object is created');
+
+                my $getopt = MooseX::Getopt::Session->new( parser => $parser, classes_filter => sub { $_ eq 'DerivedApp::Getopt' } );
+                isa_ok($getopt, 'MooseX::Getopt::Session');
+
+                DerivedApp::Getopt->new_with_options( getopt => $getopt );
+            } 'Can create DerivedApp';
+        }
+
+        sub app_ok {
+            my $app = shift;
+
+            is( $app->required_from_config, 'from_config_1',
+                '... required_from_config is from_config_1 as expected' );
+
+            is( $app->optional_from_config, 'from_config_2',
+                '... optional_from_config is from_config_2 as expected' );
+
+            is( $app->required_from_argv, '1',
+                '... required_from_argv is 1 as expected' );
+        }
+
+    }
 }

diff --git a/t/009_gld_and_explicit_options.t b/t/009_gld_and_explicit_options.t
index e99a216..40a20e4 100644 (file)
--- a/t/009_gld_and_explicit_options.t
+++ b/t/009_gld_and_explicit_options.t
@@ -39,8 +39,7 @@ BEGIN {
     my $parser = MooseX::Getopt::Parser::Descriptive->new;
     isa_ok($parser, 'MooseX::Getopt::Parser::Descriptive');
 
-    my %params = (baz => 100);
-    my $getopt = MooseX::Getopt::Session->new(parser => $parser, params => \%params);
+    my $getopt = MooseX::Getopt::Session->new(parser => $parser, options => {baz => 100});
 
     my $foo;
     lives_ok {