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

pod2html: Various markup errors with (nested) definition lists #9021

Closed
p5pRT opened this issue Sep 5, 2007 · 7 comments
Closed

pod2html: Various markup errors with (nested) definition lists #9021

p5pRT opened this issue Sep 5, 2007 · 7 comments

Comments

@p5pRT
Copy link

p5pRT commented Sep 5, 2007

Migrated from rt.perl.org#45211 (status was 'resolved')

Searchable as RT45211$

@p5pRT
Copy link
Author

p5pRT commented Sep 5, 2007

From elendil@planet.nl

Created by elendil@planet.nl

I have noticed several issues with definition lists generated
from pod files​:
- <dt> tags are not closed
- generated code contains spurious </li> closing tags
- when other (definition) lists are nested in a definition list, the
  indentation of sublevels gets messed up because of incorrect placement
  of <dt> tags

I have written a patch to lib/Pod/Html.pm that fixes these issues and
does some minor code cleanup related to list markup generation.

Full details, a testcase and the patch are available in the Debian bug
tracking system at​: http​://bugs.debian.org/440946.

Cheers,
Frans Pop

Perl Info

Flags:
    category=library
    severity=low

Site configuration information for perl v5.8.8:

Configured by Debian Project at Tue Dec  5 22:42:31 EST 2006.

Summary of my perl5 (revision 5 version 8 subversion 8) configuration:
  Platform:
    osname=linux, osvers=2.6.18-1-amd64, archname=x86_64-linux-gnu-thread-multi
    uname='linux gkar 2.6.18-1-amd64 #1 smp sat oct 21 18:36:02 cest 2006 x86_64 gnulinux '
    config_args='-Dusethreads -Duselargefiles -Dccflags=-DDEBIAN -Dcccdlflags=-fPIC -Darchname=x86_64-linux-gnu -Dprefix=/usr -Dprivlib=/usr/share/perl/5.8 -Darchlib=/usr/lib/perl/5.8 -Dvendorprefix=/usr -Dvendorlib=/usr/share/perl5 -Dvendorarch=/usr/lib/perl5 -Dsiteprefix=/usr/local -Dsitelib=/usr/local/share/perl/5.8.8 -Dsitearch=/usr/local/lib/perl/5.8.8 -Dman1dir=/usr/share/man/man1 -Dman3dir=/usr/share/man/man3 -Dsiteman1dir=/usr/local/man/man1 -Dsiteman3dir=/usr/local/man/man3 -Dman1ext=1 -Dman3ext=3perl -Dpager=/usr/bin/sensible-pager -Uafs -Ud_csh -Uusesfio -Uusenm -Duseshrplib -Dlibperl=libperl.so.5.8.8 -Dd_dosuid -des'
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=define use5005threads=undef useithreads=define usemultiplicity=define
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=define use64bitall=define uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-D_REENTRANT -D_GNU_SOURCE -DTHREADS_HAVE_PIDS -DDEBIAN -fno-strict-aliasing -pipe -I/usr/local/include'
    ccversion='', gccversion='4.1.2 20061115 (prerelease) (Debian 4.1.1-20)', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -L/usr/local/lib'
    libpth=/usr/local/lib /lib /usr/lib
    libs=-lgdbm -lgdbm_compat -ldb -ldl -lm -lpthread -lc -lcrypt
    perllibs=-ldl -lm -lpthread -lc -lcrypt
    libc=/lib/libc-2.3.6.so, so=so, useshrplib=true, libperl=libperl.so.5.8.8
    gnulibc_version='2.3.6'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -L/usr/local/lib'

Locally applied patches:
    


@INC for perl v5.8.8:
    /etc/perl
    /usr/local/lib/perl/5.8.8
    /usr/local/share/perl/5.8.8
    /usr/lib/perl5
    /usr/share/perl5
    /usr/lib/perl/5.8
    /usr/share/perl/5.8
    /usr/local/lib/site_perl
    .


Environment for perl v5.8.8:
    HOME=/home/fjp
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/fjp/bin:/usr/lib/ccache:/usr/local/bin:/usr/bin:/bin:/usr/bin/X11:/usr/games
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Sep 6, 2007

From schubiger@cpan.org

On Mi. 05. Sep. 2007, 15​:42​:37, elendil@​planet.nl wrote​:

This is a bug report for perl from elendil@​planet.nl,
generated with the help of perlbug 1.35 running under perl v5.8.8.

Since you've submitted your bug-report against 5.8.8, I feel obligated
to tell you that it is recommended practice to send patches against
latest Perl development track (also known as 'bleadperl'). There's the
chance that the bugs are no longer present in bleadperl. If you're
curious nonetheless, according informations on how to sync bleadperl and
send according patches can be obtained through 'perlhack'.

@p5pRT
Copy link
Author

p5pRT commented Sep 6, 2007

The RT System itself - Status changed from 'new' to 'open'

@p5pRT
Copy link
Author

p5pRT commented Sep 7, 2007

From elendil@planet.nl

Adding module author to CC as suggested in 'perlhack'.

Original BR​:
- http​://rt.perl.org/rt3/Public/Bug/Display.html?id=45211
Details about issue and patch at​:
- http​://bugs.debian.org/440946

Issue is issues with (nested) definition lists in lib/Pod/Html.pm
- <dt> tags are not closed
- generated code contains spurious </li> closing tags
- when other (definition) lists are nested in a definition list, the
  indentation of sublevels gets messed up because of incorrect placement
  of <dt> tags

On Friday 07 September 2007, Steven Schubiger via RT wrote​:

Since you've submitted your bug-report against 5.8.8, I feel obligated
to tell you that it is recommended practice to send patches against
latest Perl development track (also known as 'bleadperl'). There's the
chance that the bugs are no longer present in bleadperl. If you're
curious nonetheless, according informations on how to sync bleadperl and
send according patches can be obtained through 'perlhack'.

Fine. But please remember that there is such a thing as relatively dumb
users who are just using perl from their distro and read man perlbug which
just says "go on, report against the version you are running".

And then there are users who are just slightly less dumb and say "hey, I
think I can fix that; let's give it a try" and who, instead of keeping the
fix to themselves (or just within their own distro), decide to share it
back with the developers.

I really don't think you can expect people to graduate from dumb user to
bleedperl hacker overnight and I don't even think you can expect people to
just "find" perlhack. I have only scanned 'perlhack', so I may well still
have missed some things.

Anyway​:
- no, the issue is not fixed in bleedperl
- running the tests luckily proved simple; testcases even already cover
  the issues, but the current "expect" output is broken (invalid XML)

Attached a new patch updated against bleedperl, including updated tests.

Note that one test now throws a few warnings for a check I added, but that
is because the content in the input file is basically nonsense.

Cheers,
Frans Pop
(Fairly dumb user, at least when it comes to Perl, with no aspirations to
become a Perl developer)

@p5pRT
Copy link
Author

p5pRT commented Sep 7, 2007

From elendil@planet.nl

DL_bleed.patch
--- lib/Pod/Html.pm.sv	2007-03-23 15:49:04.000000000 +0100
+++ lib/Pod/Html.pm	2007-09-07 02:13:07.000000000 +0200
@@ -246,8 +246,8 @@
 my $Doindex;
 
 my $Backlink;
-my($Listlevel, @Listend);
-my $After_Lpar;
+my($Listlevel, @Listtype);
+my $ListNewTerm;
 use vars qw($Ignore);  # need to localize it later.
 
 my(%Items_Named, @Items_Seen);
@@ -286,7 +286,7 @@
     $Htmldir = "";	    	# The directory to which the html pages
 				# will (eventually) be written.
     $Htmlfile = "";		# write to stdout by default
-    $Htmlfileurl = "" ;		# The url that other files would use to
+    $Htmlfileurl = "";		# The url that other files would use to
 				# refer to this file.  This is only used
 				# to make relative urls that point to
 				# other files.
@@ -302,8 +302,9 @@
     $Doindex = 1;   	    	# non-zero if we should generate an index
     $Backlink = '';		# text for "back to top" links
     $Listlevel = 0;		# current list depth
-    @Listend = ();		# the text to use to end the list.
-    $After_Lpar = 0;            # set to true after a par in an =item
+    @Listtype = ();		# list types for open lists
+    $ListNewTerm = 0;		# indicates new term in definition list; used
+    				# to correctly open/close <dd> tags
     $Ignore = 1;		# whether or not to format text.  we don't
 				#   format text until we hit our first pod
 				#   directive.
@@ -519,7 +520,6 @@
 
     # now convert this file
     my $after_item;             # set to true after an =item
