Bug #116101 for re-engine-PCRE: [patch] Add support for partial matching

Wed Jul 13 02:02:07 2016 maxwellhaydn [...] gmail.com - Ticket created

Subject:	[patch] Add support for partial matching
Date:	Wed, 13 Jul 2016 00:01:49 -0600
To:	bug-re-engine-PCRE [...] rt.cpan.org
From:	Max Carey <maxwellhaydn [...] gmail.com>

PCRE supports partial matching as documented in pcrepartial(3). The attached patch adds support for partial matching to re::engine::PCRE so you can do use re::engine::PCRE qw(partial); "foo" =~ /foo bar/; # matches no re::engine::PCRE qw(partial); "foo" =~ /foo bar/; # doesn't match I think this is a pretty neat feature because it's one of the few things PCRE can do that Perl regex can't. Let me know if you would prefer a pull request. I tested on Perl 5.14.2 and 5.20.2 with libpcre 7.9, 8.32, and 8.39 (latest stable). I also tried libpcre 5.0 (the first release to support partial matching), but the build fails even without my changes.

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

Wed Jul 13 04:08:50 2016 avar [...] cpan.org - Correspondence added

RT-Send-CC:

cpan [...] audreyt.org

On 2016-07-13T02:02:07-04:00, maxwellhaydn@gmail.com wrote: Show quoted text

> PCRE supports partial matching as documented in pcrepartial(3). The > attached patch adds support for partial matching to re::engine::PCRE so you > can do > > use re::engine::PCRE qw(partial); > > "foo" =~ /foo bar/; # matches > > no re::engine::PCRE qw(partial); > > "foo" =~ /foo bar/; # doesn't match > > I think this is a pretty neat feature because it's one of the few things > PCRE can do that Perl regex can't. > > Let me know if you would prefer a pull request. I tested on Perl 5.14.2 and > 5.20.2 with libpcre 7.9, 8.32, and 8.39 (latest stable). I also tried > libpcre 5.0 (the first release to support partial matching), but the build > fails even without my changes.

This patch looks good to me. The only nitpick I'd have on it is that you could check the pcre_version() at compile time, e.g.: use constant PCRE_VERSION => ...::pcre_version(): And then the check you're doing in the import() routine could be mostly done at compile-time. I.e. if you had a request for "partial" the whole "$major == 8 && $minor < 33" etc. check could already have been done already, but it's just a micro-optimization. I have comaint on this dist, Audrey has maintainership, since neither of us are interested in this distro anymore (at least I'm not) I'd much rather just give you (co-)?maintainership rather than apply your patch and upload it myself. Do you have a PAUSE account so that can be sorted out?

Wed Jul 13 04:08:51 2016 The RT System itself - Status changed from 'new' to 'open'

Wed Jul 13 15:24:10 2016 maxwellhaydn [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #116101] [patch] Add support for partial matching
Date:	Wed, 13 Jul 2016 13:24:00 -0600
To:	bug-re-engine-PCRE [...] rt.cpan.org
From:	Max Carey <maxwellhaydn [...] gmail.com>

On Wed, Jul 13, 2016 at 2:08 AM, AEvar Arnfjord Bjarmason via RT < bug-re-engine-PCRE@rt.cpan.org> wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=116101 > > > The only nitpick I'd have on it is that you could check the pcre_version() > at compile time, e.g.: > > use constant PCRE_VERSION => ...::pcre_version(): > > And then the check you're doing in the import() routine could be mostly > done at compile-time. I.e. if you had a request for "partial" the whole > "$major == 8 && $minor < 33" etc. check could already have been done > already, but it's just a micro-optimization. >

Good idea! No need to parse the same version string on every single import. Show quoted text

> I have comaint on this dist, Audrey has maintainership, since neither of > us are interested in this distro anymore (at least I'm not) I'd much rather > just give you (co-)?maintainership rather than apply your patch and upload > it myself. > > Do you have a PAUSE account so that can be sorted out? >

Just got one: MHC. Thanks for responding so quickly!

Wed Jul 13 16:13:27 2016 avarab [...] gmail.com - Correspondence added

CC:	Audrey Tang <audreyt [...] audreyt.org>, cpan [...] audreyt.org
Subject:	Re: [rt.cpan.org #116101] [patch] Add support for partial matching
Date:	Wed, 13 Jul 2016 22:12:57 +0200
To:	bug-re-engine-PCRE [...] rt.cpan.org
From:	Ævar Arnfjörð Bjarmason <avarab [...] gmail.com>

On Wed, Jul 13, 2016 at 9:24 PM, Max Carey via RT <bug-re-engine-PCRE@rt.cpan.org> wrote: Show quoted text

> Queue: re-engine-PCRE > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=116101 > > > On Wed, Jul 13, 2016 at 2:08 AM, AEvar Arnfjord Bjarmason via RT < > bug-re-engine-PCRE@rt.cpan.org> wrote: >

>> <URL: https://rt.cpan.org/Ticket/Display.html?id=116101 > >> >> The only nitpick I'd have on it is that you could check the pcre_version() >> at compile time, e.g.: >> >> use constant PCRE_VERSION => ...::pcre_version(): >> >> And then the check you're doing in the import() routine could be mostly >> done at compile-time. I.e. if you had a request for "partial" the whole >> "$major == 8 && $minor < 33" etc. check could already have been done >> already, but it's just a micro-optimization. >>

> > Good idea! No need to parse the same version string on every single import. > >

>> I have comaint on this dist, Audrey has maintainership, since neither of >> us are interested in this distro anymore (at least I'm not) I'd much rather >> just give you (co-)?maintainership rather than apply your patch and upload >> it myself. >> >> Do you have a PAUSE account so that can be sorted out? >>

> > Just got one: MHC. Thanks for responding so quickly!

Fantastic, I'm just co-maint so I can't give you permissions, but CC'd Audry who can.