Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MooX::Options::Role _options_fix_argv strips leading and trailing quotes from parameter values #68

Open
jmccarv opened this issue Dec 4, 2018 · 2 comments

Comments

@jmccarv
Copy link

jmccarv commented Dec 4, 2018

This bit of code in the _options_fix_argv routine of MooX::Options::Role strips leading and trailing quotes:

#remove the quoted if exist to chain
$_[0] =~ s/^['"]|['"]$//gx;

This is a problem when you want to preserve those quotes. A small example:

#!/usr/bin/env perl

package Test;

use Moo;
use MooX::Options;

option test => (
    is         => 'ro',
    short      => 't',
    format     => 's',
);  

1;

package main;

my $t = Test->new_with_options;
print $t->test."\n";

Run the above:

./test.pl -t "This is 'a test'"
This is 'a test

When I would expect to see

This is 'a test'

as its output.

@rehsack
Copy link
Collaborator

rehsack commented Dec 5, 2018

Valid point. I think this should probably improved - 'cause it looks like a Windows only solution.

@jmccarv
Copy link
Author

jmccarv commented Dec 5, 2018

I tried commenting that line out and it causes some test failures. It looks like it's also related to the autorange feature.

This test is one of two that fail (from t/base.st line 626):

   {
        local @ARGV = ( '-r=1', '-r="2,3"', '-r=4' );
        my $t = rg_str_short->new_with_options();
        is_deeply( $t->range_str, [ '1', "2,3", '4' ], 'str7 req is ok' );
    }

Maybe the code that strips quotes should only run when autorange is set? something like this:

--- a/lib/MooX/Options/Role.pm
+++ b/lib/MooX/Options/Role.pm
@@ -174,9 +174,9 @@ sub _options_fix_argv {
             my $autorange = $option_data->{$original_long_option}{autorange};
             my $argv_processor = sub {
 
-                #remove the quoted if exist to chain
-                $_[0] =~ s/^['"]|['"]$//gx;
                 if ($autorange) {
+                    #remove the quoted if exist to chain
+                    $_[0] =~ s/^['"]|['"]$//gx;
                     push @new_argv,
                         map { $arg_name => $_ } _expand_autorange( $_[0] );
                 }

Making that change fixes the problem I was having and passes a 'make test'. But I admit I don't really know why that code was there so my change might introduce other problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants