rjbs/Sub-Exporter

Sub::Exporter should use its own 'installer' method to install the import sub

Opened this issue · 6 comments

This should be possible:

use Sub::Exporter::ForMethods 'method_installer';
use Sub::Exporter -setup => {
    installer => method_installer,
    ...,
};

...and have a clean namespace. However, this is not the case -- the import sub itself appears to be from outside this package. If the passed &install method was used (or a derivative of it?) instead of _do_import, then the import sub would be installed using Sub::Name::subname, via 'method_installer'.

rjbs commented
  • Karen Etheridge notifications@github.com [2013-11-23T17:25:16]

    This should be possible:

    use Sub::Exporter::ForMethods 'method_installer';
    use Sub::Exporter -setup => {
        installer => method_installer,
        ...,
    };
    

I'm just leaving home. I didn't test this:

  use Sub::Exporter::ForMethods 'method_installer';
  use Sub::Exporter
      { installer => method_install },
      -setup => {
        installer => method_installer,
        ...,
      };

rjbs

yup, that works! thanks!

sadly, Class::Tiny methods aren't installed such that namespace::autoclean leaves them alone, so the place where I needed this can't use it yet. (dagolden/Class-Tiny#12)

Actually, this isn't good after all -- the imported subs are being cleaned from the target namespace. Repro case:

use strict;
use warnings;

{
    package Foo;
    use Sub::Exporter::ForMethods 'method_installer';
    use Sub::Exporter
        { installer => method_installer },
        -setup => {
            installer => method_installer,
            exports => [qw(your_new_sub)],
        };
    sub your_new_sub { print "ohhai" }
}

{
    package Bar;
    Foo->import(qw(your_new_sub));
}

die 'cannot call Bar::your_new_sub' unless Bar->can('your_new_sub');

If the first 'installer' line is commented out, this test passes -- is it
undermining the import itself?

The problem is that the installed subs are wrappers generated by ForMethods. This means that inside the wrapped sub, caller will report Sub::Exporter::ForMethods as the calling package. import subs are on the few things that care about the calling package.

I tried changing the wrapper sub to use goto, but the current behavior is at least partly intentional, so that the wrapper sub shows up in the call stack. With a goto, the installed name of the sub would not show up in a stack trace.

Possibly method_installer needs to accept an option to switch between the behaviors?

rjbs commented

I just pushed 097ceb1, which adds method_goto_installer, which should do the trick.

How do we like it?

I'm gonna say we like it.