asciidoctor / asciidoctorj

:coffee: Java bindings for Asciidoctor. Asciidoctor on the JVM!
http://asciidoctor.org
Apache License 2.0
618 stars 172 forks source link

Attributes in parts not supported? #139

Open magnars opened 10 years ago

magnars commented 10 years ago

Calling getAttributes on the header returns a map of attributes, like expected.

Calling getAttributes on a part returns this:

#<RubySymbol attribute_entries> [#<RubyObject #<Asciidoctor::Document::AttributeEntry:0x769bd926>>]

Am I doing something wrong?

lordofthejars commented 10 years ago

I don't think you are doing nothing wrong I will take a look but I think that your part contains as attribute the parent document which is a complex type and JRuby tries to proxy it. Can you provide a simple test?

El dimecres 15 de gener de 2014, Magnar Sveen ha escrit:

Calling getAttributes on the header returns a map of attributes, like expected.

Calling getAttributes on a part returns this:

[#<RubyObject #>]

Am I doing something wrong?

— Reply to this email directly or view it on GitHubhttps://github.com/asciidoctor/asciidoctorj/issues/139 .

Enviat amb Gmail Mobile

magnars commented 10 years ago

This is using a StructuredDocument.

= My document

== Part 1

Hello

== Part 2

Hello again

Now doc.getHeader().getAttributes() is a map of lots of vars. And doc.getParts()[0].getAttributes() is empty.

Adding

:my-var: custom!

to Part 1, I can access it through the header:

doc.getHeader().getAttributes().get("my-var"); // => "custom!"

but I can't access it through the part:

doc.getParts()[0].getAttributes().get("my-var"); // nope

because getAttributes on a part does not return a Map<String, String>, but has these types instead (inspected with toString):

#<RubySymbol attribute_entries> [#<RubyObject #<Asciidoctor::Document::AttributeEntry:0x769bd926>>]
mojavelinux commented 10 years ago

This has to do with something called "playback attribute entries". After parsing is complete, all attributes after the header are put back on the proverbial shelf (hence the unexpected object type). Then, as each block is rendered, its attributes are "played back" in turn. To get the effective attributes at a point in the document, you have to playback the attributes leading up to it.

The reason Asciidoctor does this has to do with how AsciiDoc is implement. AsciiDoc renders as it parses, whereas Asciidoctor uses two phases. Since attributes can be modified mid-document, we need to emulate that timeline.

We need to reason about the best way to allow access to attributes at an arbitrary point in the document after the parse phase, but before rendering. I think the answer is to snapshot the attributes at each nice instead of rolling them back and forward again. We need to find a way to store them using a persistent map (for efficiency, see Clojure).

mojavelinux commented 10 years ago

Since Asciidoctor Ruby experiences this same issue, we should probably address it upstream.

A quick way to get the attributes for the part is to iterate the nodes up to that part and call playback_attributes on them. You'll need to restore them afterwards in the same way that Asciidoctor does it.

See https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/document.rb#L631

lordofthejars commented 10 years ago

Thanks, yes I will need to do something similar to get all attributes. It is funny because this part of retrieving data from document was a collaboration of AsciidoctorJ user and it is been used by a lot of people, so it was a use case that I completely missed and was implemented by community. So happy about this feedback.

2014/1/21 Dan Allen notifications@github.com

Since Asciidoctor Ruby experiences this same issue, we should probably address it upstream.

A quick way to get the attributes for the part is to iterate the nodes up to that part and call playback_attributes on them. You'll need to restore them afterwards in the same way that Asciidoctor does it.

See https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/document.rb#L631

— Reply to this email directly or view it on GitHubhttps://github.com/asciidoctor/asciidoctorj/issues/139#issuecomment-32900655 .

+----------------------------------------------------------+ Alex Soto Bueno - Computer Engineer www.lordofthejars.com +----------------------------------------------------------+

lordofthejars commented 10 years ago

@mojavelinux So internally when I want to get attributes from a Part first I should call the playback_attributes from document method. Is this right?

mojavelinux commented 9 years ago

Correct. But we really want to move away from that in core because it's not a pretty solution.

lordofthejars commented 9 years ago

yes now it is a way to get data eagerly. we need to inspect if we can do the same currently (which I think so), and then plan when to remove this feature, which it may be a problem for example for spring people because I know they are currently using it.

2014-09-19 4:13 GMT+02:00 Dan Allen notifications@github.com:

Correct. But we really want to move away from that in core because it's not a pretty solution.

— Reply to this email directly or view it on GitHub https://github.com/asciidoctor/asciidoctorj/issues/139#issuecomment-56127902 .

+----------------------------------------------------------+ Alex Soto Bueno - Computer Engineer www.lordofthejars.com +----------------------------------------------------------+

mojavelinux commented 9 years ago

Ideally we'll handle the transition transparently. That's why I recommend putting it behind some sort of interface that handles the "playback" and then in the future we just won't have to do the playback but just read the attributes. I'm going to open an issue in Asciidoctor core (if it's not already there) and expand a bit more on my plan for this change. It will probably land in 1.6.0 or 1.7.0.

lordofthejars commented 9 years ago

I think you have already opened one related with this kind of information but maybe it was only related and not exactly this one.

2014-09-19 9:27 GMT+02:00 Dan Allen notifications@github.com:

Ideally we'll handle the transition transparently. That's why I recommend putting it behind some sort of interface that handles the "playback" and then in the future we just won't have to do the playback but just read the attributes. I'm going to open an issue in Asciidoctor core (if it's not already there) and expand a bit more on my plan for this change. It will probably land in 1.6.0 or 1.7.0.

— Reply to this email directly or view it on GitHub https://github.com/asciidoctor/asciidoctorj/issues/139#issuecomment-56145298 .

+----------------------------------------------------------+ Alex Soto Bueno - Computer Engineer www.lordofthejars.com +----------------------------------------------------------+

mojavelinux commented 9 years ago

It's related to https://github.com/asciidoctor/asciidoctor/issues/692, but I'm going to open a separate issue to deal with how document-level attributes are stored in the document. In short, they should be stored as nodes in the tree and also reified when each block is created so that they don't have to be played back when viewing the structure of the document.