Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow plugins being preloaded/embedded in a bundle [rt.cpan.org #77554] #112

Open
atoomic opened this issue Oct 5, 2018 · 8 comments
Open

Comments

@atoomic
Copy link
Collaborator

atoomic commented Oct 5, 2018

Migrated from rt.cpan.org#77554 (status was 'new')

Requestors:

Attachments:

From rehsack@cpan.org on 2012-05-31 09:32:52:

Hi,

during a PoC generating C code for GLib's GDBus-Object infrastructure I
needed some handy routines I wanted to provide via TT plugin - but not
freely usable, because their context is limited to generated code from
the introspection information.

So I created "private" packages in the code generator per language - but
T:P always wanted a file. The attached patch accepts plugins which are
loaded already, too (must derived from T:P).

/Jens

@toddr
Copy link
Collaborator

toddr commented Oct 5, 2018

--- lib/Template/Plugins.pm.orig	2012-01-21 08:19:04.000000000 +0000
+++ lib/Template/Plugins.pm
@@ -189,7 +189,7 @@ sub _load {
         $file =~ s|::|/|g;
         $self->debug("loading $module.pm (PLUGIN_NAME)")
             if $self->{ DEBUG };
-        $ok = eval { require "$file.pm" };
+        $ok = eval { $module->isa("Template::Plugin") or require "$file.pm" };
         $error = $@;
     }
     else {
@@ -203,7 +203,7 @@ sub _load {
             $self->debug("loading $file.pm (PLUGIN_BASE)")
                 if $self->{ DEBUG };
             
-            $ok = eval { require "$file.pm" };
+            $ok = eval { $pkg->isa("Template::Plugin") or require "$file.pm" };
             last unless $@;
             
             $error .= "$@\n" 

@toddr
Copy link
Collaborator

toddr commented Oct 5, 2018

Needs a PR

@toddr
Copy link
Collaborator

toddr commented Oct 8, 2018

@rehsack I opened #196 PR for this. Is there a reason we wouldn't check %INC and only load if it's not there? the @ISA thing makes me nervous even though it looks perfectly safe.

@rehsack
Copy link

rehsack commented Oct 9, 2018

@toddr I think that's pretty clear described in the ticket. It's quite usual to have multiple packages in one file. Checking %INC is superfluous.

@toddr
Copy link
Collaborator

toddr commented Oct 9, 2018

Ok good point. We're still going to want tests before merging this. Thanks!

@rehsack
Copy link

rehsack commented Oct 10, 2018

That's ages ago - can't promise anything. Need to work on my modules either ;)
The test should be easy - maybe you can spend some effort ...

@toddr
Copy link
Collaborator

toddr commented Oct 10, 2018

Sorry, I wasn’t saying you needed to do it. I was saying it needed to be done before we merge

@rehsack
Copy link

rehsack commented Oct 10, 2018

@toddr Understood, no worries. I just could have quoted Tim Bunce who said: "Don't wait for ... coming from me. It won't." - I just wanted to show the red flag before anyone waits for me (implicitly).

atoomic pushed a commit that referenced this issue Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants