Bug #77526 for Pod-Weaver-Plugin-EnsureUniqueSections: Quietly depends on position in weaver.ini

Tue May 29 14:50:31 2012 cjm [...] cpan.org - Ticket created

Subject:

Quietly depends on position in weaver.ini

If you put [-EnsureUniqueSections] at the beginning of weaver.ini, it will quietly ignore any duplicate sections you might have. You have to place it at the end of weaver.ini for it to work. At the very least, the documentation needs to mention that. It would be better if you could make that unnecessary, or at least detect that there are sections listed after [-EnsureUniqueSections] and warn that they will not be checked.

Tue May 29 16:46:32 2012 rct [...] thompsonclan.org - Correspondence added

CC:	undisclosed-recipients:;
Subject:	Re: [rt.cpan.org #77526] Quietly depends on position in weaver.ini
Date:	Tue, 29 May 2012 13:45:47 -0700
To:	bug-pod-weaver-plugin-ensureuniquesections [...] rt.cpan.org
From:	"Ryan C. Thompson" <rct [...] thompsonclan.org>

Hmm. I'm not sure why that would happen. The plugin implements the Pod::Weaver::Role::Finalizer role, which is supposed to do its work after weaving is finished. In any case, it's definitely a bug. For now, though, I'll add a warning to the documentation. On Tue 29 May 2012 11:50:31 AM PDT, Christopher J. Madsen via RT wrote: Show quoted text

> Tue May 29 14:50:31 2012: Request 77526 was acted upon. > Transaction: Ticket created by CJM > Queue: Pod-Weaver-Plugin-EnsureUniqueSections > Subject: Quietly depends on position in weaver.ini > Broken in: 0.111220 > Severity: Important > Owner: Nobody > Requestors: cjm@cpan.org > Status: new > Ticket<URL: https://rt.cpan.org/Ticket/Display.html?id=77526> > > > If you put [-EnsureUniqueSections] at the beginning of weaver.ini, it > will quietly ignore any duplicate sections you might have. You have to > place it at the end of weaver.ini for it to work. > > At the very least, the documentation needs to mention that. It would be > better if you could make that unnecessary, or at least detect that there > are sections listed after [-EnsureUniqueSections] and warn that they > will not be checked. >

Tue May 29 16:46:33 2012 The RT System itself - Status changed from 'new' to 'open'

Tue May 29 18:00:43 2012 RTHOMPSON [...] cpan.org - Taken

Tue May 29 18:04:52 2012 RTHOMPSON [...] cpan.org - Correspondence added

Taking a closer look at the code for Pod::Weaver, in particular the Pod::Weaver::weave_document method[1], and noting that EnsureUniqueSections does the Finalizer role, it seems like regardless of order, it should run after all Sections and Transformers. I'm not really sure how to debug this (I suppose I could start by adding a test suite to my dist). [1] http://cpansearch.perl.org/src/RJBS/Pod-Weaver-3.101636/lib/Pod/Weaver.pm

Tue May 29 18:13:56 2012 cjm [...] cpan.org - Correspondence added

I figured out the cause from your remarks. Pod::Weaver::Section::Leftovers (the plugin most likely to produce duplicate sections) is also a Finalizer. If [-EnsureUniqueSections] appears before [Leftovers], then any duplicate sections handled by Leftovers won't be in the document when EnsureUniqueSections runs.

Tue May 29 19:33:25 2012 RTHOMPSON [...] cpan.org - Correspondence added

On Tue May 29 18:13:56 2012, CJM wrote: Show quoted text

> I figured out the cause from your remarks. > Pod::Weaver::Section::Leftovers (the plugin most likely to produce > duplicate sections) is also a Finalizer. If [-EnsureUniqueSections] > appears before [Leftovers], then any duplicate sections handled by > Leftovers won't be in the document when EnsureUniqueSections runs.

Well, that's annoying. I suppose I could cheat by adding

Tue May 29 19:38:11 2012 RTHOMPSON [...] cpan.org - Correspondence added

On Tue May 29 19:33:25 2012, RTHOMPSON wrote: Show quoted text

> On Tue May 29 18:13:56 2012, CJM wrote:

> > I figured out the cause from your remarks. > > Pod::Weaver::Section::Leftovers (the plugin most likely to produce > > duplicate sections) is also a Finalizer. If [-EnsureUniqueSections] > > appears before [Leftovers], then any duplicate sections handled by > > Leftovers won't be in the document when EnsureUniqueSections runs.

Well, that's annoying. I suppose I could be evil and add a prepare_input method that modifies the weaver object by putting the EnsureUniqueSections at the end of the plugin list. That seems a bit abusive, but at the same time I think it is ok because EnsureUniqueSections never actually modifies the document, so in theory there should be no ill effects (other than making a typo and silently blowing away the plugin list entirely).

Tue May 29 20:16:24 2012 RTHOMPSON [...] cpan.org - Correspondence added

On Tue May 29 19:38:11 2012, RTHOMPSON wrote: Show quoted text

> Well, that's annoying. I suppose I could be evil and add a prepare_input > method that modifies the weaver object by putting the > EnsureUniqueSections at the end of the plugin list. That seems a bit > abusive, but at the same time I think it is ok because > EnsureUniqueSections never actually modifies the document, so in theory > there should be no ill effects (other than making a typo and silently > blowing away the plugin list entirely).

Ok, I just did this. I've attahed a tarball of a trial version with the fix. I'd appreciate it if you had the time to test it.

Subject:

Pod-Weaver-Plugin-EnsureUniqueSections-0.121510-TRIAL.tar.gz

Download Pod-Weaver-Plugin-EnsureUniqueSections-0.121510-TRIAL.tar.gz
application/x-gzip 18.4k

Message body not shown because it is not plain text.

Tue May 29 20:28:19 2012 cjm [...] cpan.org - Correspondence added

On Tue, May 29, 2012 7:16:24 PM, RTHOMPSON wrote: Show quoted text

> Ok, I just did this. I've attahed a tarball of a trial version with the > fix. I'd appreciate it if you had the time to test it.

It's always best to try running code before releasing it. :-) Can't modify non-lvalue subroutine call at Pod/Weaver/Plugin/EnsureUniqueSections.pm line 51. However, my $plugins = $self->weaver->plugins; @$plugins = ((grep { $_ != $self } @$plugins), $self); works just fine. Sometimes autoboxing is overkill.

Tue May 29 20:34:36 2012 RTHOMPSON [...] cpan.org - Correspondence added

On Tue May 29 20:28:19 2012, CJM wrote: Show quoted text

> On Tue, May 29, 2012 7:16:24 PM, RTHOMPSON wrote:

> > Ok, I just did this. I've attahed a tarball of a trial version with the > > fix. I'd appreciate it if you had the time to test it.

> > It's always best to try running code before releasing it. :-) > > Can't modify non-lvalue subroutine call at > Pod/Weaver/Plugin/EnsureUniqueSections.pm line 51. > > However, > > my $plugins = $self->weaver->plugins; > @$plugins = ((grep { $_ != $self } @$plugins), $self); > > works just fine. Sometimes autoboxing is overkill.

Yes, testing is good. But I'm not a a place right now where I can easily run it. Thanks for the working code. I'll certainly test it before doing a real release. :)

