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

Syntax extension API: allow to create extension nodes #89

Merged
merged 1 commit into from
Sep 13, 2018
Merged

Syntax extension API: allow to create extension nodes #89

merged 1 commit into from
Sep 13, 2018

Conversation

nojb
Copy link

@nojb nojb commented Feb 22, 2018

Currently it is not possible to create nodes defined by syntax extensions using the C API if they use the opaque field (e.g. the "table" extension) because there is no way to correctly allocate this field without exposing the details of the syntax extension in question. Concretely this means that it is not possible to programmatically create documents containing syntax extension nodes.

This PR adds a field opaque_alloc_functo the cmark_syntax_extension struct of type

typedef void (*cmark_opaque_alloc_func) (cmark_syntax_extension *extension,
                                         cmark_mem *mem,
                                         cmark_node *node);

which is supposed to fill the opaque field of a cmark_node in those syntax extensions that need it.

Since this field must be allocated when creating the cmark_node, the function cmark_node_new has been extended to take a cmark_syntax_extension * argument:

CMARK_EXPORT cmark_node *cmark_node_new(cmark_node_type type,
                                        cmark_syntax_extension *extension);

(a new function could be introduced for this purpose to avoid changing the signature of cmark_node_new.)

What do you think? Do you find this approach reasonable? Opinions welcome!

Thanks!

@kivikakk
Copy link

This PR adds a field opaque_alloc_functo the cmark_syntax_extension

I think this is great!

(a new function could be introduced for this purpose to avoid changing the signature of cmark_node_new.)

From an API design perspective, I think this would be best.

Thank you so much for the PR! I'd be happy to merge with this last change, and whatever else is needed to get the tests passing again. (Looks like it's the cmark_node_new signature change, and maybe some new warnings re: redefining cmark_syntax_extension?)

@nojb
Copy link
Author

nojb commented Sep 13, 2018

Sorry, I was quite busy for the last couple of months and could not see to this, but I have now cleaned up the patch, and introduced a separate function as per discussion above. All tests pass. Let me know if you have any other observations. Thanks!

@kivikakk
Copy link

Thank you so much! It's looking great 👍

@kivikakk kivikakk merged commit 22d1149 into github:master Sep 13, 2018
@nojb
Copy link
Author

nojb commented Sep 14, 2018

Thanks!

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 this pull request may close these issues.

2 participants