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

Add Cell.getRawText() and ListItem.getRawText() to get unsubstituted … #517

Conversation

robertpanzer
Copy link
Member

…text of table cells and list items.

This PR goes for the first point in #513 (comment) :
Add accessors to get the raw, unsubstituted text from a Table Cell or a ListItem.
The methods are each called getRawText() and they get the field @text of the associated nodes instead of invoking the accessor method text on the respective Ruby nodes, as these apply the substitutions.

I am not really happy with the naming of the methods yet, as we have String getContent() and List<String> getLines() on Blocks, but String getText() and String getRawText() on Cells and ListItems.

But the thing that we're even able now to directly access members of the Ruby objects is already a great thing! This approach wouldn't work on 1.5.x without reimplementing all the functionality in the Java nodes in the same way as it works in the Ruby nodes.

@mojavelinux
Copy link
Member

I agree we need to think about the naming. That's one of the reasons I haven't added these methods in core yet. I'd prefer either getSourceText or just getSource. I don't like the term "raw".

@robertpanzer
Copy link
Member Author

Absolutely right.
I renamed getRawText() to getSource().
Then the methods also align with their pendants in org.asciidoctor.ast.Block.

Wonder if it would make sense to put an interface on it containing getContent() and getSource()?

@mojavelinux
Copy link
Member

Those methods probably belong on ContentNode (though we may still have some children that don't yet implement them).

@robertpanzer
Copy link
Member Author

Will merge it now then.

Regarding where to put those methods: Don't these methods only make sense for nodes that are leafs in the AST? Sections or List don't have an own source or content, do they?
This only under the premise that it isn't defined as the source or content collected from the contained subtree.
This would also create problems with the corresponding setSource() methods.

@robertpanzer robertpanzer merged commit a52fe97 into asciidoctor:asciidoctorj-1.6.0 Oct 3, 2016
@mojavelinux
Copy link
Member

That's a good point. They do only apply for leafs at the moment. Though it's reasonable to assume, I suppose, that all nodes should be able to access their source. It's a bit of a fuzzy area in the AST right now.

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.

None yet

2 participants