comrak icon indicating copy to clipboard operation
comrak copied to clipboard

Construct an AST programmatically in 0.20

Open yannham opened this issue 2 years ago • 3 comments

Hello,

We use comrak for the Nickel configuration language, in order to parse in-code markdown documentation and assemble it into one markdown file (think of generating the documentation associated to a module).

We are currently using v0.17, and were constructing nodes programmatically like:

            let document = AstNode::from(NodeValue::Document);

            // Our nodes in the Markdown document are owned by this arena
            let arena = Arena::new();

            // The default ComrakOptions disables all extensions (essentially reducing to
            // CommonMark)
            let options = ComrakOptions::default();

            self.markdown_append(0, &arena, &document, &options);
            format_commonmark(&document, &options, out)
                .map_err(|e| Error::IOError(IOError(e.to_string())))?;

            Ok(())

However, as of the latest version, AstNode::from<NodeValue> doesn't exist anymore. We need to provide AstNode with an Ast, which is itself a struct with NodeValue and a Sourcepos. However it seems Ast requires both, including the Sourcepos. Alas, I don't have any Sourcepos, because the node isn't parsed from source but constructed programmatically.

What would be the idiomatic equivalent with comrak v0.20 ?

yannham avatar Nov 30 '23 15:11 yannham

@yannham I'm curious, based on https://github.com/kivikakk/comrak/issues/371 it looks like you're building the AST programmatically. Are you still using 0.17, or did you get the latest working?

digitalmoksha avatar Apr 25 '24 22:04 digitalmoksha

We haven't taken the time to update to latest comrak, but I think one of my teammate got things working by generating dummy positions (with zeros everywhere, probably). FWIW, in Nickel, the position in the AST is more or less isomorphic to an option Option<Span>, which allows to just set None when generating terms programmatically.

yannham avatar Apr 26 '24 07:04 yannham

I'd be happy to make it easier to supply zero sourceposes (or perhaps make the type Optional), and restore AstNode::from<NodeValue> accordingly, fwiw!

kivikakk avatar Apr 28 '24 10:04 kivikakk

For the record, we finally got back tho this and simply used this small helper (in https://github.com/tweag/nickel/pull/1961):

    fn ast_node<'a>(val: NodeValue) -> AstNode<'a> {
        // comrak allows for ast nodes to be tagged with source location. This location
        // isn't need for rendering; it seems to be mainly for plugins to use. Since our
        // markdown is generated anyway, we just stick in a dummy value.
        let pos = comrak::nodes::LineColumn::from((0, 0));
        AstNode::new(std::cell::RefCell::new(Ast::new(val, pos)))
    }

I guess a Default implementation and/or a From implementation could avoid having to write this helper (the helper is short and not an important issue, but the greater benefit and the reason why I opened the issue initially would be to make it explicit in the documentation that doing this is both possible and ok - as long as you only output the markdown I guess)

yannham avatar Jun 18 '24 10:06 yannham

I've merged the old/new From implementations (one of which does essentially what you've listed here), so I'm calling this one fixed for now! Open to further changes that make programmatic AST construction easier.

kivikakk avatar Jul 06 '24 08:07 kivikakk