Skip to content

Conversation

@theory
Copy link
Contributor

@theory theory commented Jan 22, 2014

There are a few reasons for this:

  • It's HTML output is much cleaner.
  • You can tell it not to emit headers and footers directly.

The only downside is that you need a fairly recent version of Pod::Simple for
it to be there and really solid. I recommend 3.12 or higher, and at least
3.11.

@bkeepers
Copy link
Contributor

That was quick ;)

Do the tests pass for you? They're failing for me:

$ perl -v       
This is perl 5, version 16, subversion 2 (v5.16.2) built for darwin-thread-multi-2level

$ bundle exec rake
  1) Failure:
test_pod(MarkupTest) [/Users/brandon/github/markup/test/markup_test.rb:28]:
README.pod.html's contents don't match command output:
--- -   2014-01-21 21:50:33.000000000 -0800
…

@theory
Copy link
Contributor Author

theory commented Jan 22, 2014

I don't run the tests, but noticed the failure in Travis. Hopefully 8d326ff will fix it.

@theory
Copy link
Contributor Author

theory commented Jan 22, 2014

Hrm. Looking at this failure, it appears that perldoc isn't getting called. Is there something that's not recognizing that file as Pod? I don't know enough about how this works to judge.

@theory
Copy link
Contributor Author

theory commented Jan 22, 2014

For some reason, it is comparing the raw Pod to the HTML. Curious. It looks like it properly generated HTML in this test, but not after I committed the updated README.pod.html file. I'm sure I messed something up. :-(

@bkeepers
Copy link
Contributor

You mentioned that Pod 3.12 is recommended. What version of perl is that installed with? How do we install the latest?

@theory
Copy link
Contributor Author

theory commented Jan 22, 2014

3.12 was released in 2009, so it should be in 5.12. You just need to have Pod::Simple::XHTML installed. You can get the latest from CPAN with cpan Pod::Simple (or, better perhaps, cpanm Pod::Simple). Not sure what package systems you have, but in RPM it would be perl-Pod-Simple. No idea what version, though.

@bkeepers
Copy link
Contributor

Any idea why this is failing?

@theory
Copy link
Contributor Author

theory commented Jan 24, 2014

What version of Pod::Simple is on those boxes? We could try updating it by running cpanm Pod::Simple in .travis.yml.

@theory
Copy link
Contributor Author

theory commented Jan 24, 2014

I don't get it. Maybe perldoc needs to be installed?

@theory
Copy link
Contributor Author

theory commented Jan 24, 2014

/me sighs

@bkeepers Can you suggest how to get command() to throw an exception instead of silently throwing away errors. That's the only thing I can guess it's doing. Need to know what the problem is. Thanks.

@bkeepers
Copy link
Contributor

@theory The easiest thing would probably just be to make a call to perldoc in the before_install.

@theory
Copy link
Contributor Author

theory commented Jan 24, 2014

Well, it stupidly sent output to a pager, but the output looks right. So I don't know WTF is going on.

@theory
Copy link
Contributor Author

theory commented Jan 24, 2014

@bkeepers The before_install command worked. Is there some way to get command() to throw an exception or something?

@bkeepers
Copy link
Contributor

bkeepers commented Feb 4, 2014

@theory
Copy link
Contributor Author

theory commented Feb 4, 2014

Hrm. 6786b1f's not going into the Travis build queue… :-(

@theory
Copy link
Contributor Author

theory commented Feb 5, 2014

Any idea what the issue is with Travis, @bkeepers?

@bkeepers
Copy link
Contributor

bkeepers commented Feb 5, 2014

No clue. I'm not seeing it show up here either https://travis-ci.org/github/markup/pull_requests

What happens if you push something again?

@theory
Copy link
Contributor Author

theory commented Feb 5, 2014

Let's find out.

@theory
Copy link
Contributor Author

theory commented Feb 5, 2014

Crap. I guess I need to create another pull request?

@bkeepers
Copy link
Contributor

bkeepers commented Feb 6, 2014

I'm not sure what's going on with travis.

I tried running the perl command locally after installing the latest Pod::Simple and it is showing me usage info:

$ cat test/markups/README.pod | \
  /usr/bin/env perldoc -MPod::Simple::XHTML -w html_header: -w html_footer: -T    
Usage: perldoc [-hVriDtumFXlT] [-n nroffer_program]
    [-d output_filename] [-o output_format] [-M FormatterModule]
    [-w formatter_option:option_value] [-L translation_code]
    PageName|ModuleName|ProgramName

Examples:

    perldoc -f PerlFunc
    perldoc -q FAQKeywords
    perldoc -v PerlVar

The -h option prints more help.  Also try "perldoc perldoc" to get
acquainted with the system.                        [Perldoc v3.17]

But let's try this, create a pod2html file in lib/github/commands that's just a perl script that reads the from STDIN and outputs html to STDOUT. If you get that working, I can make sure the ruby side is working.

@theory
Copy link
Contributor Author

theory commented Feb 7, 2014

Oh, that’s an old version of env that fails to pass arguments to Perl. Good idea on the script; will do that shortly.

@theory
Copy link
Contributor Author

theory commented Feb 7, 2014

Okay, done in adbadcc. Be aware that Perl itself ships with a script named pod2html, so be sure to call this one.

There are a few reasons for this:

* It's HTML output is *much* cleaner.
* You can tell it not to emit headers and footers directly.

The only downside is that you need a fairly recent version of Pod::Simple for
it to be there and really solid. I recommend 3.12 or higher, and at least
3.11.
I'm sure there is one, since Perl builds use it, but I don't know what it's called. This will do for now.
To see what the errors are.
By putting it all into a Perl script, we can more easily control things,
including being able to add code to sent the indentation for verbatim
blocks to the indentation of the first line.
@theory
Copy link
Contributor Author

theory commented Feb 10, 2014

Rebased.

The perl-doc package failed to install with a 404. But maybe it will not be needed now.
@theory
Copy link
Contributor Author

theory commented Feb 10, 2014

Okay, back to the thing refusing to convert the Pod again. Can you look into it now, @bkeepers?

@bkeepers
Copy link
Contributor

Yep, it'll be a few days, but it's on my list.

@theory
Copy link
Contributor Author

theory commented Feb 11, 2014

Great, thank you.

@zmughal
Copy link

zmughal commented Dec 27, 2014

Is this worth merging? It should be combined with #421, imo.

@mohawk2
Copy link

mohawk2 commented Dec 27, 2014

@zmughal
Copy link

zmughal commented Apr 26, 2016

The spammers above have brought this back to my attention. What can we do to move this forward?

@zmughal zmughal mentioned this pull request Apr 26, 2016
@zmughal
Copy link

zmughal commented Sep 19, 2016

Can we move this forward?

@zmughal
Copy link

zmughal commented Oct 26, 2016

@bkeepers @gjtorikian ?

@gjtorikian
Copy link
Contributor

@github/workflow for determination on rendering these documents.

@kivikakk kivikakk mentioned this pull request Mar 15, 2017
@kivikakk
Copy link
Contributor

I've resurrected this at #241 with a master merge. I would love to get this merged!

@kivikakk
Copy link
Contributor

I've also folded #421 into #1012.

kivikakk pushed a commit that referenced this pull request Mar 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants