From: Piotr Roszatycki Date: Thu, 13 Nov 2008 18:04:29 +0000 (+0000) Subject: * MooseX::Getopt: _get_options_from_configfile renamed to get_options_from_configfile. X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=10ed52cb080cfbe757fdb2a4c7ceb463fba93e6c;p=gitmo%2FMooseX-Getopt.git * 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 --- 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. This method returns the list of parameters collected from command line without creating the new object. +=item B + +This method returns the list of parameters collected with +L mechanism. + =item B 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 --- 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 --- 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 --- 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 will search the attributes only in own class, if new session is created implicity. -=item B - -This accessor contains the parameters which will be included to each -L->new_with_options call. - =item B This accessor contains a reference to a copy of the C<@ARGV> array as it @@ -189,8 +177,9 @@ un-mangled. =item B -This accessor contains an arrayref of options parsed from command line -by L. +This accessor contains an arrayref of options parsed from command line by +L. If the options list are not empty before parsing +the command line, the old list will be included to new list. =item B diff --git a/t/008_configfromfile.t b/t/008_configfromfile.t index 93700b9..8fc724c 100644 --- 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 --- 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 {