Expose Document's source and source_lines properties #1145
Conversation
abelsromero
force-pushed
the
issue-1143-expose-document-source-and-source_lines
branch
from
5e99631
to
4d64810
Compare
|
|
||
| @Test | ||
| public void should_return_empty_when_document_is_null() { | ||
| assertEmptySources(loadDocument(null)); |
There was a problem hiding this comment.
This test seems to fail for me with a NullPointerException.
It seems like this could be fixed by changing JRubyAsciidoctor.load() like so:
@Override
public Document load(String content, Options options) {
RubyHash rubyHash = RubyHashUtil.convertMapToRubyHashWithSymbols(rubyRuntime, options.map());
return (Document) NodeConverter.createASTNode(getAsciidoctorModule().callMethod("load",
Optional.ofNullable(content).map(rubyRuntime::newString).orElse(null), rubyHash));
}There was a problem hiding this comment.
I don't know how I run it locally, but I'd rather remove the test and deal with supporting null consistently in all cases (String, File, etc.) in a separate PR. wdyt?
There was a problem hiding this comment.
Being purist, if null was an error case, changing it now would be a breaking change.
There was a problem hiding this comment.
Since main is supposed to be the next 3.0.0 it would be allowed to introduce breaking changes :D
But of course we could keep it like this and change the test case.
There was a problem hiding this comment.
Created a story #1146 and looked into the required changes. If it's OK, I can spend time tomorrow.
abelsromero
force-pushed
the
issue-1143-expose-document-source-and-source_lines
branch
from
4d64810
to
c33af8f
Compare
|
Btw, I assume this is fine going to 3.0.0? Or a 2.5.8 is planned? No issue at all backporting it if necessary. |
|
Yes, main would be 3.0.0. |
If people do not ask for it, I would not do it. |
|
Changes done, anything else needs improving? 👀 |
|
Thank you! Looks good! |
abelsromero
deleted the
issue-1143-expose-document-source-and-source_lines
branch
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?
Expose Document's
sourceandsource_linesproperties.This is required to be able to use other asciidoctor components like asciidoctor-reducer.
How does it achieve that?
Using already present AsciidoctorJ APIs and practices, nothing special.
Are there any alternative ways to implement this?
Not that I know.
Are there any implications of this pull request? Anything a user must know?
No
Issue
If this PR fixes an open issue, please add a line of the form:
Fixes #1143
Release notes
Please add a corresponding entry to the file CHANGELOG.adoc