-    my $need_dd = 0;
     warn "Converting input file $Podfile\n" if $Verbose;
     foreach my $i (0..$#poddata){
 	$_ = $poddata[$i];
@@ -527,7 +527,6 @@
 	if (/^(=.*)/s) {	# is it a pod directive?
 	    $Ignore = 0;
 	    $after_item = 0;
-	    $need_dd = 0;
 	    $_ = $1;
 	    if (/^=begin\s+(\S+)\s*(.*)/si) {# =begin
 		process_begin($1, $2);
@@ -543,12 +542,12 @@
 		if (/^=(head[1-6])\s+(.*\S)/s) {	# =head[1-6] heading
 		    process_head( $1, $2, $Doindex && $index );
 		} elsif (/^=item\s*(.*\S)?/sm) {	# =item text
-		    $need_dd = process_item( $1 );
+		    process_item( $1 );
 		    $after_item = 1;
 		} elsif (/^=over\s*(.*)/) {		# =over N
 		    process_over();
 		} elsif (/^=back/) {		# =back
-		    process_back($need_dd);
+		    process_back();
 		} elsif (/^=for\s+(\S+)\s*(.*)/si) {# =for
 		    process_for($1,$2);
 		} else {
@@ -563,8 +562,14 @@
 	    next if $Ignore;
 	    next if @Begin_Stack && $Begin_Stack[-1] ne 'html';
 	    print HTML and next if @Begin_Stack && $Begin_Stack[-1] eq 'html';
-	    print HTML "<dd>\n" if $need_dd;
 	    my $text = $_;
+
+	    # Open tag for definition list as we have something to put in it
+	    if( $ListNewTerm ){
+		print HTML "<dd>\n";
+		$ListNewTerm = 0;
+	    }
+
 	    if( $text =~ /\A\s+/ ){
 		process_pre( \$text );
 	        print HTML "<pre>\n$text</pre>\n";
@@ -594,12 +599,8 @@
 		}
 		## end of experimental
 
-		if( $after_item ){
-		    $After_Lpar = 1;
-		}
 		print HTML "<p>$text</p>\n";
 	    }
-	    print HTML "</dd>\n" if $need_dd;
 	    $after_item = 0;
 	}
     }
@@ -1074,12 +1075,12 @@
 
 	# figure out what kind of item it is.
 	# Build string for referencing this item.
-	if ( $txt =~ /\A=item\s+\*\s*(.*)\Z/s ) { # bullet
+	if ( $txt =~ /\A=item\s+\*\s*(.*)\Z/s ) { # bulleted list
 	    next unless $1;
 	    $item = $1;
         } elsif( $txt =~ /\A=item\s+(?>\d+\.?)\s*(.*)\Z/s ) { # numbered list
 	    $item = $1;
-	} elsif( $txt =~ /\A=item\s+(.*)\Z/s ) { # plain item
+	} elsif( $txt =~ /\A=item\s+(.*)\Z/s ) { # definition list
 	    $item = $1;
 	} else {
 	    next;
@@ -1099,12 +1100,7 @@
     $tag =~ /head([1-6])/;
     my $level = $1;
 
-    if( $Listlevel ){
-	warn "$0: $Podfile: unterminated list at =head in paragraph $Paragraph.  ignoring.\n" unless $Quiet;
-        while( $Listlevel ){
-            process_back();
-        }
-    }
+    finish_list();
 
     print HTML "<p>\n";
     if( $level == 1 && ! $Top ){
@@ -1143,19 +1139,32 @@
         $name = anchorify($name);
 	print HTML qq{<a name="$name" class="item">}, process_text( \$otext ), '</a>';
     }
-    print HTML "</strong>\n";
+    print HTML "</strong>";
     undef( $EmittedItem );
 }
 
-sub emit_li {
+sub new_listitem {
     my( $tag ) = @_;
+    # Open tag for definition list as we have something to put in it
+    if( ($tag ne 'dl') && ($ListNewTerm) ){
+	print HTML "<dd>\n";
+	$ListNewTerm = 0;
+    }
+
     if( $Items_Seen[$Listlevel]++ == 0 ){
-	push( @Listend, "</$tag>" );
+	# start of new list
+	push( @Listtype, "$tag" );
 	print HTML "<$tag>\n";
+    } else {
+	# if this is not the first item, close the previous one
+	if ( $tag eq 'dl' ){
+	    print HTML "</dd>\n" unless $ListNewTerm;
+	} else {
+	    print HTML "</li>\n";
+	}
     }
-    my $emitted = $tag eq 'dl' ? 'dt' : 'li';
-    print HTML "<$emitted>";
-    return $emitted;
+    my $opentag = $tag eq 'dl' ? 'dt' : 'li';
+    print HTML "<$opentag>";
 }
 
 #
@@ -1163,7 +1172,6 @@
 #
 sub process_item {
     my( $otext ) = @_;
-    my $need_dd = 0; # set to 1 if we need a <dd></dd> after an item
 
     # lots of documents start a list without doing an =over.  this is
     # bad!  but, the proper thing to do seems to be to just assume
@@ -1173,43 +1181,43 @@
 	process_over();
     }
 
-    # formatting: insert a paragraph if preceding item has >1 paragraph
-    if( $After_Lpar ){
-	print HTML $need_dd ? "</dd>\n" : "</li>\n" if $After_Lpar;
-	$After_Lpar = 0;
-    }
-
     # remove formatting instructions from the text
     my $text = depod( $otext );
 
-    my $emitted; # the tag actually emitted, used for closing
-
     # all the list variants:
     if( $text =~ /\A\*/ ){ # bullet
-        $emitted = emit_li( 'ul' );
+        new_listitem( 'ul' );
         if ($text =~ /\A\*\s+(\S.*)\Z/s ) { # with additional text
             my $tag = $1;
             $otext =~ s/\A\*\s+//;
             emit_item_tag( $otext, $tag, 1 );
+            print HTML "\n";
         }
 
     } elsif( $text =~ /\A\d+/ ){ # numbered list
-        $emitted = emit_li( 'ol' );
+        new_listitem( 'ol' );
         if ($text =~ /\A(?>\d+\.?)\s*(\S.*)\Z/s ) { # with additional text
             my $tag = $1;
             $otext =~ s/\A\d+\.?\s*//;
             emit_item_tag( $otext, $tag, 1 );
+            print HTML "\n";
         }
 
     } else {			# definition list
-        $emitted = emit_li( 'dl' );
-        if ($text =~ /\A(.+)\Z/s ){ # should have text
+        # new_listitem takes care of opening the <dt> tag
+        new_listitem( 'dl' );
+        if( $text =~ /\A(.+)\Z/s ){ # should have text
             emit_item_tag( $otext, $text, 1 );
+        } else {
+            warn "$0: $Podfile: no term text provided for definition list in paragraph $Paragraph.  ignoring.\n" unless $Quiet;
         }
-        $need_dd = 1;
+        # write the definition term and close <dt> tag
+        print HTML "</dt>\n";
+        # trigger opening a <dd> tag for the actual definition; will not
+        # happen if next paragraph is also a definition term (=item)
+        $ListNewTerm = 1;
     }
     print HTML "\n";
-    return $need_dd;
 }
 
 #
@@ -1219,30 +1227,31 @@
     # start a new list
     $Listlevel++;
     push( @Items_Seen, 0 );
-    $After_Lpar = 0;
 }
 
 #
 # process_back - process a pod back tag and convert it to HTML format.
 #
 sub process_back {
-    my $need_dd = shift;
     if( $Listlevel == 0 ){
 	warn "$0: $Podfile: unexpected =back directive in paragraph $Paragraph.  ignoring.\n" unless $Quiet;
 	return;
     }
 
-    # close off the list.  note, I check to see if $Listend[$Listlevel] is
+    # close off the list.  note, I check to see if $Listtype[$Listlevel] is
     # defined because an =item directive may have never appeared and thus
-    # $Listend[$Listlevel] may have never been initialized.
+    # $Listtype[$Listlevel] may have never been initialized.
     $Listlevel--;
-    if( defined $Listend[$Listlevel] ){
-	print HTML $need_dd ? "</dd>\n" : "</li>\n" if $After_Lpar;
-	print HTML $Listend[$Listlevel];
-        print HTML "\n";
-        pop( @Listend );
+    if( defined $Listtype[$Listlevel] ){
+        if ( $Listtype[$Listlevel] eq 'dl' ){
+            print HTML "</dd>\n" unless $ListNewTerm;
+        } else {
+            print HTML "</li>\n";
+        }
+        print HTML "</$Listtype[$Listlevel]>\n";
+        pop( @Listtype );
+        $ListNewTerm = 0;
     }
-    $After_Lpar = 0;
 
     # clean up item count
     pop( @Items_Seen );
@@ -2025,9 +2034,11 @@
 # after the entire pod file has been read and converted.
 #
 sub finish_list {
-    while ($Listlevel > 0) {
-	print HTML "</dl>\n";
-	$Listlevel--;
+    if( $Listlevel ){
+	warn "$0: $Podfile: unterminated list(s) at =head in paragraph $Paragraph.  ignoring.\n" unless $Quiet;
+	while( $Listlevel ){
+            process_back();
+        }
     }
 }
 
--- lib/Pod/t/htmllink.t.sv	2007-09-07 02:10:01.000000000 +0200
+++ lib/Pod/t/htmllink.t	2007-09-07 02:13:21.000000000 +0200
@@ -108,24 +108,21 @@
 <h2><a name="section_three">section three</a></h2>
 <p>This is section three.</p>
 <dl>
-<dt><strong><a name="item1" class="item">item1</a></strong>
+<dt><strong><a name="item1" class="item">item1</a></strong></dt>
 
 <dd>
 <p>This is item one.</p>
 </dd>
-</li>
-<dt><strong><a name="item_2" class="item">item 2</a></strong>
+<dt><strong><a name="item_2" class="item">item 2</a></strong></dt>
 
 <dd>
 <p>This is item two.</p>
 </dd>
-</li>
-<dt><strong><a name="item_three" class="item">item three</a></strong>
+<dt><strong><a name="item_three" class="item">item three</a></strong></dt>
 
 <dd>
 <p>This is item three.</p>
 </dd>
-</li>
 </dl>
 
 </body>
--- lib/Pod/t/htmlview.t.sv	2007-09-07 01:58:23.000000000 +0200
+++ lib/Pod/t/htmlview.t	2007-09-07 02:13:21.000000000 +0200
@@ -86,17 +86,15 @@
 <h2><a name="new__"><code>new()</code></a></h2>
 <p>Constructor method.  Accepts the following config options:</p>
 <dl>
-<dt><strong><a name="foo" class="item">foo</a></strong>
+<dt><strong><a name="foo" class="item">foo</a></strong></dt>
 
 <dd>
 <p>The foo item.</p>
 </dd>
-</li>
-<dt><strong><a name="bar" class="item">bar</a></strong>
+<dt><strong><a name="bar" class="item">bar</a></strong></dt>
 
 <dd>
 <p>The bar item.</p>
-</dd>
 <p>This is a list within a list</p>
 <ul>
 <li>
@@ -106,48 +104,54 @@
 <p>The waz item.</p>
 </li>
 </ul>
-<dt><strong><a name="baz" class="item">baz</a></strong>
+</dd>
+<dt><strong><a name="baz" class="item">baz</a></strong></dt>
 
 <dd>
 <p>The baz item.</p>
 </dd>
-</li>
 </dl>
 <p>Title on the same line as the =item + * bullets</p>
 <ul>
 <li><strong><a name="black_cat" class="item"><code>Black</code> Cat</a></strong>
 
+</li>
 <li><strong><a name="sat_on_the" class="item">Sat <em>on</em>&nbsp;the</a></strong>
 
+</li>
 <li><strong><a name="mat" class="item">Mat&lt;!&gt;</a></strong>
 
+</li>
 </ul>
 <p>Title on the same line as the =item + numerical bullets</p>
 <ol>
 <li><strong><a name="cat" class="item">Cat</a></strong>
 
+</li>
 <li><strong><a name="sat" class="item">Sat</a></strong>
 
+</li>
 <li><strong><a name="mat2" class="item">Mat</a></strong>
 
+</li>
 </ol>
 <p>No bullets, no title</p>
 <dl>
-<dt>
+<dt></dt>
+
 <dd>
 <p>Cat</p>
 </dd>
-</li>
-<dt>
+<dt></dt>
+
 <dd>
 <p>Sat</p>
 </dd>
-</li>
-<dt>
+<dt></dt>
+
 <dd>
 <p>Mat</p>
 </dd>
-</li>
 </dl>
 <p>
 </p>

@p5pRT
Copy link
Author

p5pRT commented Dec 26, 2007

From @smpeters

On Wed Sep 05 15​:42​:37 2007, elendil@​planet.nl wrote​:

This is a bug report for perl from elendil@​planet.nl,
generated with the help of perlbug 1.35 running under perl v5.8.8.

-----------------------------------------------------------------
[Please enter your report here]
I have noticed several issues with definition lists generated
from pod files​:
- <dt> tags are not closed
- generated code contains spurious </li> closing tags
- when other (definition) lists are nested in a definition list, the
indentation of sublevels gets messed up because of incorrect
placement
of <dt> tags

I have written a patch to lib/Pod/Html.pm that fixes these issues and
does some minor code cleanup related to list markup generation.

Full details, a testcase and the patch are available in the Debian bug
tracking system at​: http​://bugs.debian.org/440946.

The Debian patch has been applied as change #32727. Thanks!

@p5pRT
Copy link
Author

p5pRT commented Dec 26, 2007

@smpeters - Status changed from 'open' to 'resolved'

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

1 participant