Skip Menu |

This queue is for tickets about the MooX-Cmd CPAN distribution.

Report information
The Basics
Id: 91481
Status: resolved
Priority: 0/
Queue: MooX-Cmd

People
Owner: Nobody in particular
Requestors: djerius [...] cpan.org
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 0.008
Fixed in: 0.009



Subject: command line option values which contain spaces are corrupted
I'm using MooX::Cmd (0.008) and MooX::Options 4.005. If a command line option value contains spaces, the content up to the first space is assigned to the option, and the rest is left on the command line for further processing. Example code: package MyApp; use Moo; use MooX qw[ Cmd Options ]; option value => ( short => 'v', is => 'ro', format => 's', ); sub execute { my $self = shift; print "value = ", $self->value, "\n"; } package main; MyApp->new_with_cmd; And the tests: % perl topt2 --value '3 4 5' value = 3 % perl topt2 --value '3 -4 -5' Unknown option: 4 Thanks, Diab
On Tue Dec 17 14:59:19 2013, DJERIUS wrote: Show quoted text
> I'm using MooX::Cmd (0.008) and MooX::Options 4.005. > > If a command line option value contains spaces, the content up to the > first space is assigned to the option, and the rest is left on the > command line for further processing. Example code:
I thought I might send in a patch, but I'm afraid there are assumptions made in the code which I don't understand. Here's the code in MooX::Cmd::Role which seems to be at the root of the symptoms: 140 my $opts_record = Data::Record->new({ 141 split => qr{\s+}, 142 unless => $RE{quoted}, 143 }); 144 145 my @args = $opts_record->records(join(' ',@ARGV)); Since the (Unix) shell has already stripped the quotes before Perl sees @ARGV, there's nothing to indicate that the spaces in values are special after the join. To be honest, I don't understand why the command line needs to be re-processed; it seems like that's already been done, and I don't see any evidence in the rest of MooX::Cmd which would indicate that something else is rewriting @ARGV in a fashion which would require re-tokenizing. In any case, would Text::ParseWords::shellwords suffice instead of Data::Record? It provides the functionality that's needed in this case and has the benefit of being the Perl core. Thanks, Diab
Subject: Re: [rt.cpan.org #91481] command line option values which contain spaces are corrupted
Date: Wed, 18 Dec 2013 17:27:57 +0100
To: bug-MooX-Cmd [...] rt.cpan.org
From: Jens Rehsack <rehsack [...] gmail.com>
Am 18.12.2013 um 17:22 schrieb Diab Jerius via RT <bug-MooX-Cmd@rt.cpan.org>: Show quoted text
> Queue: MooX-Cmd > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=91481 > > > On Tue Dec 17 14:59:19 2013, DJERIUS wrote:
>> I'm using MooX::Cmd (0.008) and MooX::Options 4.005. >> >> If a command line option value contains spaces, the content up to the >> first space is assigned to the option, and the rest is left on the >> command line for further processing. Example code:
> > I thought I might send in a patch, but I'm afraid there are assumptions made in the code which I don't understand. > > Here's the code in MooX::Cmd::Role which seems to be at the root of the symptoms: > > 140 my $opts_record = Data::Record->new({ > 141 split => qr{\s+}, > 142 unless => $RE{quoted}, > 143 }); > 144 > 145 my @args = $opts_record->records(join(' ',@ARGV)); > > Since the (Unix) shell has already stripped the quotes before Perl sees @ARGV, there's nothing to indicate that the spaces in values are special after the join. > > To be honest, I don't understand why the command line needs to be re-processed; it seems like that's already been done, and I don't see any evidence in the rest of MooX::Cmd which would indicate that something else is rewriting @ARGV in a fashion which would require re-tokenizing. > > In any case, would Text::ParseWords::shellwords suffice instead of Data::Record? It provides the functionality that's needed in this case and has the benefit of being the Perl core.
I will take a deeper look once I feel better (some cold headache now …). I think the code itself is broken there and I will add a test and fix it upon that. Thanks -- Jens Rehsack rehsack@gmail.com
On Wed Dec 18 11:28:11 2013, rehsack@gmail.com wrote: Show quoted text
> I will take a deeper look once I feel better (some cold headache now > …). > > I think the code itself is broken there and I will add a test and fix > it upon that.
Thanks. FWIW, here's a patch which simply fixes (hopefully) the quotation problem: diff --git a/lib/MooX/Cmd/Role.pm b/lib/MooX/Cmd/Role.pm index 1d0e06e..8b58018 100644 --- a/lib/MooX/Cmd/Role.pm +++ b/lib/MooX/Cmd/Role.pm @@ -9,7 +9,7 @@ use Moo::Role; use Carp; use Module::Runtime qw/ use_module /; use Regexp::Common; -use Data::Record; +use Text::ParseWords 'shellwords'; use Module::Pluggable::Object; use List::Util qw/first/; @@ -254,12 +254,8 @@ sub _initialize_from_cmd { my ( $class, %params ) = @_; - my $opts_record = Data::Record->new({ - split => qr{\s+}, - unless => $RE{quoted}, - }); + my @args = shellwords( join ' ', map { quotemeta } @ARGV ); - my @args = $opts_record->records(join(' ',@ARGV)); my (@used_args, $cmd, $cmd_name); my %cmd_create_params = %params; Hope you feel better!
On Wed Dec 18 14:06:36 2013, DJERIUS wrote: Show quoted text
> On Wed Dec 18 11:28:11 2013, rehsack@gmail.com wrote:
Show quoted text
> Thanks. FWIW, here's a patch which simply fixes (hopefully) the > quotation problem: >
[...] Applied with credentials. I would feel better when you could provide a patch (Pull request appreciated ^^) Show quoted text
> Hope you feel better!
Define better ;) Not sick anymore *gg*
On Sat Feb 01 16:19:36 2014, REHSACK wrote: Show quoted text
> On Wed Dec 18 14:06:36 2013, DJERIUS wrote:
> > On Wed Dec 18 11:28:11 2013, rehsack@gmail.com wrote:
>
> > Thanks. FWIW, here's a patch which simply fixes (hopefully) the > > quotation problem: > >
> [...] > > Applied with credentials. I would feel better when you could provide a > patch (Pull request appreciated ^^)
My patch was more of a "please make it stop hurting" rather than a "this will cure the disease" sort of thing. To be honest I didn't think it'd go any further... Point taken, however. Show quoted text
>
> > Hope you feel better!
> > Define better ;) > Not sick anymore *gg*
Well, that's at least an improvement!
On Sat Feb 01 16:19:36 2014, REHSACK wrote: Show quoted text
> On Wed Dec 18 14:06:36 2013, DJERIUS wrote:
> > On Wed Dec 18 11:28:11 2013, rehsack@gmail.com wrote:
>
> > Thanks. FWIW, here's a patch which simply fixes (hopefully) the > > quotation problem: > >
> [...] > > Applied with credentials. I would feel better when you could provide a > patch (Pull request appreciated ^^) >
Any chance of a release? (I just stubbed my toe on this again today) Or, are you waiting for a pull request (I see that you've applied the patch, so I think I'm no longer in the loop...). Thanks, Diab
On Tue Jul 01 17:04:18 2014, DJERIUS wrote: Show quoted text
> On Sat Feb 01 16:19:36 2014, REHSACK wrote:
> > On Wed Dec 18 14:06:36 2013, DJERIUS wrote:
> > > On Wed Dec 18 11:28:11 2013, rehsack@gmail.com wrote:
> >
> > > Thanks. FWIW, here's a patch which simply fixes (hopefully) the > > > quotation problem: > > >
> > [...] > > > > Applied with credentials. I would feel better when you could provide > > a > > patch (Pull request appreciated ^^) > >
> > Any chance of a release? (I just stubbed my toe on this again today) > Or, are you waiting for a pull request (I see that you've applied the > patch, so I think I'm no longer in the loop...).
I'm a bit busy with other tasks. Chance of soon release is low, mid-term for sure. Cheers Jens
Resolved with 0.009 - a test would be nice anyway :)