Skip Menu |

This queue is for tickets about the Parse-RecDescent CPAN distribution.

Report information
The Basics
Id: 49039
Status: resolved
Priority: 0/
Queue: Parse-RecDescent

People
Owner: Nobody in particular
Requestors: castaway [...] desert-island.me.uk
Cc:
AdminCc:

Bug Information
Severity: Critical
Broken in: 1.962.0
Fixed in: (no value)



Subject: Regression: Repetitions using a separator pattern now also return the separator
When parsing a string using repetitions and separators, eg: 'some (repeated, repeated, repeated, repeated) elements', using a grammar like: my $parser = Parse::RecDescent->new( q{ sep: some(?) '(' repeated(s? /,/) ')' 'elements' { $return = $item[3]; } repeated: 'repeated' some: 'some' }); The $return used to just contain the list of 'repeated', since 1.962 it also contains the separator ',' in this case. See attached file for a simple test. This is causing many test failures for SQL-Translator as we use lots of P::RD in that fashion. See recent spate of fail reports: http:// static.cpantesters.org/distro/S/SQL-Translator.html
Subject: separated_repitition.t
#!/usr/bin/perl use strict; use warnings; use Test::More 'no_plan'; use Data::Dumper; use Parse::RecDescent; my $parser = Parse::RecDescent->new( q{ sep: some(?) '(' repeated(s? /,/) ')' 'elements' { $return = $item[3]; } repeated: 'repeated' some: 'some' }); ok($parser, 'Created parser'); my $str = 'some (repeated, repeated, repeated, repeated) elements'; my $result = $parser->sep($str); my $expected = ['repeated', 'repeated', 'repeated', 'repeated']; is_deeply($result, $expected); print Data::Dumper::Dumper($result);
Subject: Re: [rt.cpan.org #49039] Regression: Repetitions using a separator pattern now also return the separator
Date: Wed, 26 Aug 2009 22:58:24 +0200
To: bug-Parse-RecDescent [...] rt.cpan.org
From: Damian Conway <damian [...] conway.org>
Thanks for the bug report, Jess. Show quoted text
> This is causing many test failures for SQL-Translator as we use lots of > P::RD in that fashion. See recent spate of fail reports: http:// > static.cpantesters.org/distro/S/SQL-Translator.html
I tracked down the problem (I think!). The attached beta fixes it. Can you check that it also fixes your SQL-Translator problems? If it does, I'll upload a new release tomorrow. Thanks for the excellent bug report (and extra test, which is now in the test suite). Damian

Message body is not shown because sender requested not to inline it.

Hi Damian, Thanks, that was snappy! It looks like it does fix the bug nicely. I was going to wait to say until I'd figured out what the other bugs in sqlt trunk are, but I haven't got access to my machine today, so if you could upload with that patch, I'll cross my fingers and look at the others later. I've been assured by another in the team that my other errors don't happen for him. Jess On Wed Aug 26 16:59:14 2009, damian@conway.org wrote: Show quoted text
> Thanks for the bug report, Jess. >
> > This is causing many test failures for SQL-Translator as we use
lots of Show quoted text
> > P::RD in that fashion. See recent spate of fail reports: http:// > > static.cpantesters.org/distro/S/SQL-Translator.html
> > I tracked down the problem (I think!). > The attached beta fixes it. > Can you check that it also fixes your SQL-Translator problems? > If it does, I'll upload a new release tomorrow. > > Thanks for the excellent bug report (and extra test, which is now in
the Show quoted text
> test suite). > > Damian
On Wed Aug 26 16:59:14 2009, damian@conway.org wrote: Show quoted text
> Thanks for the bug report, Jess. >
> > This is causing many test failures for SQL-Translator as we use lots of > > P::RD in that fashion. See recent spate of fail reports: http:// > > static.cpantesters.org/distro/S/SQL-Translator.html
> > I tracked down the problem (I think!). > The attached beta fixes it. > Can you check that it also fixes your SQL-Translator problems? > If it does, I'll upload a new release tomorrow. > > Thanks for the excellent bug report (and extra test, which is now in the > test suite). > > Damian
Unfortunately there are more regressions (even though Jess's test is passes). Please find attached yet another test. In short $return = $1 doesn't work anymore (test passes on 1.96.0) Cheers P.S. The test file reentry.t from RT#45262 didn't make it to the last release, it's probably a good idea to keep it around.
#!/usr/bin/perl use strict; use warnings; use Test::More 'no_plan'; use Data::Dumper; use Parse::RecDescent; my $parser = Parse::RecDescent->new(<<'EOG'); { my %ret; } CONFIG : KV_PAIR(s) { return \%ret } KV_PAIR : WORD /\s*=\s*/ MAYBE_QUOTED_WORD { $ret{$item[1]} = $item[3] } MAYBE_QUOTED_WORD: WORD | /'([^']+)'/ { $return = $1 } | /"([^"]+)"/ { $return = $1 } WORD : /\w+/ EOG ok($parser, 'Created parser'); my $str = q|a=1 b="2" c ="33" d= '12'|; my $result = $parser->CONFIG($str); is_deeply($result, { a => 1, b => 2, c => 33, d => 12 } ) || warn Dumper $result;
Subject: Re: [rt.cpan.org #49039] Regression: Repetitions using a separator pattern now also return the separator
Date: Thu, 27 Aug 2009 20:12:08 +0200
To: bug-Parse-RecDescent [...] rt.cpan.org
From: Damian Conway <damian [...] conway.org>
Show quoted text
> Unfortunately there are more regressions (even though Jess's test is > passes). Please find attached yet another test. In short $return = $1 > doesn't work anymore (test passes on 1.96.0)
Thanks for the follow-up, Peter. The attached beta should fix that properly now. If you could test it again, I'll upload ASAP (though it may be a day or two, as I'm scheduled to fly from Europe to Australia tomorrow). Thanks again, Damian PS: The problem was that, in removing all instances of $&, we had added an extra capture around each complete regex, throwing out the capture numbers. This latest alpha reverts that optimization and implements another solution instead.

Message body is not shown because sender requested not to inline it.

On Thu Aug 27 14:12:51 2009, damian@conway.org wrote: Show quoted text
> > Unfortunately there are more regressions (even though Jess's test is > > passes). Please find attached yet another test. In short $return = $1 > > doesn't work anymore (test passes on 1.96.0)
> > Thanks for the follow-up, Peter. > > The attached beta should fix that properly now. If you could test it > again, I'll upload ASAP (though it may be a day or two, as I'm scheduled > to fly from Europe to Australia tomorrow). > > Thanks again, > > Damian > > > PS: The problem was that, in removing all instances of $&, we had added > an extra capture around each complete regex, throwing out the > capture numbers. This latest alpha reverts that optimization and > implements another solution instead.
This does it, at least for SQL::Translator. Please release whenever possible.
Subject: Re: [rt.cpan.org #49039] Regression: Repetitions using a separator pattern now also return the separator
Date: Thu, 27 Aug 2009 21:45:10 +0200
To: bug-Parse-RecDescent [...] rt.cpan.org
From: Damian Conway <damian [...] conway.org>
Show quoted text
> This does it, at least for SQL::Translator. Please release whenever > possible.
Uploading to CPAN now. Damian
On Thu Aug 27 15:45:48 2009, damian@conway.org wrote: Show quoted text
> > This does it, at least for SQL::Translator. Please release whenever > > possible.
> > Uploading to CPAN now. > > Damian
Hi Damian, Somehow a use 5.010 snuck into the separated_repetition.t file, so now the module is uninstallable on 5.8 and lower. Please upload a new release with a fixed test as time permits. (Also there is a print Data::Dumper::Dumper($result) left in the same test file).
Subject: Re: [rt.cpan.org #49039] Regression: Repetitions using a separator pattern now also return the separator
Date: Fri, 28 Aug 2009 13:40:38 +0200
To: bug-Parse-RecDescent [...] rt.cpan.org
From: Damian Conway <damian [...] conway.org>
Hi Peter, Show quoted text
> Somehow a use 5.010  snuck into the separated_repetition.t file, so now > the module is uninstallable on 5.8 and lower. Please upload a new > release with a fixed test as time permits. (Also there is a print > Data::Dumper::Dumper($result) left in the same test file).
Uploaded. Hopefully, this is finally correct. %-) Damian
On Fri Aug 28 07:41:19 2009, damian@conway.org wrote: Show quoted text
> Hi Peter, >
> > Somehow a use 5.010  snuck into the separated_repetition.t file, so now > > the module is uninstallable on 5.8 and lower. Please upload a new > > release with a fixed test as time permits. (Also there is a print > > Data::Dumper::Dumper($result) left in the same test file).
> > Uploaded. Hopefully, this is finally correct. %-) >
Um... Sorry to bother but this doesn't work either. The tarball is somehow double-gzipped, so cpan doesn't know what to do with it. Please re-upload (same version number probably won't work, so please bump again).
Subject: Re: [rt.cpan.org #49039] Regression: Repetitions using a separator pattern now also return the separator
Date: Fri, 28 Aug 2009 18:54:42 +0200
To: bug-Parse-RecDescent [...] rt.cpan.org
From: Damian Conway <damian [...] conway.org>
Show quoted text
> Um... Sorry to bother but this doesn't work either. The tarball is > somehow double-gzipped, so cpan doesn't know what to do with it. Please > re-upload (same version number probably won't work, so please bump again).
Apparently the good elves of the CPAN fixed it. Thank-you, Great Ones! It's up and operational now, according to search.cpan.org. Damian
The ticket history indicates that this issue has been resolved.