From: Stevan Little <stevan.little@iinteractive.com>
Date: Thu, 10 Jul 2008 14:10:27 +0000 (+0000)
Subject: 0.14
X-Git-Tag: 0_15~2
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?a=commitdiff_plain;h=630657d529277b7fb600febf001d8667d8e85184;p=gitmo%2FMooseX-Getopt.git

0.14
---

diff --git a/ChangeLog b/ChangeLog
index bcf6126..fc6358b 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,8 +1,18 @@
 Revision history for Perl extension MooseX-Getopt
 
-0.14 ???
+0.14 Thurs. July 10, 2008
 	* MooseX::Getopt::OptionTypeMap
 	  - Change 'Float' which doesn't exist to 'Num' which does (perigrin)
+	 
+	* MooseX::Getopt
+	  - removed the default handling with Getopt::Long::Descriptive
+	    as it would override constructor parameters and that was 
+	    bad. Besides, Moose will just DWIM on this anyway. (stevan)
+	    - added test for it (stevan)
+
+    * t/
+      - fixed Getopt::Long::Descriptive specific tests to only run 
+        if Getopt::Long::Descriptive is there (stevan)
 
 0.13 Saturday, May 24, 2008
     * MooseX::Getopt
diff --git a/MANIFEST b/MANIFEST
index 1c258b8..6e004d2 100644
--- a/MANIFEST
+++ b/MANIFEST
@@ -30,5 +30,7 @@ t/005_strict.t
 t/006_metaclass_traits.t
 t/007_nogetopt_trait.t
 t/008_configfromfile.t
+t/009_gld_and_explicit_options.t
+t/100_gld_default_bug.t
 t/pod.t
 t/pod_coverage.t
diff --git a/lib/MooseX/Getopt.pm b/lib/MooseX/Getopt.pm
index 0455723..85747e5 100644
--- a/lib/MooseX/Getopt.pm
+++ b/lib/MooseX/Getopt.pm
@@ -11,7 +11,7 @@ use Carp ();
 use Getopt::Long (); # GLD uses it anyway, doesn't hurt
 use constant HAVE_GLD => not not eval { require Getopt::Long::Descriptive };
 
-our $VERSION   = '0.13';
+our $VERSION   = '0.14';
 our $AUTHORITY = 'cpan:STEVAN';
 
 has ARGV       => (is => 'rw', isa => 'ArrayRef', metaclass => "NoGetopt");
@@ -137,7 +137,13 @@ sub _gld_spec {
             $opt->{doc} || ' ', # FIXME new GLD shouldn't need this hack
             {
                 ( ( $opt->{required} && !exists($constructor_params->{$opt->{init_arg}}) ) ? (required => $opt->{required}) : () ),
-                ( exists $opt->{default}  ? (default  => $opt->{default})  : () ),
+                # NOTE:
+                # remove this 'feature' because it didn't work 
+                # all the time, and so is better to not bother
+                # since Moose will handle the defaults just 
+                # fine anyway.
+                # - SL
+                #( exists $opt->{default}  ? (default  => $opt->{default})  : () ),
             },
         ];
 
@@ -199,7 +205,15 @@ sub _attrs_to_options {
             init_arg   => $attr->init_arg,
             opt_string => $opt_string,
             required   => $attr->is_required && !$attr->has_default && !$attr->has_builder && !exists $config_from_file->{$attr->name},
-            ( ( $attr->has_default && ( $attr->is_default_a_coderef xor $attr->is_lazy ) ) ? ( default => $attr->default({}) ) : () ),
+            # NOTE:
+            # this "feature" was breaking because 
+            # Getopt::Long::Descriptive would return 
+            # the default value as if it was a command 
+            # line flag, which would then override the
+            # one passed into a constructor.
+            # See 100_gld_default_bug.t for an example
+            # - SL
+            #( ( $attr->has_default && ( $attr->is_default_a_coderef xor $attr->is_lazy ) ) ? ( default => $attr->default({}) ) : () ),
             ( $attr->has_documentation ? ( doc => $attr->documentation ) : () ),
         }
     }
@@ -435,6 +449,8 @@ Stevan Little E<lt>stevan@iinteractive.comE<gt>
 
 Brandon L. Black, E<lt>blblack@gmail.comE<gt>
 
+Yuval Kogman, E<lt>nothingmuch@woobling.orgE<gt>
+
 =head1 CONTRIBUTORS
 
 Ryan D Johnson, E<lt>ryan@innerfence.comE<gt>
diff --git a/t/009_gld_and_explicit_options.t b/t/009_gld_and_explicit_options.t
index 189a0c3..266ff8c 100644
--- a/t/009_gld_and_explicit_options.t
+++ b/t/009_gld_and_explicit_options.t
@@ -3,10 +3,15 @@
 use strict;
 use warnings;
 
-use Test::More tests => 5;
+use Test::More;
 use Test::Exception;
 
-BEGIN { use_ok('MooseX::Getopt') }
+BEGIN { 
+    eval 'use Getopt::Long::Descriptive;';
+    plan skip_all => "Getopt::Long::Descriptive required for this test" if $@;
+    plan tests => 5;
+    use_ok('MooseX::Getopt');
+}
 
 {
     package Testing::Foo;
diff --git a/t/100_gld_default_bug.t b/t/100_gld_default_bug.t
new file mode 100644
index 0000000..4a2b42a
--- /dev/null
+++ b/t/100_gld_default_bug.t
@@ -0,0 +1,48 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+
+use Test::More;
+use Test::Exception;
+
+BEGIN {
+    eval 'use Getopt::Long::Descriptive;';
+    plan skip_all => "Getopt::Long::Descriptive required for this test" if $@;
+    plan tests => 5;    
+    use_ok('MooseX::Getopt');
+}
+
+{
+    package Engine::Foo;
+    use Moose;
+    
+    with 'MooseX::Getopt';
+    
+    has 'nproc' => (
+        metaclass   => 'Getopt',
+        is          => 'ro',
+        isa         => 'Int',
+        default     => sub { 1 },
+        cmd_aliases => 'n',
+    );
+}
+
+@ARGV = ();
+
+{
+    my $foo = Engine::Foo->new_with_options(nproc => 10);
+    isa_ok($foo, 'Engine::Foo');
+
+    is($foo->nproc, 10, '... got the right value (10), not the default (1)');
+}
+
+{
+    my $foo = Engine::Foo->new_with_options();
+    isa_ok($foo, 'Engine::Foo');
+
+    is($foo->nproc, 1, '... got the right value (1), without GLD needing to handle defaults');
+}
+
+
+