asciidoctor / asciidoctor-diagram

:left_right_arrow: Asciidoctor diagram extension, with support for AsciiToSVG, BlockDiag (BlockDiag, SeqDiag, ActDiag, NwDiag), Ditaa, Erd, GraphViz, Mermaid, Msc, PlantUML, Shaape, SvgBob, Syntrax, UMLet, Vega, Vega-Lite and WaveDrom.
http://asciidoctor.org
MIT License
442 stars 107 forks source link

Block processor should honor the subs attributes #105

Closed mojavelinux closed 8 years ago

mojavelinux commented 8 years ago

In AsciiDoc, it's possible to modify the substitutions that get applied to a block. Most notably, this feature allows attribute references to be resolved inside of a literal / listing block. This is especially useful for diagram reuse and dynamic text. The block processor in Asciidoctor Diagram should honor this option.

As a test case, ensure that the attribute references are resolved in the following example:

:parent-class: Parent
:child-class: Child

[plantuml,class-inheritence,png,subs=attributes+]
....
class {parent-class}
class {child-class}
{parent-class} <|-- {child-class}
....

There are various substitution helpers on the subs class, which is mixed into the parent node (or any created node). See https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/substitutors.rb.

pepijnve commented 8 years ago

@mojavelinux any recommendations on how to test this? My unit tests so far have been black box tests that run Asciidoctor and check that the resulting AST is correct. The diagram text gets converted to an image which makes it difficult to check for correctness of the result automatically.

I'm thinking of generating SVG for this particular case and the checking if the resulting SVG files contains the correct strings. Better suggestions are more than welcome.

pepijnve commented 8 years ago

Should substitutions also be applied to the contents of the target of diagram block macros? Does it for instance make sense to write something like

plantuml::classdiagram.puml[subs=attributes]
pepijnve commented 8 years ago

The logic to derive the set of substitutions to apply seems to be located in Block#initialize. Is the best way to apply the subs to create a temporary Block instance passing in the correct parameters and then calling Block#content on it?

mojavelinux commented 8 years ago

Should substitutions also be applied to the contents of the target of diagram block macros?

Yes, I think so.

I'm thinking of generating SVG for this particular case and the checking if the resulting SVG files contains the correct strings.

My thoughts exactly.

Is the best way to apply the subs to create a temporary Block instance passing in the correct parameters and then calling Block#content on it?

In this case, a better approach is to use the low-level apply_subs method on any node. See https://github.com/asciidoctor/asciidoctor/blob/master/lib/asciidoctor/block.rb#L118 for example usage.

mojavelinux commented 8 years ago

Another example of where apply_subs is used is in the docinfo logic.

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

That's a perfect corollary for what you are doing in Asciidoctor Diagram.

pepijnve commented 8 years ago

Changeset a62837503d5b4ac6232d7cb56ce07ad59c3749c0 changes internal API a bit. The constructors of the source implementations take an extra parameter now. Do we care about semver here? The only user of this I know of is (maybe) the Cacoo integration that was a while back. Seems kind of absurd to have to jump to 2.0 for this...

mojavelinux commented 8 years ago

It's up to you. We haven't officially committed to semver in the Asciidoctor ecosystem yet, so I wouldn't worry about it until we do (we plan to have 1.6.0 core set this precedent).

pepijnve commented 8 years ago

It'll be 1.4.1 then or maybe 1.5 since this a new feature. I think it's more useful to apply semver (backwards (in)compatible) changes in terms of the asciidoc interface that the extension provides rather than the internal code interfaces. The code hasn't really been designed to be a software library.

pepijnve commented 8 years ago

BTW, I've defaulted to no substitutions. Seems like the option that will cause the least surprise for users. A few of the diagram syntaxes use { and } as well. If we default to attribute substitution people might get unexpected results.

mojavelinux commented 8 years ago

I agree. There's definitely an expectation that the diagram source is treated as is by default. It's almost identical to a listing/literal block, except that both of those use two substitutions intended for the output (specialchars and callouts). Since neither of those apply to a diagram, no subs is the most logical and intuitive default.

mojavelinux commented 8 years ago

:+1:

charlottedeclercq commented 8 years ago

Hi,

I have the 1.5.4 version and I am trying to render this plantuml diagram:

:parent-class: Parent :child-class: Child

[plantuml,class-inheritence,png,subs=attributes+] .... class {parent-class} class {child-class} {parent-class} <|-- {child-class} ....

But it doesn't work.

What I am doing wrong ?

Thanks,

Charlotte

pepijnve commented 8 years ago

@charlottedeclercq could you describe in a bit more detail what isn't working exactly? I tried this out locally and it does seem to work as intended for me.

charlottedeclercq commented 8 years ago

I figure dit out, it was my mistake I thought 1.5.2 designed asciidoctor version. I upgraded asciidoctor-diagram and it works perfectly ! Sorry I am quite new to all this stuff…

Thank you for your answer.