Skip to content
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

AsciidoctorJ fails when registering a block processor instance while passing the name as a parameter #754

Closed
robertpanzer opened this issue Jan 12, 2019 · 5 comments · Fixed by #757
Assignees
Labels

Comments

@robertpanzer
Copy link
Member

To reproduce:
Register a Block processor passing the name as an argument, i.e. by calling this method:

javaExtensionRegistry.block("yell", new MyBlockProcessor());

Rendering a document fails because Asciidoctor calls the name accessor of the extension when classes are registered.
This error affects AsciidoctorJ 1.6.0 only as previously extension instances were registered to Asciidoctor as instances.
This changed with 1.6.0 as now instances are wrapped in a Ruby class per registration.

@robertpanzer robertpanzer self-assigned this Jan 12, 2019
@mojavelinux
Copy link
Member

Will this require a change in Asciidoctor? (either to fix it or to remove unused code?)

@robertpanzer
Copy link
Member Author

No, I can fix it in Asciidoctorj.
I’ll check today if there is a case in
https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/extensions.rb that is no longer required by AsciidoctoJ.

@robertpanzer
Copy link
Member Author

I have to admit though I don't fully understand this part of the code:
https://github.com/asciidoctor/asciidoctor/blob/1bf20fc2e8054e7724dc12d0aa26e2a04b871a5a/lib/asciidoctor/extensions.rb#L1397-L1400

First Asciidoctor instantiates the extension passing in the name, and then it calls the accessor again.

@robertpanzer
Copy link
Member Author

While working on this I was wondering if we really want users to implement getName() and setName() on processors.
My understanding is that this was used to interface with Asciidoctor/Ruby as it uses these properties and this bridge called them.
But we should probably deprecate them and require block name registration:

  • explicitly in the call to JavaExtensionRegistry.block(name, (instance|class))
  • Implicitly via the @Name annotation.

As there are proxies between Asciidoctor/Ruby and the users implementation, there is actually no need for these methods anymore.

robertpanzer added a commit to robertpanzer/asciidoctorj that referenced this issue Jan 13, 2019
robertpanzer added a commit to robertpanzer/asciidoctorj that referenced this issue Jan 13, 2019
robertpanzer added a commit to robertpanzer/asciidoctorj that referenced this issue Jan 13, 2019
robertpanzer added a commit that referenced this issue Jan 20, 2019
…essor-instance-with-name

Fixes #754. Initialize BlockProcessor.name correctly
@mojavelinux
Copy link
Member

I have to admit though I don't fully understand this part of the code:
asciidoctor/asciidoctor:lib/asciidoctor/extensions.rb@1bf20fc#L1397-L1400

The name is optional. The implementation can provide it. The implementation must also self-identify with the same name that was used to create. That's all the code is doing there. It's a lot of edge cases. Some may not apply to AsciidoctorJ.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants