asciidoctor / asciidoctor-gradle-plugin

A Gradle plugin that uses Asciidoctor via JRuby to process AsciiDoc source files within the project.
https://asciidoctor.github.io/asciidoctor-gradle-plugin/
Apache License 2.0
286 stars 122 forks source link

Setting custom font directory in Kotlin DSL script file (.kts) does not take semicolon into consideration #561

Closed halitanildonmez closed 4 years ago

halitanildonmez commented 4 years ago

Did:

Happened:

Expected:

See also https://github.com/junit-team/junit5/issues/2315#issuecomment-667431555

mojavelinux commented 4 years ago

In summary, the pdf-fontsdir attribute can accept multiple paths. Multiple entries must be separated by either a comma or a semi-colon (but not a colon).

It properly makes sense for the Gradle property to accept either a string or an Array, then prepare the value accordingly for Asciidoctor PDF.

ysb33r commented 4 years ago

Right, I see what is gong on here. Gradle validates the single path so that i knows whether it has changes. What we need here is the ability to supply Gradle with a list of directories, which is can then check and finally before calling the converter conctenate the list of paths using the OS path separator.

For this I'll need to deprecate the current method and add an additional one which can take more than one path.

mojavelinux commented 4 years ago

:+1: setFontDirs seems like the logical choice.

halitanildonmez commented 4 years ago

Just out of curiosity: can I tackle this one ? Or has it already been solved ?

ysb33r commented 4 years ago

@halitanildonmez Please go ahead an try. I'll be happy to review and provide feedback on the PR. (please do it against the development-3.x branch and not master).

halitanildonmez commented 4 years ago

Ok forking and working on it.

halitanildonmez commented 4 years ago

I have updated some files but have a quick question: how can I test this ? I ma updating "intTest" but not sure that is the correct place ?

ysb33r commented 4 years ago

I have updated some files but have a quick question: how can I test this ? I ma updating "intTest" but not sure that is the correct place ?

halitanildonmez commented 4 years ago

Question: right now the font path is a file. What I am trying is to return a list of files. I was thinking of using @InputFiles annotation. Would that be the correct thing to do ?

ysb33r commented 4 years ago

That is the correct approach. Remember that you need to leave the old method in place and annotate it with @Deprecated. You can remove the @InputFile annotation from that and forward the method to call your new method with the single file.

halitanildonmez commented 4 years ago

I have been working on this and right now I am doing this: https://github.com/halitanildonmez/asciidoctor-gradle-plugin/commit/b680bc9afb5566fab131259ec797538985ca951a

Yet I do not know how to verify and whether this is what you mean ?

ysb33r commented 4 years ago

For getFontsDir:

halitanildonmez commented 4 years ago

Fixed I think. @ysb33r can you take a look at the merge request ? I can add more tests or any other test you want