search

asciidoctor / asciidoctorj

:coffee: Java bindings for Asciidoctor. Asciidoctor on the JVM!
http://asciidoctor.org
Apache License 2.0
617 stars 172 forks source link

Reuse Asciidoctor Ruby invoker #1249

Closed robertpanzer closed 6 months ago

robertpanzer commented 7 months ago

Thank you for opening a pull request and contributing to AsciidoctorJ!

Please take a bit of time giving some details about your pull request:

Kind of change

Description

What is the goal of this pull request?

This PR should do the same thing as #1248, that is better align the behavior of the AsciidoctorJ CLI with the behavior of the Asciidoctor Ruby CLI. This PR however, reuses the original Asciidoctor::Cli::Invoker and feeds its own options into it instead of letting Asciidoctor Ruby parse them.

So chances are that this works a bit better than the other PR.

The code is still raw, but I'd like to get some feedback on what approach to pursue.

How does it achieve that?

Are there any alternative ways to implement this?

Are there any implications of this pull request? Anything a user must know?

Issue

If this PR fixes an open issue, please add a line of the form:

Fixes #Issue

Release notes

Please add a corresponding entry to the file CHANGELOG.adoc

abelsromero commented 7 months ago

The code is still raw, but I'd like to get some feedback on what approach to pursue.

I can have a look tomorrow. But 1 question, from the description it seems this is preferable to #1248, is there something between the two you are not convinced, so both should be checked. Or this is the final approach? To me it makes sense to reuse the invoker, but I haven't looked into the details.

mojavelinux commented 7 months ago

I think this PR addresses #193. Although this implementation takes a different approach, the spirit of the solution is still the same, to reuse the Asciidoctor CLI instead of maintaining a clone of it. In that regard, I think it's a great step forward.

robertpanzer commented 6 months ago

@abelsromero About

only the fact that it seems this does not support input redirection

I tried this, and I see the output piped to less:

echo '= TEST' | ../asciidoctorj-distribution/build/install/asciidoctorj-distribution/bin/asciidoctorj - |less

Can you share some details about what failed for you?

abelsromero commented 6 months ago

Can you share some details about what failed for you?

I mentioned because I saw the code being remove, but it's working fine for me (Linux). I built a distribution and converted both files and from cli cat sample.adoc | asciidoctorj - > sample.html perfectly.

mojavelinux commented 6 months ago

:+1: