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

Fix Macro APIs to return Structural or Phrase Nodes instead of java.l… #1084

Merged
merged 2 commits into from
Aug 20, 2022

Conversation

robertpanzer
Copy link
Member

…ang.Object.

Kind of change

  • Bug fix
  • New non-breaking feature
  • New breaking feature
  • Documentation update
  • Build improvement

Description

What is the goal of this pull request?

As @melix had to experience painfully this week the macro APIs are not really showing what type of node an extension has to return. Currently both InlineMacroProcessors and BlockMacroProcessors define java.lang.Object as the return type.
Instead of parameterizing the return type of the method I parameterized the parameter for the parent node 🤦
I don't know what I had drunk when looking over that.

This PR tries to update the signatures so that it's clearer for the user what type of nodes they have to return.

How does it achieve that?

It updates the InlineMacroProcessor.process() to return a PhraseNode instead of java.lang.Object and BlockMacroProcessor.process() to return StructuralNode.

@mojavelinux Is it even possible that anything else than a StructuralNode/AbstractBlock is passed as a parent to an Inline- or BlockMacroProcessor?
We currently have the topmost base class ContentNode as the type of the parent parameter.
If not I could remove that type parameter.

Are there any alternative ways to implement this?

I don't think so.

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

This is a breaking change. The source code of existing extensions must be updated.

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

@robertpanzer
Copy link
Member Author

If I read the code correctly in substitutors.rb the parent passed to an InlineMacro extension should always be a Block or a Section, that is a StructuralNode, instead of the more generic ContentNodes.
Therefore I changed the signature correspondingly and both Block- and InlineMacros now receive StructuralNodes as the first parameter.
Also added a description about required changes to the migration guide.

@mojavelinux
Copy link
Member

If I read the code correctly in substitutors.rb the parent passed to an InlineMacro extension should always be a Block or a Section, that is a StructuralNode

That's correct.

@robertpanzer robertpanzer merged commit 3d1d047 into asciidoctor:main Aug 20, 2022
@robertpanzer robertpanzer deleted the macro-apis branch August 20, 2022 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants