Bug #78403 for Monkey-Patch: Providing more info for patcher sub

Mon Jul 16 11:26:12 2012 sharyanto [...] cpan.org - Ticket created

Subject:

Providing more info for patcher sub

Hi Paul, First of all, Monkey::Patch rocks (especially the stacked/restore thing). Thanks for writing it. I'm developing a module Module::Patch which basically groups several patches together for convenient deployment. Sometimes a single patcher sub is patching a bunch of subroutines instead of just one (for example, wrapping all public methods of a class to add logging). The patcher sub wants to know the original name of the subroutine it patched, so it can write a proper log message ("Entering Foo::bar (...) ...", and so on). I've written a patch for Monkey::Patch (it's currently on my laptop at home so I have no access to it now), it basically add an options hashref to patch_*() in the 4th argument. There's an option called supply_named which if enabled, will cause wrapper() to set the 2nd argument to $self->name before calling the patcher subroutine. Though to be honest I don't really like this way of modification (what if the patcher sub needs more info? more args?) So I've also forked Monkey::Patch 0.03 (I should've forked the Monkey- Patch from github, probably will re-fork later), it's called Monkey::Patch::Context (just uploaded to CPAN). It's incompatible with Monkey::Patch in that the first argument given to patcher subroutine is not the original subroutine, but a context hash. The context hash contains the original subroutine in 'orig_sub' key, but also other information 'orig_name' and others in the future. There's also a way to make the context hash patch compatible with Monkey::Patch, via options. I'm consulting you on how best to proceed. I'd like to merge my changes or make Monkey::Patch do what I needed (provide more info to the patcher sub). Thanks for reading this. Regards, Steven

Mon Jul 16 12:11:31 2012 FRODWITH [...] cpan.org - Correspondence added

I think actually your need here (needing to know the name of the subroutine you're patching) is sort of orthogonal to what Monkey::Patch does, particularly since you have to actually supply the name of the method when you call it. I'm a big fan of small, sharp tools that do one thing well, so I'd rather not throw more into Monkey::Patch than what needs to be there. It'd be fairly easy to have your ::Context module depend on Monkey::Patch to do the lifting (rather than forking) if you find it's a generically useful module. Really, though, unless you find yourself writing this wrapper sub all the time, it might not really need its own module. Something like: sub patch_class { my ($class, $method_name, $patcher) = @_; Monkey::Patch::patch_class($class, $method_name, sub { unshift @_, $method_name; goto &$patcher}); }

Mon Jul 16 12:11:32 2012 The RT System itself - Status changed from 'new' to 'open'

Mon Jul 16 22:06:33 2012 sharyanto [...] cpan.org - Correspondence added

On Mon Jul 16 12:11:31 2012, frodwith wrote: Show quoted text

> I think actually your need here (needing to know the name of the > subroutine you're patching) > is sort of orthogonal to what Monkey::Patch does, particularly since > you have to actually > supply the name of the method when you call it. I'm a big fan of > small, sharp tools that do > one thing well, so I'd rather not throw more into Monkey::Patch than > what needs to be there. > > It'd be fairly easy to have your ::Context module depend on > Monkey::Patch to do the lifting > (rather than forking) if you find it's a generically useful module. > > Really, though, unless you find yourself writing this wrapper sub all > the time, it might not > really need its own module. > > Something like: > > sub patch_class { > my ($class, $method_name, $patcher) = @_; > Monkey::Patch::patch_class($class, $method_name, sub { unshift @_, > $method_name; goto > &$patcher}); > }

Agreed. Thanks for your input. Purging my fork Monkey::Patch::Context from CPAN. Regards, Steven

Mon Jul 16 22:06:49 2012 sharyanto [...] cpan.org - Status changed from 'open' to 'rejected'

Fri Aug 03 09:01:04 2012 sharyanto [...] cpan.org - Correspondence added

On Mon Jul 16 22:06:33 2012, SHARYANTO wrote: Show quoted text

> On Mon Jul 16 12:11:31 2012, frodwith wrote:

> > I think actually your need here (needing to know the name of the > > subroutine you're patching) > > is sort of orthogonal to what Monkey::Patch does, particularly since > > you have to actually > > supply the name of the method when you call it. I'm a big fan of > > small, sharp tools that do > > one thing well, so I'd rather not throw more into Monkey::Patch than > > what needs to be there. > > > > It'd be fairly easy to have your ::Context module depend on > > Monkey::Patch to do the lifting > > (rather than forking) if you find it's a generically useful module. > > > > Really, though, unless you find yourself writing this wrapper sub

all Show quoted text

> > the time, it might not > > really need its own module. > > > > Something like: > > > > sub patch_class { > > my ($class, $method_name, $patcher) = @_; > > Monkey::Patch::patch_class($class, $method_name, sub { unshift @_, > > $method_name; goto > > &$patcher}); > > }

> > Agreed. Thanks for your input. > > Purging my fork Monkey::Patch::Context from CPAN. > > Regards, > Steven

Just a FYI that I just released another derivative work: Alt::Monkey::Patch::SHARYANTO. I needed the ability to add/replace/ delete subs in addition to wrapping, so this is a more major change. There is an incompatible change of interface, so I thought I'll just put the context thingy in. Regards, Steven

Fri Aug 03 09:01:05 2012 The RT System itself - Status changed from 'rejected' to 'open'

Fri Aug 03 09:02:38 2012 sharyanto [...] cpan.org - Correspondence added

Don't know why RT still changes the status from rejected to open, when the reply form says 'rejected (unchanged)'. I think it's an RT bug. If this reply doesn't close the ticket (as rejected), please close it. Sorry for the trouble. Regards, Steven

Fri Aug 03 09:02:39 2012 sharyanto [...] cpan.org - Status changed from 'open' to 'rejected'