upgrade Asciidoctorj to 1.5.4.1 #258

ggrossetie · 2016-08-10T11:54:41Z

Resolves #257

ggrossetie · 2016-08-10T12:03:56Z

Tests are failing:

org.asciidoctor.internal.AsciidoctorCoreException: org.jruby.exceptions.RaiseException: (NoMethodError) undefined method `level' for nil:NilClass
    at org.asciidoctor.internal.JRubyAsciidoctor.renderFile(JRubyAsciidoctor.java:345)
    at org.asciidoctor.maven.AsciidoctorMojo.renderFile(AsciidoctorMojo.java:396)
    at org.asciidoctor.maven.AsciidoctorMojo.execute(AsciidoctorMojo.java:220)
    at org.asciidoctor.maven.test.AsciidoctorMojoExtensionsTest.successfully renders html with Preprocessor, DocinfoProcessor, InlineMacroProcessor and IncludeProcessor(AsciidoctorMojoExtensionsTest.groovy:340)
Caused by: org.jruby.exceptions.RaiseException: (NoMethodError) undefined method `level' for nil:NilClass
    at RUBY.outline(/home/travis/.m2/repository/org/asciidoctor/asciidoctorj/1.5.4.1/asciidoctorj-1.5.4.1.jar!/gems/asciidoctor-1.5.4/lib/asciidoctor/converter/html5.rb:300)
    at RUBY.document(/home/travis/.m2/repository/org/asciidoctor/asciidoctorj/1.5.4.1/asciidoctorj-1.5.4.1.jar!/gems/asciidoctor-1.5.4/lib/asciidoctor/converter/html5.rb:171)
    at RUBY.convert(/home/travis/.m2/repository/org/asciidoctor/asciidoctorj/1.5.4.1/asciidoctorj-1.5.4.1.jar!/gems/asciidoctor-1.5.4/lib/asciidoctor/converter/base.rb:34)
    at RUBY.convert(/home/travis/.m2/repository/org/asciidoctor/asciidoctorj/1.5.4.1/asciidoctorj-1.5.4.1.jar!/gems/asciidoctor-1.5.4/lib/asciidoctor/document.rb:1044)
    at RUBY.convert(/home/travis/.m2/repository/org/asciidoctor/asciidoctorj/1.5.4.1/asciidoctorj-1.5.4.1.jar!/gems/asciidoctor-1.5.4/lib/asciidoctor.rb:1503)
    at RUBY.convert_file(/home/travis/.m2/repository/org/asciidoctor/asciidoctorj/1.5.4.1/asciidoctorj-1.5.4.1.jar!/gems/asciidoctor-1.5.4/lib/asciidoctor.rb:1576)
    at RUBY.convertFile(<script>:68)
    at org.jruby.gen.InterfaceImpl1642706835.convertFile(org/jruby/gen/InterfaceImpl1642706835.gen:13)
    ... 1 more

An error is thrown at this line: https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/converter/html5.rb#L300

slevel = (first_section = sections[0]).level

sections[0] resolves to nil and level method obsiously does not exist on NilClass.
I didn't find the root cause of this. @robertpanzer any idea ?

robertpanzer · 2016-08-10T15:22:35Z

Strange.
It seems to go back to this test always returning false: https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/abstract_block.rb#L204

This is actually perfectly possible if multiple Asciidoctor instance come into play.
I just wonder why this doesn't occur with 1.5.3.

If I fix this locally (by comparing the string representations of the symbols (Please don't beat me ;-) ) I stumble over the next problem:

Aug 10, 2016 5:18:45 PM org.asciidoctor.internal.JRubyAsciidoctor renderFile
SCHWERWIEGEND: (NoMethodError) undefined method `caption' for #<Java::OrgAsciidoctorAst::SectionImpl:0x7d8b66d9>

org.asciidoctor.internal.AsciidoctorCoreException: org.jruby.exceptions.RaiseException: (NoMethodError) undefined method `caption' for #<Java::OrgAsciidoctorAst::SectionImpl:0x7d8b66d9>
    at org.asciidoctor.internal.JRubyAsciidoctor.renderFile(JRubyAsciidoctor.java:345)
    at org.asciidoctor.maven.AsciidoctorMojo.renderFile(AsciidoctorMojo.java:396)
    at org.asciidoctor.maven.AsciidoctorMojo.execute(AsciidoctorMojo.java:220)
    at org.asciidoctor.maven.test.AsciidoctorMojoExtensionsTest.tests that a #processorType is registered, initialized and executed(AsciidoctorMojoExtensionsTest.groovy:119)
Caused by: org.jruby.exceptions.RaiseException: (NoMethodError) undefined method `caption' for #<Java::OrgAsciidoctorAst::SectionImpl:0x7d8b66d9>
    at RUBY.outline(/Users/robertpanzer/.m2/repository/org/asciidoctor/asciidoctorj/1.5.5-SNAPSHOT/asciidoctorj-1.5.5-SNAPSHOT.jar!/gems/asciidoctor-1.5.4/lib/asciidoctor/converter/html5.rb:305)
    at org.jruby.RubyArray.each(org/jruby/RubyArray.java:1613)
    at RUBY.outline(/Users/robertpanzer/.m2/repository/org/asciidoctor/asciidoctorj/1.5.5-SNAPSHOT/asciidoctorj-1.5.5-SNAPSHOT.jar!/gems/asciidoctor-1.5.4/lib/asciidoctor/converter/html5.rb:304)

Will investigate further.

mojavelinux · 2016-08-10T18:27:21Z

That's really weird because it means it gets past the following check:

return unless node.sections?

However, if we look at the implementation of the sections? method, we see where things might be going wrong:

def sections?
  @next_section_index > 0
end

So the @next_section_index doesn't get reset somehow.

mojavelinux · 2016-08-10T18:37:02Z

If I fix this locally (by comparing the string representations of the symbols (Please don't beat me ;-) ) I stumble over the next problem:

Is it possible that AsciidoctorJ is changing the symbol to a string? This would indicate that AsciidoctorJ is mutating the AST unexpectedly, which would indicate a bigger problem.

Take a look at assign_index method and make sure the context is always a Symbol :section and not a String section. That will tell us what we need to know.

robertpanzer · 2016-08-10T18:58:04Z

I think that’s a Bingo.

We do not intentionally convert the context symbol to a String, but when iterating over the Blocks they are converted to the Java counterparts and replace the original Ruby objects.
(And inserting puts into the Ruby code shows that the blocks are Java objects)

Now Asciidoctor Ruby calls getContext() on the Java Object, and that one returns a String and not a Symbol which would be useless in the Java API.

But still not sure, where this comes from in 1.5.4.1 and why it doesn’t happen in 1.5.3.2.

Am 10.08.2016 um 20:37 schrieb Dan Allen [email protected]:

If I fix this locally (by comparing the string representations of the symbols (Please don't beat me ;-) ) I stumble over the next problem:

Is it possible that AsciidoctorJ is changing the symbol to a string? This would indicate that AsciidoctorJ is mutating the AST unexpectedly, which would indicate a bigger problem.

Take a look at assign_index method and make sure the context is always a Symbol :section and not a String section. That will tell us what we need to know.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #258 (comment), or mute the thread https://github.com/notifications/unsubscribe-auth/ABHBjg5-NBIn6XMLGC69HW3ABDylFFpxks5qehpPgaJpZM4JhEKE.

mojavelinux · 2016-08-10T18:59:14Z

Perhaps this is related somehow to asciidoctor/asciidoctorj#392 (adding an empty TreeProcessor in AsciidoctorJ 1.5.x kills the TOC).

robertpanzer · 2016-08-10T19:10:02Z

Completely strange.

I just compared the changes in AsciidoctorJ between 1.5.3.2 and 1.5.4.1 and it's mostly build stuff, the addition of support for Tables in the AST and that's it besides additional tests :-(

mojavelinux · 2016-08-10T19:18:22Z

Did the version of JRuby change?

robertpanzer · 2016-08-10T19:19:12Z

It's nailed to 1.7.21 in the maven plugin

Am Mittwoch, 10. August 2016 schrieb Dan Allen :

Did the version of JRuby change?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#258 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABHBjrNuIV1XBLr1kYaf5ZlVlqLrbTioks5qeiP_gaJpZM4JhEKE
.

robertpanzer · 2016-08-11T05:25:20Z

Another interesting observation:
I just retested with current master (as I did yesterday) but nailed the version of the asciidoctor gem to 1.5.3. (So like an AsciidoctorJ 1.5.4.1 using Asciidoctor 1.5.3)
.... And tests pass.

So it seems to be an unlucky combination of some changes in Asciidoctor and the fact that AsciidoctorJ mixes up the context symbols.

robertpanzer · 2016-08-11T08:05:41Z

Is it possibly related to asciidoctor/asciidoctor#1637 ?

mojavelinux · 2016-08-11T08:52:28Z

That definitely looks like the culprit. You could monkeypatch the sections? method. However, I suspect that if the context is being converted from a Symbol to a String, I'd expect there to be errors in other places. Now that we understand the reason for the change between versions, I think the key is to focus on how to avoid converting the context to a string (or reverting it back at a strategic point).

robertpanzer · 2016-08-11T09:02:19Z

I can't imagine that returning Symbols instead of Strings is possible.
It should be solved with 1.6.0 because we no longer let Java objects shine into the Ruby world.

I already tried to fix it by comparing the context.to_s with "section" and it complained afterwards about getCaption() not implemented. :-)

Monkey patching the original code seems awkward, but maybe the only possibility with 1.5.x.

mojavelinux · 2016-08-11T09:05:44Z

I can't imagine that returning Symbols instead of Strings is possible.

I totally agree. But that's not what I'm concerned about. What I'm hoping for is the internal state of the AST to remain unchanged. When Ruby executes .context, the result should be a Symbol (while still in Ruby).

robertpanzer · 2016-08-11T09:12:09Z

I am not sure why the blocks are already replaced when iterating over them, but at latest when creating new sections in a processor it will be a pure Java object returning a String.
If creation of the doc wouldn't fail then, it would miss the created section.

Will try to monkey patch this on master.

mojavelinux · 2016-08-11T09:14:18Z

when creating new sections in a processor it will be a pure Java object returning a String.

I'm okay with saying that creating new sections from a TreeProcessor in AsciidoctorJ 1.5.x while using the TOC is an unsupported scenario (instead, it requires AsciidoctorJ 1.6.x). I think the focus here is to get the TOC working when the TreeProcessor does not create new sections.

ggrossetie · 2016-08-11T09:50:01Z

Is Asciidoctorj 1.6.0 (final) coming out soon ?
Version 1.6.0-alpha.3 is working fine, maybe we should align asciidoctor-maven-plugin and asciidoctorj ?

Create a 1.5.x branch
Switch master to 1.6.x

mojavelinux · 2016-08-11T09:59:05Z

Is Asciidoctorj 1.6.0 (final) coming out soon ?

No, there's still a lot of road in front of us. However, a beta could be on the horizon much sooner. I would encourage all early adopters to be using the beta of 1.6.0. Then we need to accelerate core. I've just been a bit overwhelmed at the moment to give that matter the attention it needs.

robertpanzer · 2016-08-12T06:30:21Z

I think the focus here is to get the TOC working when the TreeProcessor does not create new sections.

Not converting the existing Ruby blocks is unfortunately also not an option.
When getBlocks() or blocks() is called from Java the result is expected to be a List of AbstractBlocks. If I used JavaEmbedUtils.rubyToJava() to make the Ruby object mimic the Java interface iterating the child blocks of this "masked" block will no longer return an AbstractBlock.
We are in the pure Ruby world then so that AsciidoctorJ is no longer able to intercept so that the whole thing breaks:

section.getBlocks()  // would return a list of RubyObjectProxyHolders when using JavaEmbedUtils.rubyToJava()
    .get(0)          // Returns a RubyObject implementing AbstractBlock
    .getBlocks()     // Invokes the Ruby method Asciidoctor::AbstractBlock#blocks and returns an List of RubyObjects implementing nothing
    .get(0)          // ClassCastException: org.jruby.RubyObject cannot be cast to org.asciidoctor.ast.AbstractBlock

I think patching the Symbol problem is the only option to go.

mojavelinux · 2016-08-12T06:58:45Z

I think patching the Symbol problem is the only option to go.

I would say yes, but I think that the comparison to :section is only the tip of an iceberg. Could you be more specific about the patch you are proposing?

We might just have to say that you can't create Java-based TreeProcessor extensions with 1.5.x without side effects.

robertpanzer · 2016-08-12T07:05:54Z

I didn't find the time to completely dig through that.
I tried patching it manually and directly stumbled over getCaption() not implemented in AsciidoctorJ.
So I think you're absolutely right that this would be just the tip of the iceberg.

The thing I tried was replacing this:

    @blocks.select {|block| block.context == :section }

with this:

    @block.select(|block| block.context.to_s == 'section' }

I have unfortunately no clue about the performance impact this might have.

I just wonder that we do not have a test case in AsciidoctorJ yet that reveals this problem, that should be the first task imo.

mojavelinux · 2016-08-12T07:06:54Z

The performance implication is extremely minor. That will certainly get the job done.

slachiewicz · 2018-02-11T10:41:57Z

Hi, this pull may already be closed, higher dependency versions are already in the master.

abelsromero · 2018-02-11T10:49:48Z

You are right. Thanks for the heads up

Resolves asciidoctor#257, upgrade Asciidoctorj to 1.5.4.1

b7d1231

This was referenced Aug 12, 2016

Using a Processor makes rendering documents with a TOC fail asciidoctor/asciidoctorj#497

Closed

Fixes #497. Patch Asciidoctor::AbstractBlock#section? in abstract_blo… asciidoctor/asciidoctorj#498

Merged

This was referenced Sep 21, 2016

269 Exclude docinfo files by default #270

Merged

Bump to AsciidoctorJ 1.5.4 #271

Closed

abelsromero closed this Feb 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

upgrade Asciidoctorj to 1.5.4.1 #258

upgrade Asciidoctorj to 1.5.4.1 #258

ggrossetie commented Aug 10, 2016

ggrossetie commented Aug 10, 2016

robertpanzer commented Aug 10, 2016

mojavelinux commented Aug 10, 2016

mojavelinux commented Aug 10, 2016

robertpanzer commented Aug 10, 2016

mojavelinux commented Aug 10, 2016

robertpanzer commented Aug 10, 2016

mojavelinux commented Aug 10, 2016

robertpanzer commented Aug 10, 2016

robertpanzer commented Aug 11, 2016

robertpanzer commented Aug 11, 2016

mojavelinux commented Aug 11, 2016

robertpanzer commented Aug 11, 2016

mojavelinux commented Aug 11, 2016 •

edited

Loading

robertpanzer commented Aug 11, 2016

mojavelinux commented Aug 11, 2016

ggrossetie commented Aug 11, 2016

mojavelinux commented Aug 11, 2016

robertpanzer commented Aug 12, 2016

mojavelinux commented Aug 12, 2016 •

edited

Loading

robertpanzer commented Aug 12, 2016

mojavelinux commented Aug 12, 2016

slachiewicz commented Feb 11, 2018

abelsromero commented Feb 11, 2018

upgrade Asciidoctorj to 1.5.4.1 #258

upgrade Asciidoctorj to 1.5.4.1 #258

Conversation

ggrossetie commented Aug 10, 2016

ggrossetie commented Aug 10, 2016

robertpanzer commented Aug 10, 2016

mojavelinux commented Aug 10, 2016

mojavelinux commented Aug 10, 2016

robertpanzer commented Aug 10, 2016

mojavelinux commented Aug 10, 2016

robertpanzer commented Aug 10, 2016

mojavelinux commented Aug 10, 2016

robertpanzer commented Aug 10, 2016

robertpanzer commented Aug 11, 2016

robertpanzer commented Aug 11, 2016

mojavelinux commented Aug 11, 2016

robertpanzer commented Aug 11, 2016

mojavelinux commented Aug 11, 2016 • edited Loading

robertpanzer commented Aug 11, 2016

mojavelinux commented Aug 11, 2016

ggrossetie commented Aug 11, 2016

mojavelinux commented Aug 11, 2016

robertpanzer commented Aug 12, 2016

mojavelinux commented Aug 12, 2016 • edited Loading

robertpanzer commented Aug 12, 2016

mojavelinux commented Aug 12, 2016

slachiewicz commented Feb 11, 2018

abelsromero commented Feb 11, 2018

mojavelinux commented Aug 11, 2016 •

edited

Loading

mojavelinux commented Aug 12, 2016 •

edited

Loading