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

Construct an AST programmatically in 0.20 #350

Closed
yannham opened this issue Nov 30, 2023 · 5 comments · Fixed by #424
Closed

Construct an AST programmatically in 0.20 #350

yannham opened this issue Nov 30, 2023 · 5 comments · Fixed by #424

Comments

@yannham
Copy link
Contributor

yannham commented Nov 30, 2023

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 ?

@digitalmoksha
Copy link
Collaborator

@yannham I'm curious, based on #371 it looks like you're building the AST programmatically. Are you still using 0.17, or did you get the latest working?

@yannham
Copy link
Contributor Author

yannham commented Apr 26, 2024

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.

@kivikakk
Copy link
Owner

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!

@yannham
Copy link
Contributor Author

yannham commented Jun 18, 2024

For the record, we finally got back tho this and simply used this small helper (in tweag/nickel#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)

@kivikakk
Copy link
Owner

kivikakk commented Jul 6, 2024

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.

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 a pull request may close this issue.

3 participants