Tue May 29 23:13:20 2012 RTHOMPSON [...] cpan.org - Correspondence added

On Tue May 29 20:34:36 2012, RTHOMPSON wrote: Show quoted text

> On Tue May 29 20:28:19 2012, CJM wrote:

> > On Tue, May 29, 2012 7:16:24 PM, RTHOMPSON wrote:

> > > Ok, I just did this. I've attahed a tarball of a trial version

with the Show quoted text

> > > fix. I'd appreciate it if you had the time to test it.

> > > > It's always best to try running code before releasing it. :-) > > > > Can't modify non-lvalue subroutine call at > > Pod/Weaver/Plugin/EnsureUniqueSections.pm line 51. > > > > However, > > > > my $plugins = $self->weaver->plugins; > > @$plugins = ((grep { $_ != $self } @$plugins), $self); > > > > works just fine. Sometimes autoboxing is overkill.

> > Yes, testing is good. But I'm not a a place right now where I can easily > run it. Thanks for the working code. I'll certainly test it before doing > a real release. :)

In any case, do you think reshuffling the plugin list is the right solution? Or can you think of a better fix?

Fri Jun 01 19:17:26 2012 cjm [...] cpan.org - Correspondence added

On Tue, May 29, 2012 10:13:20 PM, RTHOMPSON wrote: Show quoted text

> In any case, do you think reshuffling the plugin list is the right > solution? Or can you think of a better fix?

I'm not really the right person to ask. I just started working with Pod::Weaver a few days ago. It's not a very nice solution, but it should work. I pointed this ticket out to RJBS on IRC, but I don't know if he had time to look at it.

Sun Jun 03 15:28:52 2012 RTHOMPSON [...] cpan.org - Correspondence added

Ok, I just pushed the updated version of this dist, with the fix for this bug as well as an actual test suite that tests for it. I'm marking this resolved.

Sun Jun 03 15:28:53 2012 RTHOMPSON [...] cpan.org - Status changed from 'open' to 'resolved'