Update `pod2html` to output HTML5 instead of XHTML #17987

scottchiefbaker · 2020-07-20T22:04:02Z

This should address issue #17986.

This commit is best viewed with whitespace ignored. I normalized the line endings to not have trailing whitespace, and it makes the diff noisier than it really is.

jkeenan · 2020-07-20T22:06:43Z

Is your subject line for the Issue correct? I would think the update would be for pod2html rather than pod2man.

scottchiefbaker · 2020-07-20T22:08:16Z

@jkeenan oh good catch. It's definitely pod2html I typod.

Adds a --xhtml to use output XHTML instead

References: https://design.max.gov/ui-typography.html https://www.smashingmagazine.com/2015/11/using-system-ui-fonts-practical-guide/

Adds a --xhtml to use output XHTML instead

References: https://design.max.gov/ui-typography.html https://www.smashingmagazine.com/2015/11/using-system-ui-fonts-practical-guide/

xsawyerx · 2020-10-04T23:04:54Z

ext/Pod-Html/lib/Pod/Html.pm

+		$ret = "$block\n</body>\n\n</html>";
+	} elsif ($mode eq 'html') {
+		$ret = "$block\n</body>\n\n</html>";
+	}


I read this twice (first when reviewing the entire changeset and again when reviewing this single commit) and twice I stared at this trying to understand what the difference is between the content strings until I realized they are exactly the same. (Both times I skipped the comment line.)

And it's true that it's well past midnight here, but I doubt having the same line of code twice with these conditions is a good idea.

if ( $mode eq 'xhtml' || $mode eq 'html' ) { $ret = "$block\n</body>\n\n</html>"; }

This is easier to read and allows the author to easily refactor this in the future, which I think is the intention.

This was just forward thinking on my part... Eventually the HTML5 and XHTML versions will probably be different so I broke them out to prepare for that. You are 100% right, that currently they're identical. If you feel very strongly about it I can merge them.

xsawyerx · 2020-10-04T23:14:18Z

ext/Pod-Html/lib/Pod/Html.pm

@@ -842,4 +834,53 @@ sub relativize_url {
    return $rel_path;
 }

