-
Notifications
You must be signed in to change notification settings - Fork 188
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
Upgrade to Asciidoctor.js 2+ #254
Upgrade to Asciidoctor.js 2+ #254
Conversation
I would prefer the unrelated changes to be split out. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: I haven't tested it yet.
It's cool to get rid of the need to have users create their own converter script but to also keep it as an option with a stable API.
Could you be more specific ? All changes are related to the upgrade 🤔 |
I would prefer if the new syntax highlighter changes would be separate. I think this is what is breaking the tests right now. |
A test was failing when running with Asciidoctor.js: https://github.com/asciidoctor/asciidoctor-reveal.js/blob/master/examples/source-pygments.adoc But you're right, I'm using the following (which is working with Asciidoctor.js but failing with Asciidoctor): - linkcss = node.attr? 'linkcss' I think it should be |
6f94920
to
84414e7
Compare
Is this good to merge? |
@obilodeau Please let me know if there's anything to fix before we can merge this upgrade 😃 |
I just did a new review. Only minor stuff, mostly documentation and comments. |
This pull request has too much changes, I will open pull request with smaller changes so it's easier for you to review. |
5b519c1
to
886da22
Compare
886da22
to
02c004f
Compare
02c004f
to
cdd1cea
Compare
Now the documentation is Windows/PowerShell friendly. |
Resolves #252