Re: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2013-02-13 Thread David E . Wheeler 
On Mar 2, 2012, at 8:57 AM, David E. Wheeler  wrote:

>> What sort of confirmation do you need?  I agree with the two bulleted items, 
>> if you're looking for that kind of confirmation.
> 
> Just wanted to make sure I correctly understood the consensus. I’ve added 
> those two items to the ticket.
> 
>  https://rt.cpan.org/Ticket/Display.html?id=74389

I've now applied a fix for this issue:

  
https://github.com/theory/pod-simple/commit/a2047c42501c47f8fe8ff610f7f4cf65616ec54e

Thanks,

David

Re: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-03-02 Thread David E. Wheeler 
On Mar 2, 2012, at 6:49 AM, Karl Williamson wrote:

>>> • Pod::Simple::HTML needs to be fixed so that it does not include the 
>>> contents of X<>
>>> • The parser overall should be adjusted to remove superfluous whitespace
>> 
>> FWIW, I could use confirmation on this.
> 
> What sort of confirmation do you need?  I agree with the two bulleted items, 
> if you're looking for that kind of confirmation.

Just wanted to make sure I correctly understood the consensus. I’ve added those 
two items to the ticket.

  https://rt.cpan.org/Ticket/Display.html?id=74389

Best,

David

Re: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-03-02 Thread Karl Williamson 

On 03/02/2012 12:34 AM, David E. Wheeler wrote:

On Jan 29, 2012, at 3:15 PM, David E. Wheeler wrote:


And "NAME" and not "NAME "

It should probably not just become an empty string, but it should be collapse
whitespace around it, so pathological cases like:

=head1 NAME X  THIS X  TUNE
X

...should be "NAME THIS TUNE"

But in the simpler case, I think that "NAME" and not "NAME " is actually likely
to come up.


Okay, so if I follow this thread correctly, the upshot is that:

• Pod::Simple::HTML needs to be fixed so that it does not include the contents of 
X<>
• The parser overall should be adjusted to remove superfluous whitespace


FWIW, I could use confirmation on this.


What sort of confirmation do you need?  I agree with the two bulleted 
items, if you're looking for that kind of confirmation.


Meanwhile, here's a test case showing the original bug with PullParser:

diff --git a/t/pulltitl.t b/t/pulltitl.t
index 22934f5..a846048 100644
--- a/t/pulltitl.t
+++ b/t/pulltitl.t
@@ -7,7 +7,7 @@ BEGIN {

  use strict;
  use Test;
-BEGIN { plan tests =>  116 };
+BEGIN { plan tests =>  117 };

  #use Pod::Simple::Debug (5);

@@ -408,6 +408,13 @@ ok( $t&&  $t->type eq 'start'&&  $t->tagname, 'Document' );
  }

  ###
+print "# Testing a title with an X<>, at line ", __LINE__, "\n";
+my $p = Pod::Simple::PullParser->new;
+$p->set_source( \qq{\n=head1 NAME\nX\n} );
+
+ok $p->get_title(), 'NAME';
+
+###
  ###



That fails with:

not ok 116
# Test 116 got: "NAME Some entry" (t/pulltitl.t at line 415)
# Expected: "NAME"
#  t/pulltitl.t line 415 is: ok $p->get_title(), 'NAME';

So it seems as though X<>  issues my be all over the place, eh?

Best,

David

Re: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-03-01 Thread David E. Wheeler 
On Jan 29, 2012, at 3:15 PM, David E. Wheeler wrote:

>> And "NAME" and not "NAME "
>> 
>> It should probably not just become an empty string, but it should be collapse
>> whitespace around it, so pathological cases like:
>> 
>> =head1 NAME X THIS X TUNE
>> X
>> 
>> ...should be "NAME THIS TUNE"
>> 
>> But in the simpler case, I think that "NAME" and not "NAME " is actually 
>> likely
>> to come up.
> 
> Okay, so if I follow this thread correctly, the upshot is that:
> 
> • Pod::Simple::HTML needs to be fixed so that it does not include the 
> contents of X<>
> • The parser overall should be adjusted to remove superfluous whitespace

FWIW, I could use confirmation on this.

Meanwhile, here's a test case showing the original bug with PullParser:

diff --git a/t/pulltitl.t b/t/pulltitl.t
index 22934f5..a846048 100644
--- a/t/pulltitl.t
+++ b/t/pulltitl.t
@@ -7,7 +7,7 @@ BEGIN {
 
 use strict;
 use Test;
-BEGIN { plan tests => 116 };
+BEGIN { plan tests => 117 };
 
 #use Pod::Simple::Debug (5);
 
@@ -408,6 +408,13 @@ ok( $t && $t->type eq 'start' && $t->tagname, 'Document' );
 }
 
 ###
+print "# Testing a title with an X<>, at line ", __LINE__, "\n";
+my $p = Pod::Simple::PullParser->new;
+$p->set_source( \qq{\n=head1 NAME\nX\n} );
+
+ok $p->get_title(), 'NAME';
+
+###
 ###
 
 

That fails with:

not ok 116
# Test 116 got: "NAME Some entry" (t/pulltitl.t at line 415)
# Expected: "NAME"
#  t/pulltitl.t line 415 is: ok $p->get_title(), 'NAME';

So it seems as though X<> issues my be all over the place, eh?

Best,

David

Re: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-01-29 Thread David E. Wheeler 
On Jan 29, 2012, at 6:29 AM, Ricardo Signes wrote:

> And "NAME" and not "NAME "
> 
> It should probably not just become an empty string, but it should be collapse
> whitespace around it, so pathological cases like:
> 
> =head1 NAME X THIS X TUNE
> X
> 
> ...should be "NAME THIS TUNE"
> 
> But in the simpler case, I think that "NAME" and not "NAME " is actually 
> likely
> to come up.

Okay, so if I follow this thread correctly, the upshot is that:

• Pod::Simple::HTML needs to be fixed so that it does not include the contents 
of X<>
• The parser overall should be adjusted to remove superfluous whitespace

Correct?

David

Re: Fwd: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-01-29 Thread Ricardo Signes 
* Marek Rouchal  [2012-01-29T03:05:50]
> I agree... the content of X<...> is not visible text, so for
> 
> =head1 NAME
> X
> 
> Pod::Simple should return "NAME" for the heading text, not
> "NAME some text"

And "NAME" and not "NAME "

It should probably not just become an empty string, but it should be collapse
whitespace around it, so pathological cases like:

=head1 NAME X THIS X TUNE
X

...should be "NAME THIS TUNE"

But in the simpler case, I think that "NAME" and not "NAME " is actually likely
to come up.

-- 
rjbs


signature.asc
Description: Digital signature

AW: Fwd: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-01-29 Thread Marek Rouchal 
I agree... the content of X<...> is not visible text, so for

=head1 NAME
X

Pod::Simple should return "NAME" for the heading text, not
"NAME some text"

-Marek

-Ursprüngliche Nachricht-
Von: Karl Williamson [mailto:pub...@khwilliamson.com] 
Gesendet: Samstag, 28. Januar 2012 22:09
An: David E. Wheeler
Cc: pod-people@perl.org
Betreff: Re: Fwd: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title
should ignore X<...>

On 01/25/2012 01:12 PM, David E. Wheeler wrote:
>
> Begin forwarded message:
>
>> Subject: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title 
>> should ignore X<...>
>> Date: January 25, 2012 12:10:00 PM PST
>> Reply-To: bug-pod-sim...@rt.cpan.org
>>
>> With Pod::Simple 3.14.
>>
>> If a pod file has index entries after head:
>>
>> =head1 NAME
>> X
>>
>> Pod::Simple::Pullparser get_title expands 'Some entry' in the title.  
>> It seems to me that it should not, and instead should replace it with 
>> an empty string.  This  behaviour is also hinted in the pod
documentation.
>>
>> A look at the code in git makes me think that the same issue is still 
>> there (l. 363 and following all the text are used, X<>  text is not 
>> discarded).
>>
>> This kind of construct is not rare, for instance it is present in the 
>> main perl pod documentation.
>
> Opinions?
>
> Thanks,
>
> David
>
>

My opinion is that it should replace it with an empty string.

Re: Fwd: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-01-28 Thread Karl Williamson 

On 01/25/2012 01:12 PM, David E. Wheeler wrote:


Begin forwarded message:


Subject: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore 
X<...>
Date: January 25, 2012 12:10:00 PM PST
Reply-To: bug-pod-sim...@rt.cpan.org

With Pod::Simple 3.14.

If a pod file has index entries after head:

=head1 NAME
X

Pod::Simple::Pullparser get_title expands 'Some entry' in the title.  It
seems to me that it should not, and instead should replace it with an
empty string.  This  behaviour is also hinted in the pod documentation.

A look at the code in git makes me think that the same issue is still
there (l. 363 and following all the text are used, X<>  text is not
discarded).

This kind of construct is not rare, for instance it is present in the
main perl pod documentation.


Opinions?

Thanks,

David




My opinion is that it should replace it with an empty string.

Re: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-01-26 Thread Patrice Dumas 
On Wed, Jan 25, 2012 at 07:03:26PM -0800, David E. Wheeler wrote:
> 
> HTML:
> 
> > air ~> perl -MPod::Simple::HTML -E 
> > 'Pod::Simple::HTML->filter(\"=pod\n\n=head1 NAME\nX")' 
> > NAME Some entry

It does only here, because HTML is the only PullParser based converter
to use get_title.  I think the others do not use PullParser and/or
get_title.

-- 
Pat

