-
Notifications
You must be signed in to change notification settings - Fork 46
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
Get rid of Link_def node #217
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Why is it no longer useful? Isn't it good to have the link definitions in the AST to disambiguate between the two uses of links? Especially if we have an AST -> markdown printer (you probably want to preserve the defs in that case). |
It is no longer useful because reference links are resolved at parsing time since #209. Since we do not keep a reference to the link definition in the AST, having the link definition node does not serve any purpose. There is the larger question of how lossy/lossless do we want the AST to be. Lately I have been leaning towards making the AST easier to use (and so more lossy). Preserving link reference and definition nodes allows to reconstruct the original markdown, but makes the AST harder to use, as the user needs to resolve link references.
Indeed, if we add such a printer, it won't be able to recreate link references, and it will just inline the link. I am not too worried about this (but opinions welcome). The only use I can think of for the AST->markdown printer (apart from pretty printing) is for generating markdown programmatically; the use of link references/definitions does not seem to be crucial for this. |
As you observed, another case of lossy-ness in the AST is that we do not record which delimiter was used for emphasis/strong emphasis ( |
I understand you want to reduce complexity, but the case can be made that link definitions are semantically different (so can be treated differently in a particular printer). About the star or underscore emphasis a case can be made that there is no difference, but it can be pretty handy for a printer to allow supporting a richer feature set while not extending the syntax. These add a bit of complexity to the AST, but it can also easily be ignored by printers? An addition that I don't really understand is the attributes that are added to the AST. |
This is an extension of the standard used by Pandoc which is quite handy to enrich the AST with custom metadata. In the HTML printer, they correspond to usual attributes. See https://github.com/jgm/commonmark-hs/blob/master/commonmark-extensions/test/attributes.md for the spec. |
Ah ok thanks for referencing, I was wondering where that was coming from. That can definitely be handy indeed. |
Thanks for the feedback re: keeping link references in the AST. I want to mull it over, but I see that it could be useful in some cases. |
As observed by @shonfeder, the
Link_def
node is no longer useful in the AST. Let's remove it.