+sub get_header {
+	my $mode = shift() // "html";
+	my ($title, $csslink, $bodyid, $block) = @_;


This is very odd to me.

You're sending the mode (html/xhtml) in the only call to this function, so it's pointless to check // "html".

You're taking out the title, css link, body ID, and block immediately afterward, which is used by both modes.

I can only imagine one situation in which you would lack a mode but have correct values for the other variables - when you are sending undef to the function as the first argument. I can't see this happening.

You're 100% right on this... this is an easy fix.

xsawyerx · 2020-10-04T23:15:48Z

ext/Pod-Html/lib/Pod/Html.pm

@@ -543,6 +530,7 @@ sub parse_command_line {
                       'recurse!'   => \$opt_recurse,
                       'title=s'    => \$opt_title,
                       'verbose!'   => \$opt_verbose,
+                       'xhtml'      => \$opt_xhtml,


I think it more prudent to keep the current mode and turn on HTML 5 by having an --html5 mode.

My concern is what would break for various other non-HTML5 browsers. Before promoting making HTML 5 the default, I would look at lynx, elinks, et. al. to understand how they deal with it.

I just installed and tested with lynx 2.8.9 and elinks 0.12-0.62 and both render the HTML5 how I would expect a text browser to do so. Both are 100% readable and happy.

…TML5

scottchiefbaker · 2020-11-04T18:47:39Z

I have done some fixes on this and am happy with the output. Please take a look and review.

jkeenan · 2021-02-04T02:50:54Z

This should address issue #17986.

This commit is best viewed with whitespace ignored. I normalized the line endings to not have trailing whitespace, and it makes the diff noisier than it really is.

For those playing along at home, I think this is done via adding ?w=1 to the URL.

https://github.com/Perl/perl5/pull/17987/files?w=1

jkeenan · 2021-02-04T03:12:12Z

Given that we're changing the output of pod2html, the expected output in all of the files in this library's test suite must change. I checked out this branch from the contributor's repository, configured and built perl, then ran this library's test suite. They all failed.

If the expected output in our test suite is failing, then so to is the output of any darkpan use of pod2html as a command-line utility. That suggests that whatever changes we settle on will have to go through a deprecation cycle before being released.

That in turn suggests that we have to decide on the merits of the change, for which the discussion is ongoing in #17986.

Thank you very much.
Jim Keenan

Grinnz · 2021-02-04T03:18:25Z

For this reason I would agree with Sawyer's comment here that an opt-in --html5 option would be better. #17987 (comment)

But I am not strongly attached to this opinion; the likelihood of darkpan code relying on pod2html's exact output is low.

jkeenan · 2021-12-30T01:53:32Z

There has been no further discussion in this pull request since February of this year. The p.r. has had unresolved merge conflicts since July of this year.

My recommendation is that this p.r. be closed and that @scottchiefbaker (or anyone else who wants HTML5 output rather than XHTML) should explore this approach via a CPAN module. Such a CPAN module may wish to subclass ext/Pod-Html, especially since the sequence of method calls invoked by the Pod::Html::pod2html subroutine now documents the logical steps needed to convert POD to HTML. Such a CPAN module should also include a test suite that demonstrates the expected output of various arguments to that subroutine.

Unless someone objects I will close this p.r. around January 10.

Thank you very much.
Jim Keenan

scottchiefbaker changed the title ~~Update pod2man to output HTML5 instead of XHTML~~ Update pod2html to output HTML5 instead of XHTML Jul 20, 2020

Grinnz added ext/Pod-Html issues in the blead-upstream Pod-Html distribution type-utilities labels Jul 20, 2020

khwilliamson requested review from Grinnz and karenetheridge July 20, 2020 22:27

scottchiefbaker added 2 commits July 27, 2020 14:30

Output HTML5 intead of XHTML for pod2man

03c389c

Adds a --xhtml to use output XHTML instead

Modernize HTML output with font-stack heirarchy and colors

4c7c807

References: https://design.max.gov/ui-typography.html https://www.smashingmagazine.com/2015/11/using-system-ui-fonts-practical-guide/

khwilliamson added the awaiting review label Jul 30, 2020

scottchiefbaker added 4 commits August 3, 2020 14:55

Output HTML5 intead of XHTML for pod2man

52dc201

Adds a --xhtml to use output XHTML instead

Modernize HTML output with font-stack heirarchy and colors

4e2207f

References: https://design.max.gov/ui-typography.html https://www.smashingmagazine.com/2015/11/using-system-ui-fonts-practical-guide/

Normalize all the indentations, and add a vim modeline to keep it clean

0506c19

Further basic CSS updates

c17e423

scottchiefbaker force-pushed the PodHTML5 branch from d0f0ca4 to c17e423 Compare August 3, 2020 21:56

xsawyerx requested changes Oct 4, 2020

View reviewed changes

scottchiefbaker added 2 commits October 5, 2020 08:40

One big param string for consistency

1174130

For simplicity we're going to merge these two conditionals

6e62191

github-actions bot added the hasConflicts label Nov 4, 2020

scottchiefbaker added 8 commits November 3, 2020 17:16

Merge branch 'blead' of github.com:scottchiefbaker/perl5 into blead

1385e34

Merge branch 'blead' of https://github.com/Perl/perl5 into blead

8bf0531

Merge branch 'blead' of https://github.com/Perl/perl5 into blead

7325e1d

Normalize all the indentations, and add a vim modeline to keep it clean

fa5d7a5

Further basic CSS updates

ce3829e

One big param string for consistency

f50b908

For simplicity we're going to merge these two conditionals

568921d

Merge branch 'PodHTML5' of github.com:scottchiefbaker/perl5 into PodH…

390cfd9

…TML5

Don't redeclare a variable

e7e9b97

github-actions bot removed the hasConflicts label Nov 4, 2020

jkeenan added the needs-work The pull request needs changes still label Feb 4, 2021

github-actions bot added the hasConflicts label Jul 6, 2021

jkeenan added the Closable? We might be able to close this ticket, but we need to check with the reporter label Dec 30, 2021

scottchiefbaker closed this Dec 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `pod2html` to output HTML5 instead of XHTML #17987

Update `pod2html` to output HTML5 instead of XHTML #17987

scottchiefbaker commented Jul 20, 2020

jkeenan commented Jul 20, 2020

scottchiefbaker commented Jul 20, 2020

xsawyerx Oct 4, 2020

scottchiefbaker Oct 5, 2020

xsawyerx Oct 4, 2020

scottchiefbaker Oct 5, 2020

xsawyerx Oct 4, 2020

scottchiefbaker Oct 5, 2020

scottchiefbaker commented Nov 4, 2020

jkeenan commented Feb 4, 2021

jkeenan commented Feb 4, 2021

Grinnz commented Feb 4, 2021

jkeenan commented Dec 30, 2021

Update pod2html to output HTML5 instead of XHTML #17987

Update pod2html to output HTML5 instead of XHTML #17987

Conversation

scottchiefbaker commented Jul 20, 2020

jkeenan commented Jul 20, 2020

scottchiefbaker commented Jul 20, 2020

xsawyerx Oct 4, 2020

Choose a reason for hiding this comment

scottchiefbaker Oct 5, 2020

Choose a reason for hiding this comment

xsawyerx Oct 4, 2020

Choose a reason for hiding this comment

scottchiefbaker Oct 5, 2020

Choose a reason for hiding this comment

xsawyerx Oct 4, 2020

Choose a reason for hiding this comment

scottchiefbaker Oct 5, 2020

Choose a reason for hiding this comment

scottchiefbaker commented Nov 4, 2020

jkeenan commented Feb 4, 2021

jkeenan commented Feb 4, 2021

Grinnz commented Feb 4, 2021

jkeenan commented Dec 30, 2021

Update `pod2html` to output HTML5 instead of XHTML #17987

Update `pod2html` to output HTML5 instead of XHTML #17987