Re: [PATCH] MooseX::Getopt - options shouldn't be required if loaded from config

[email protected] (Ryan D Johnson) Tue, 22 Apr 2008 17:02:37 -0700

Newsgroups	perl.moose
Message-ID	<[email protected]>

Ryan D Johnson <[email protected]> writes:

> I'm attaching a patch with tests. 

I discovered another issue while messing with this stuff. Since Getopt
uses $class->meta->get_attribute instead of
$class->meta->find_attribute_by_name when looking for the 'configfile'
default value, it only works if the ConfigFromFile and Getopt roles were
applied at the "same level". In other words:

{
    package Parent;
    use Moose;
    with 'MooseX::ConfigFromFile';
    sub get_config_from_file { return {}; }
}

{
    package Child;
    use Moose;
    extends 'Parent';
    with 'MooseX::Getopt';
}

Child->new_with_options() # always throws

Here's a failing test and patch to fix it - this stacks on the previous
diff. 

http://git.innerfence.com/?p=MooseX-Getopt.git;a=commitdiff;h=021a3b5870918fe0d5c4a96846ee3fc748bdbd90

MooseX-Getopt-ConfigFromFileGetoptDifferentLevels.diff (text/x-diff, 1.8 KB)

diff --git a/ChangeLog b/ChangeLog
index 444fca5..980d80b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -5,6 +5,8 @@ Revision history for Perl extension MooseX-Getopt
       - Commandline option shouldn't be required in the
         case that the given "required" attribute has
         been loaded from config (MooseX::ConfigFromFile)
+      - Support for MooseX::ConfigFromFile shouldn't require
+	that role be added at the same level as Getopt.
 
 0.12 Fri. March 14, 2008
     ~~ updated copyright dates ~~
diff --git a/lib/MooseX/Getopt.pm b/lib/MooseX/Getopt.pm
index 8c6385f..97d59c8 100644
--- a/lib/MooseX/Getopt.pm
+++ b/lib/MooseX/Getopt.pm
@@ -27,7 +27,7 @@ sub new_with_options {
         $opt_parser->getoptions( "configfile=s" => \$configfile );
 
         if(!defined $configfile) {
-            my $cfmeta = $class->meta->get_attribute('configfile');
+            my $cfmeta = $class->meta->find_attribute_by_name('configfile');
             $configfile = $cfmeta->default if $cfmeta->has_default;
         }
 
diff --git a/t/008_configfromfile.t b/t/008_configfromfile.t
index affcd7e..93700b9 100644
--- a/t/008_configfromfile.t
+++ b/t/008_configfromfile.t
@@ -12,7 +12,7 @@ if ( !eval { require MooseX::ConfigFromFile } )
 }
 else
 {
-    plan tests => 24;
+    plan tests => 25;
 }
 
 {
@@ -150,6 +150,27 @@ else
         '... 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 { DerivedApp::Getopt->new_with_options } 'Can create DerivedApp';
+}
+
 sub app_ok {
     my $app = shift;