Bug #114075 for podlators: podlators includes tests for illegal pod

Fri Apr 29 14:30:22 2016 public [...] khwilliamson.com - Ticket created

Subject:	podlators includes tests for illegal pod
Date:	Fri, 29 Apr 2016 11:35:34 -0600
To:	bug-podlators [...] rt.cpan.org, Ricardo Signes <rjbs [...] manxome.org>
From:	Karl Williamson <public [...] khwilliamson.com>

Nesting L<> is illegal, as this quote from perlpodspec indicates: Authors must not nest L<...> codes. For example, "L<The L<Foo::Bar> man page>" should be treated as an error. Yet, the file t/data/basic.pod includes a nested L<> and expects it to work properly: L<Nested L<http://www.perl.org/>|fooE<sol>bar> We are thinking of changing Pod::Simple to detect this illegal nesting, but doing so causes these tests to fail. The tests should be changed to not use this illegal construct. I could submit a pull request, but I don't know what you would want to test instead.

Fri Apr 29 18:31:44 2016 RRA [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #114075] podlators includes tests for illegal pod
Date:	Fri, 29 Apr 2016 15:31:31 -0700
To:	"karl williamson via RT" <bug-podlators [...] rt.cpan.org>
From:	Russ Allbery <rra [...] cpan.org>

"karl williamson via RT" <bug-podlators@rt.cpan.org> writes: Show quoted text

> Nesting L<> is illegal, as this quote from perlpodspec indicates:

Show quoted text

> Authors must not nest L<...> codes. For example, "L<The L<Foo::Bar> > man page>" should be treated as an error.

Show quoted text

> Yet, the file t/data/basic.pod includes a nested L<> and expects it to > work properly:

Show quoted text

> L<Nested L<http://www.perl.org/>|fooE<sol>bar>

Show quoted text

> We are thinking of changing Pod::Simple to detect this illegal nesting, > but doing so causes these tests to fail.

So, the history of this is that this used to always work in POD documents. But when writing Pod::Simple, the original author declared a few things that had previously worked as no longer permitted in the specification. I'm sympathetic, since I'm not sure that construct makes sense. But I tend to take a very conservative approach to backward compatibility. This is being tested because I'm sure there are real POD documents out there in the wild that do this, since it has always worked in the past. So, this felt a bit like breaking the compatibility contract with POD users, and I added a test to be explicit about maintaining that contract. If you're comfortable just breaking those POD documents, I'm willing to follow along. I just want to be sure that's a considered decision to break backward compatibility and that folks have given some thought to the consequences and how this will impact various projects. (Mostly because the people whose POD documents break are all going to yell at me, and then I'll end up redirecting them to the spec and to Pod::Simple as the code that actually made this change... :P) -- #!/usr/bin/perl -- Russ Allbery, Just Another Perl Hacker $^=q;@!>~|{>krw>yn{u<$$<[~||<Juukn{=,<S~|}<Jwx}qn{<Yn{u<Qjltn{ > 0gFzD gD, 00Fz, 0,,( 0hF 0g)F/=, 0> "L$/GEIFewe{,$/ 0C$~> "@=,m,|,(e 0.), 01,pnn,y{ rw} >;,$0=q,$,,($_=$^)=~y,$/ C-~><@=\n\r,-~$:-u/ #y,d,s,(\$.),$1,gee,print

Fri Apr 29 18:31:44 2016 The RT System itself - Status changed from 'new' to 'open'

Sat Apr 30 00:41:54 2016 public [...] khwilliamson.com - Correspondence added

CC:	Ricardo Signes <rjbs [...] manxome.org>
Subject:	Re: [rt.cpan.org #114075] podlators includes tests for illegal pod
Date:	Fri, 29 Apr 2016 22:41:21 -0600
To:	bug-podlators [...] rt.cpan.org
From:	Karl Williamson <public [...] khwilliamson.com>

On 04/29/2016 04:31 PM, Russ Allbery via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=114075 > > > "karl williamson via RT" <bug-podlators@rt.cpan.org> writes: >

>> Nesting L<> is illegal, as this quote from perlpodspec indicates:

>

>> Authors must not nest L<...> codes. For example, "L<The L<Foo::Bar> >> man page>" should be treated as an error.

>

>> Yet, the file t/data/basic.pod includes a nested L<> and expects it to >> work properly:

>

>> L<Nested L<http://www.perl.org/>|fooE<sol>bar>

>

>> We are thinking of changing Pod::Simple to detect this illegal nesting, >> but doing so causes these tests to fail.

> > So, the history of this is that this used to always work in POD documents. > But when writing Pod::Simple, the original author declared a few things > that had previously worked as no longer permitted in the specification. > > I'm sympathetic, since I'm not sure that construct makes sense. But I > tend to take a very conservative approach to backward compatibility. This > is being tested because I'm sure there are real POD documents out there in > the wild that do this, since it has always worked in the past. So, this > felt a bit like breaking the compatibility contract with POD users, and I > added a test to be explicit about maintaining that contract.

Thanks for your response. I was unaware of the history. I'm thinking what you mean when you say that it used to work is that it used to display well on non-html outputs. But it could never have "worked" on the only output type (AFAIK) where one could actually follow the link to another page. This is because html <a> constructs have never nested. And my experiments showed that the output does currently look reasonable on non-html text, but not so reasonable on html text because the underlining of a link is not positioned properly for some nested inputs. Show quoted text

> > If you're comfortable just breaking those POD documents, I'm willing to > follow along. I just want to be sure that's a considered decision to > break backward compatibility and that folks have given some thought to the > consequences and how this will impact various projects. (Mostly because > the people whose POD documents break are all going to yell at me, and then > I'll end up redirecting them to the spec and to Pod::Simple as the code > that actually made this change... :P) >

I did this: http://grep.cpan.me/?q=L%3C%2B[^%3E]*L%3C but have not had a chance to go through the results. Some of them on the first page look like mistakes. I myself am not comfortable making a decision on this. I have cc'd rjbs (as still the king) for his reaction. I imagine there should be public discussion about the potential change.

Sat Apr 30 02:18:11 2016 RRA [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #114075] podlators includes tests for illegal pod
Date:	Fri, 29 Apr 2016 23:18:00 -0700
To:	"karl williamson via RT" <bug-podlators [...] rt.cpan.org>
From:	Russ Allbery <rra [...] cpan.org>

"karl williamson via RT" <bug-podlators@rt.cpan.org> writes: Show quoted text

> Thanks for your response. I was unaware of the history. I'm thinking > what you mean when you say that it used to work is that it used to > display well on non-html outputs. But it could never have "worked" on > the only output type (AFAIK) where one could actually follow the link to > another page. This is because html <a> constructs have never nested. > And my experiments showed that the output does currently look reasonable > on non-html text, but not so reasonable on html text because the > underlining of a link is not positioned properly for some nested inputs.

Yup, exactly. It used to be that roff and text were the "typical" output formats and HTML was somewhat less common, except on Windows. These days, HTML is probably the main way most people read POD, and the roff and text output, while important, aren't quite as central. Given that, what you propose makes sense to me, since it's never worked well with a major output format that's growing more important. I'm mostly just warning that there are probably people who don't ever generate HTML and have never noticed a problem whose documents will stop generating until they fix it. Many of them will probably view this as a feature (oh, hey, a bug I never realized, let me fix that), but a few people will probably be annoyed. (Any change in POD seems to annoy *someone*. The perils of having had a stable document format for as long as Perl has had one, I guess.) -- #!/usr/bin/perl -- Russ Allbery, Just Another Perl Hacker $^=q;@!>~|{>krw>yn{u<$$<[~||<Juukn{=,<S~|}<Jwx}qn{<Yn{u<Qjltn{ > 0gFzD gD, 00Fz, 0,,( 0hF 0g)F/=, 0> "L$/GEIFewe{,$/ 0C$~> "@=,m,|,(e 0.), 01,pnn,y{ rw} >;,$0=q,$,,($_=$^)=~y,$/ C-~><@=\n\r,-~$:-u/ #y,d,s,(\$.),$1,gee,print

Sat Apr 30 07:52:42 2016 SREZIC [...] cpan.org - Correspondence added

RT-Send-CC:

RRA [...] cpan.org, rjbs [...] manxome.org

On 2016-04-30 00:41:54, public@khwilliamson.com wrote: Show quoted text

> On 04/29/2016 04:31 PM, Russ Allbery via RT wrote:

> > <URL: https://rt.cpan.org/Ticket/Display.html?id=114075 > > > > > "karl williamson via RT" <bug-podlators@rt.cpan.org> writes: > >

> >> Nesting L<> is illegal, as this quote from perlpodspec indicates:

> >

> >> Authors must not nest L<...> codes. For example, "L<The L<Foo::Bar> > >> man page>" should be treated as an error.

> >

> >> Yet, the file t/data/basic.pod includes a nested L<> and expects it to > >> work properly:

> >

> >> L<Nested L<http://www.perl.org/>|fooE<sol>bar>

> >

> >> We are thinking of changing Pod::Simple to detect this illegal nesting, > >> but doing so causes these tests to fail.

> > > > So, the history of this is that this used to always work in POD documents. > > But when writing Pod::Simple, the original author declared a few things > > that had previously worked as no longer permitted in the specification. > > > > I'm sympathetic, since I'm not sure that construct makes sense. But I > > tend to take a very conservative approach to backward compatibility. This > > is being tested because I'm sure there are real POD documents out there in > > the wild that do this, since it has always worked in the past. So, this > > felt a bit like breaking the compatibility contract with POD users, and I > > added a test to be explicit about maintaining that contract.

> > Thanks for your response. I was unaware of the history. I'm thinking > what you mean when you say that it used to work is that it used to > display well on non-html outputs. But it could never have "worked" on > the only output type (AFAIK) where one could actually follow the link to > another page.

Graphical pod viewers like Tk::Pod exist and may follow hyperlinks (I guess the other graphical toolkits like Gtk2, Prima, Wx... also have their pod viewers). Actually tkpod can handle nested L<> in an interesting way: clicking on the nested part of the sample link in t/data/basic.pod would open both links simultaneously, that is, the Pod page AND open a browser window. Show quoted text

> This is because html <a> constructs have never nested. > And my experiments showed that the output does currently look reasonable > on non-html text, but not so reasonable on html text because the > underlining of a link is not positioned properly for some nested inputs.

> > > > If you're comfortable just breaking those POD documents, I'm willing to > > follow along. I just want to be sure that's a considered decision to > > break backward compatibility and that folks have given some thought to the > > consequences and how this will impact various projects. (Mostly because > > the people whose POD documents break are all going to yell at me, and then > > I'll end up redirecting them to the spec and to Pod::Simple as the code > > that actually made this change... :P) > >

> > I did this: > > http://grep.cpan.me/?q=L%3C%2B[^%3E]*L%3C > > but have not had a chance to go through the results. Some of them on > the first page look like mistakes. > > I myself am not comfortable making a decision on this. I have cc'd rjbs > (as still the king) for his reaction. I imagine there should be public > discussion about the potential change. >

Mon May 02 23:33:12 2016 public [...] khwilliamson.com - Correspondence added

CC:	Ricardo Signes <rjbs [...] manxome.org>, "David E. Wheeler" <david [...] justatheory.com>
Subject:	Re: [rt.cpan.org #114075] podlators includes tests for illegal pod
Date:	Mon, 2 May 2016 21:32:41 -0600
To:	bug-podlators [...] rt.cpan.org
From:	Karl Williamson <public [...] khwilliamson.com>

On 04/30/2016 05:52 AM, Slaven_Rezic via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=114075 > > > On 2016-04-30 00:41:54, public@khwilliamson.com wrote:

>> On 04/29/2016 04:31 PM, Russ Allbery via RT wrote:

>>> <URL: https://rt.cpan.org/Ticket/Display.html?id=114075 > >>> >>> "karl williamson via RT" <bug-podlators@rt.cpan.org> writes: >>>

>>>> Nesting L<> is illegal, as this quote from perlpodspec indicates:

>>>

>>>> Authors must not nest L<...> codes. For example, "L<The L<Foo::Bar> >>>> man page>" should be treated as an error.

>>>

>>>> Yet, the file t/data/basic.pod includes a nested L<> and expects it to >>>> work properly:

>>>

>>>> L<Nested L<http://www.perl.org/>|fooE<sol>bar>

>>>

>>>> We are thinking of changing Pod::Simple to detect this illegal nesting, >>>> but doing so causes these tests to fail.

>>> >>> So, the history of this is that this used to always work in POD documents. >>> But when writing Pod::Simple, the original author declared a few things >>> that had previously worked as no longer permitted in the specification. >>> >>> I'm sympathetic, since I'm not sure that construct makes sense. But I >>> tend to take a very conservative approach to backward compatibility. This >>> is being tested because I'm sure there are real POD documents out there in >>> the wild that do this, since it has always worked in the past. So, this >>> felt a bit like breaking the compatibility contract with POD users, and I >>> added a test to be explicit about maintaining that contract.

>> >> Thanks for your response. I was unaware of the history. I'm thinking >> what you mean when you say that it used to work is that it used to >> display well on non-html outputs. But it could never have "worked" on >> the only output type (AFAIK) where one could actually follow the link to >> another page.

> > Graphical pod viewers like Tk::Pod exist and may follow hyperlinks (I guess the other graphical toolkits like Gtk2, Prima, Wx... also have their pod viewers). > > Actually tkpod can handle nested L<> in an interesting way: clicking on the nested part of the sample link in t/data/basic.pod would open both links simultaneously, that is, the Pod page AND open a browser window.

That's interesting. Since you don't say so, I presume this doesn't mean you object to the change. But if someone does object based on this, I'll say that it's quite unlikely that a pod author would envision this behavior, and the idea of throwing up multiple tabs is poor user interaction, as the extra ones are likely to go unnoticed. Nor does it change the fact that this doesn't work on the main link-following output type: html. Show quoted text

>

>> This is because html <a> constructs have never nested. >> And my experiments showed that the output does currently look reasonable >> on non-html text, but not so reasonable on html text because the >> underlining of a link is not positioned properly for some nested inputs.

>>> >>> If you're comfortable just breaking those POD documents, I'm willing to >>> follow along. I just want to be sure that's a considered decision to >>> break backward compatibility and that folks have given some thought to the >>> consequences and how this will impact various projects. (Mostly because >>> the people whose POD documents break are all going to yell at me, and then >>> I'll end up redirecting them to the spec and to Pod::Simple as the code >>> that actually made this change... :P)

Also, by keeping the current behavior, we are misleading the authors into thinking their pod works, when it doesn't. You're right that some will complain, and if they complain to you, as I'm sure some will, you can redirect them as you said. I've been in similar positions, where my code was the immediate face to the public, and so would draw blame for upstream problems/changes. Show quoted text

>>>

>> >> I did this: >> >> http://grep.cpan.me/?q=L%3C%2B[^%3E]*L%3C >> >> but have not had a chance to go through the results. Some of them on >> the first page look like mistakes.

I looked through the results, and there are some that are clear oversights, where the author, unless demented, should thank us for finding these bugs. And there are cases where some of these appear to be deliberate. There were two main places where this occurred, so it's not like a lot of cpan would be affected: 1) Several in Moose 2) However, mostly these occur in files named Sec2.pm. I presume that authors of various distributions chose to copy that file from a source distribution instead of properly requiring it. 3) just a few other isolated instances. Show quoted text

>> >> I myself am not comfortable making a decision on this. I have cc'd rjbs >> (as still the king) for his reaction. I imagine there should be public >> discussion about the potential change.

David Wheeler, in response to a private email I sent (before I realized about the podlators issue) thinks this seems sensible. So, I'm thinking it would be good if you go ahead and change or remove that test of nested L<>. Show quoted text

>>

> > > >

Tue May 03 01:34:41 2016 david [...] justatheory.com - Correspondence added

CC:	bug-podlators [...] rt.cpan.org, Ricardo Signes <rjbs [...] manxome.org>
Subject:	Re: [rt.cpan.org #114075] podlators includes tests for illegal pod
Date:	Mon, 2 May 2016 22:34:25 -0700
To:	Karl Williamson <public [...] khwilliamson.com>
From:	"David E. Wheeler" <david [...] justatheory.com>

On May 2, 2016, at 8:32 PM, Karl Williamson <public@khwilliamson.com> wrote: Show quoted text

>>> http://grep.cpan.me/?q=L%3C%2B[^%3E]*L%3C >>> >>> but have not had a chance to go through the results. Some of them on >>> the first page look like mistakes.

> > I looked through the results, and there are some that are clear oversights, where the author, unless demented, should thank us for finding these bugs. And there are cases where some of these appear to be deliberate. There were two main places where this occurred, so it's not like a lot of cpan would be affected: > > 1) Several in Moose > 2) However, mostly these occur in files named Sec2.pm. I presume that authors of various distributions chose to copy that file from a source distribution instead of properly requiring it. > 3) just a few other isolated instances.

Might want to file bugs against them, then, in advance of this change, yes? Show quoted text

>>> >>> I myself am not comfortable making a decision on this. I have cc'd rjbs >>> (as still the king) for his reaction. I imagine there should be public >>> discussion about the potential change.

> > David Wheeler, in response to a private email I sent (before I realized about the podlators issue) thinks this seems sensible.

Can’t hurt to ping pod-people to collect consensus. People do get a bit tetchy when new warnings and errors start appearing, IME. Best to get some consensus in advance, and warn those you know will be hit by it (i.e., the modules from the search above). Show quoted text

> So, I'm thinking it would be good if you go ahead and change or remove that test of nested L<>.

+1 from me! Best, David

Download smime.p7s
application/pkcs7-signature 4k

Message body not shown because it is not plain text.

Wed May 04 15:20:01 2016 public [...] khwilliamson.com - Correspondence added

CC:	bug-podlators [...] rt.cpan.org, Ricardo Signes <rjbs [...] manxome.org>
Subject:	Re: [rt.cpan.org #114075] podlators includes tests for illegal pod
Date:	Wed, 4 May 2016 13:18:22 -0600
To:	"David E. Wheeler" <david [...] justatheory.com>
From:	Karl Williamson <public [...] khwilliamson.com>

On 05/02/2016 11:34 PM, David E. Wheeler wrote: Show quoted text

> Might want to file bugs against them, then, in advance of this change, yes?

Done

Thu May 05 14:13:56 2016 public [...] khwilliamson.com - Correspondence added

CC:	bug-podlators [...] rt.cpan.org, Ricardo Signes <rjbs [...] manxome.org>
Subject:	Re: [rt.cpan.org #114075] podlators includes tests for illegal pod
Date:	Thu, 5 May 2016 12:13:22 -0600
To:	"David E. Wheeler" <david [...] justatheory.com>
From:	Karl Williamson <public [...] khwilliamson.com>

On 05/02/2016 11:34 PM, David E. Wheeler wrote: Show quoted text

> Can’t hurt to ping pod-people to collect consensus. People do get a bit tetchy when new warnings and errors start appearing, IME. Best to get some consensus in advance, and warn those you know will be hit by it (i.e., the modules from the search above). >

>> >So, I'm thinking it would be good if you go ahead and change or remove that test of nested L<>.

> +1 from me!

I forgot that I had already posted to pod-people. So that's been done.

Sat May 07 22:41:35 2016 RRA [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #114075] podlators includes tests for illegal pod
Date:	Sat, 07 May 2016 19:41:22 -0700
To:	"karl williamson via RT" <bug-podlators [...] rt.cpan.org>
From:	Russ Allbery <rra [...] cpan.org>

"karl williamson via RT" <bug-podlators@rt.cpan.org> writes: Show quoted text

> So, I'm thinking it would be good if you go ahead and change or remove > that test of nested L<>.

Yup, sounds good to me. Done and pushed to Github, and will be in the next release. If you're about to do a release and I haven't gotten a new release out yet, feel free to ping me and I'll push one out. (Hoping to add a few more fixes rather than making a release with just that change.) -- #!/usr/bin/perl -- Russ Allbery, Just Another Perl Hacker $^=q;@!>~|{>krw>yn{u<$$<[~||<Juukn{=,<S~|}<Jwx}qn{<Yn{u<Qjltn{ > 0gFzD gD, 00Fz, 0,,( 0hF 0g)F/=, 0> "L$/GEIFewe{,$/ 0C$~> "@=,m,|,(e 0.), 01,pnn,y{ rw} >;,$0=q,$,,($_=$^)=~y,$/ C-~><@=\n\r,-~$:-u/ #y,d,s,(\$.),$1,gee,print

Mon May 09 14:38:43 2016 public [...] khwilliamson.com - Correspondence added

Subject:	Re: [rt.cpan.org #114075] podlators includes tests for illegal pod
Date:	Mon, 9 May 2016 12:38:13 -0600
To:	bug-podlators [...] rt.cpan.org
From:	Karl Williamson <public [...] khwilliamson.com>

On 05/07/2016 08:41 PM, Russ Allbery via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=114075 > > > "karl williamson via RT" <bug-podlators@rt.cpan.org> writes: >

>> So, I'm thinking it would be good if you go ahead and change or remove >> that test of nested L<>.

> > Yup, sounds good to me. Done and pushed to Github, and will be in the > next release. If you're about to do a release and I haven't gotten a new > release out yet, feel free to ping me and I'll push one out. (Hoping to > add a few more fixes rather than making a release with just that change.) >

Thank you. It seems eminently reasonable to wait. But be aware that we're likely to want this fairly soon. (I don't know what other issues there might currently be that you'd like to take care of, nor how long you want to soak a trial release.) Just now, someone asked me if this could get into 5.25.1, due in 11 days. The answer is no, as there are still several dominoes that have to fall, but people will want it for 5.25.2. This is part of a chain that has been in (excruciatingly slow) progress for more than 5 years, and been worked on in previous QAH's. Only now are we getting close to seeing what all the blocking things are, which is why podlators came up now and not earlier.

Sat Jun 11 14:07:11 2016 public [...] khwilliamson.com - Correspondence added

Subject:	Re: [rt.cpan.org #114075] podlators includes tests for illegal pod
Date:	Sat, 11 Jun 2016 12:06:58 -0600
To:	bug-podlators [...] rt.cpan.org
From:	Karl Williamson <public [...] khwilliamson.com>

On 05/07/2016 08:41 PM, Russ Allbery via RT wrote: Show quoted text

> <URL: https://rt.cpan.org/Ticket/Display.html?id=114075 > > > "karl williamson via RT" <bug-podlators@rt.cpan.org> writes: >

>> So, I'm thinking it would be good if you go ahead and change or remove >> that test of nested L<>.

> > Yup, sounds good to me. Done and pushed to Github, and will be in the > next release. If you're about to do a release and I haven't gotten a new > release out yet, feel free to ping me and I'll push one out. (Hoping to > add a few more fixes rather than making a release with just that change.) >

Well the Pod::Simple stuff has gotten pushed down on my stack, so it will be a month or two before I get to it.

Sun Sep 25 15:43:26 2016 RRA [...] cpan.org - Fixed in 4.08 added

Sun Sep 25 15:43:26 2016 RRA [...] cpan.org - Correspondence added

podlators 4.08 removes this test.

Sun Sep 25 15:43:37 2016 RRA [...] cpan.org - Status changed from 'open' to 'resolved'

Sun Sep 25 15:43:42 2016 RRA [...] cpan.org - Taken