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

helpers: Add --trace to asciidoctor args #3714

Merged
merged 3 commits into from
Jul 21, 2017

Conversation

miltador
Copy link
Contributor

This will help to understand and fix errors by seeing stacktrace of an error.

@miltador miltador requested a review from bep as a code owner July 18, 2017 08:55
@bep bep requested a review from anthonyfok July 18, 2017 08:58
@bep
Copy link
Member

bep commented Jul 18, 2017

I'm by no means an expert here, but we do fallback to asciidoc if asciidoctor isn't available -- and I don't see that flag there.

@miltador
Copy link
Contributor Author

I think asciidoc will just ignore it. Or I can add a check to only add --trace for asciidoctor.

@anthonyfok
Copy link
Member

Hi @miltador,

I think asciidoc will just ignore it.

$ asciidoc --version
asciidoc 8.6.9
$ asciidoc --trace
asciidoc: illegal command options
$ echo $?
1

So asciidoc does complain about an unknown command-line option and does exit with an error.

Or I can add a check to only add --trace for asciidoctor.

Please do so. 👍 Thank you for your contribution! 😉

This will help to understand and fix errors by
seeing stacktrace of an error.
@miltador
Copy link
Contributor Author

Thanks for a test! Added a check.

Will add a smarter check (WIP).

@miltador
Copy link
Contributor Author

Added smarter check for asciidoctor.
@anthonyfok it is ready for review now.

@miltador miltador force-pushed the asciidoctor-trace-errors branch 2 times, most recently from 967151b to 5a9eba1 Compare July 18, 2017 20:58
Copy link
Member

@anthonyfok anthonyfok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @miltador!

One tiny problem though: If both AsciiDoc and Asciidoctor are installed, your code would choose AsciiDoc over Asciidoctor. I am pretty sure that is not what you intended, especially since you use Ascidoctor yourself, and also because previous Hugo code has always "favoured" Asciidoctor over AsciiDoc. :-)

Could you please take a look again? Many thanks!

@miltador
Copy link
Contributor Author

@anthonyfok, switched the order to first check for Asciidoctor and then for Asciidoc.

Copy link
Member

@anthonyfok anthonyfok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was very quick! Thank you @miltador!
Looks good to me! :-)

@anthonyfok anthonyfok merged commit b60aa1a into gohugoio:master Jul 21, 2017
@miltador miltador deleted the asciidoctor-trace-errors branch July 21, 2017 09:09
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants