Failure getting `blocks` when using description lists

[someth2say]

See https://asciidoctor.zulipchat.com/#narrow/stream/279644-users.2Fasciidoctorj/topic/Failure.20getting.20.60blocks.60.20when.20using.20description.20lists

Seems that DescriptionListImpl.blocks returns a RubyArrayTwoObject, instead of a List<StructuralNode>. This makes a bunch of processing steps more complicated and error-prone. For example, this Treeprocessor:

   public static class SolutionsProcedureTreeprocessor extends Treeprocessor {
        public SolutionsProcedureTreeprocessor() {}
        @Override
        public Document process(Document document) {
            processBlock((StructuralNode) document, 0);
            return document;
        }

        private void processBlock(StructuralNode block, int deep) {
            List<StructuralNode> blocks = block.getBlocks();
            for (int i = 0; i < blocks.size(); i++) {
                final StructuralNode currentBlock = blocks.get(i);
                if (currentBlock instanceof StructuralNode) {
                    System.out.println(" ".repeat(deep) + currentBlock.getClass());
                    processBlock(currentBlock, ++deep);
                }
            }
        }
    }

Fails when trying to invoke block.getBlocks() because the block is not a StructuralNode, but a RubyArrayTwoObject:

org.asciidoctor.jruby.internal.AsciidoctorCoreException: org.jruby.exceptions.NoMethodError: (NoMethodError) asciidoctor: FAILED: nav.adoc: Failed to load AsciiDoc document - undefined method `blocks' for #<Array:0xfb0dc3ba>

Proposal: Overwrite the DescriptionListImpl.blocks method so it returns a flattened list of nodes.

[abelsromero]

We should think this through and maybe we don't want to follow same approach as in Ruby core. I personally have doubts of the current structure, for instance why the terms is a list.

[mojavelinux]

I agree that we don't need to follow the structure used by Asciidoctor Ruby. The structure it uses was selected quickly in the early days of Asciidoctor and never revisited. It piggy backed off of the existing List / ListItem types and is very low-level for the purpose of speed and efficiency. But that doesn't make it right. I've always felt that we needed to remodel that block type eventually.

[robertpanzer]

I just compared to Asciidoctor.js and it seems like it returns the list items on DescriptionList.getBlocks(); However, if I didn't do anything wrong, the ListItems are not Blocks so it results in a similar problem ~one level deeper~. It's actually the same problem.

I'll see if I can fix it in a similar way to Asciidoctor.js.

for instance why the terms is a list.

What is a better way to model a list item with multiple terms? https://github.com/asciidoctor/asciidoctorj/blob/c521b1c3a59474425867501df919fea3cd9f8f14/asciidoctorj-core/src/test/groovy/org/asciidoctor/WhenADocumentContainsADefinitionList.groovy#L29-L30

[robertpanzer]

I vaguely remembered that we had already touched this in the past and found this :) https://github.com/asciidoctor/asciidoctorj/pull/408#issuecomment-165699044

[robertpanzer]

Since a DescriptionListEntry, the item, is not a StructuralNode it's probably best to return an empty list.

[abelsromero]

Since a DescriptionListEntry, the item, is not a StructuralNode it's probably best to return an empty list.

How would a user traverse the whole tree then? I see no generic "getChildren()"

[robertpanzer]

Would it help if we made all interfaces generic and added a getChildren() returning the blocks or the items? I am unsure if this is really ergonomic:

public interface StructuralNode<T> {
  public List<T> getChildren();
}
public interface Block extends StructuralNode<StructuralNode> {}
public interface DescriptionList extends StructuralNode<DescriptionListEntry> {}

I am unsure if this is sth anybody would like to program against, given that StructuralNode and DescriptionListEntry have no commonalities (except what java.lang.Object has).

[abelsromero]

I am aware it's not trully an AST due to how things are parsed and I don't have enough context yet, but being able to traverse the whole data seems a requirement. Maybe we can find a compromise here and start discussing future changes for v3.0.0?