Re: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-01-25 Thread Ronald J Kimball 
On Wed, Jan 25, 2012 at 07:03:26PM -0800, David E. Wheeler wrote:
> On Jan 25, 2012, at 6:00 PM, Ricardo Signes wrote:
> 
> >>> =head1 NAME
> >>> X
> >>> 
> >>> Pod::Simple::Pullparser get_title expands 'Some entry' in the title.  It
> >>> seems to me that it should not, and instead should replace it with an 
> >>> empty string.  This  behaviour is also hinted in the pod documentation.
> > 
> > It becomes NAME Some Entry?  That would certainly be an error.
> 
> No, it doesn't. Text:
> 
> > air ~> perl -MPod::Simple::Text -E 
> > 'Pod::Simple::Text->filter(\"=pod\n\n=head1 NAME\nX")'   
> > NAME 


> XHTML:

> > NAME 
  ^

That space should not be there.


> HTML:
> 
> > air ~> perl -MPod::Simple::HTML -E 
> > 'Pod::Simple::HTML->filter(\"=pod\n\n=head1 NAME\nX")' 
> > NAME Some entry
   ^^^

Here it did become "NAME Some entry", which is wrong.

> >  > name="NAME"
> > >NAME

Re: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-01-25 Thread Ricardo Signes 
* "David E. Wheeler"  [2012-01-25T22:03:26]
> On Jan 25, 2012, at 6:00 PM, Ricardo Signes wrote:
> 
> >>> =head1 NAME
> >>> X
> >>> 
> >>> Pod::Simple::Pullparser get_title expands 'Some entry' in the title.  It
> >>> seems to me that it should not, and instead should replace it with an 
> >>> empty string.  This  behaviour is also hinted in the pod documentation.
> > 
> > It becomes NAME Some Entry?  That would certainly be an error.
> 
> No, it doesn't. Text:

...then without a failing test to demonstrate what's up, I got nothin'.

-- 
rjbs


signature.asc
Description: Digital signature

Re: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-01-25 Thread David E. Wheeler 
On Jan 25, 2012, at 6:00 PM, Ricardo Signes wrote:

>>> =head1 NAME
>>> X
>>> 
>>> Pod::Simple::Pullparser get_title expands 'Some entry' in the title.  It
>>> seems to me that it should not, and instead should replace it with an 
>>> empty string.  This  behaviour is also hinted in the pod documentation.
> 
> It becomes NAME Some Entry?  That would certainly be an error.

No, it doesn't. Text:

> air ~> perl -MPod::Simple::Text -E 
> 'Pod::Simple::Text->filter(\"=pod\n\n=head1 NAME\nX")'   
> NAME 

XHTML:

> air ~> perl -MPod::Simple::XHTML -E 
> 'Pod::Simple::XHTML->filter(\"=pod\n\n=head1 NAME\nX")'
> 
> 
> 
> 
> 
> 
> 
> 
> 
> NAME 
> 
> 
> 

HTML:

> air ~> perl -MPod::Simple::HTML -E 
> 'Pod::Simple::HTML->filter(\"=pod\n\n=head1 NAME\nX")' 
> NAME Some entry
> 
> 
> 
> 
> 
> 
> 
> 
>  name="NAME"
> >NAME 
> 
> 
> 
> 

Best,

David

Re: Fwd: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-01-25 Thread Ricardo Signes 
* "David E. Wheeler"  [2012-01-25T15:12:03]
> > Subject: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should 
> > ignore X<...> 
> > Date: January 25, 2012 12:10:00 PM PST
> > Reply-To: bug-pod-sim...@rt.cpan.org
> > 
> > With Pod::Simple 3.14.
> > 
> > If a pod file has index entries after head:
> > 
> > =head1 NAME
> > X
> > 
> > Pod::Simple::Pullparser get_title expands 'Some entry' in the title.  It
> > seems to me that it should not, and instead should replace it with an 
> > empty string.  This  behaviour is also hinted in the pod documentation.

It becomes NAME Some Entry?  That would certainly be an error.

-- 
rjbs


signature.asc
Description: Digital signature

Fwd: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore X<...>

2012-01-25 Thread David E. Wheeler 

Begin forwarded message:

> Subject: [rt.cpan.org #74389] Pod::Simple::Pullparser get_title should ignore 
> X<...> 
> Date: January 25, 2012 12:10:00 PM PST
> Reply-To: bug-pod-sim...@rt.cpan.org
> 
> With Pod::Simple 3.14.
> 
> If a pod file has index entries after head:
> 
> =head1 NAME
> X
> 
> Pod::Simple::Pullparser get_title expands 'Some entry' in the title.  It
> seems to me that it should not, and instead should replace it with an 
> empty string.  This  behaviour is also hinted in the pod documentation.
> 
> A look at the code in git makes me think that the same issue is still 
> there (l. 363 and following all the text are used, X<> text is not 
> discarded).
> 
> This kind of construct is not rare, for instance it is present in the 
> main perl pod documentation.

Opinions?

Thanks,

David