You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Add more information to Link nodes (for round-trip rendering) #98
For #12, we need to preserve link reference definitions. We also need to be able to distinguish between the different link types (inline links, autolinks, reference links).
There is an old PR which might serve as a starting point: #20 (comment)
I'm not sure yet what the final Node types should look like. Ideally we'd have an interface that all the links can implement for code that doesn't care about what the source representation was.
If someone is interested in working on this, please comment here first so we can figure out what the end result should look like.
I've been looking this over since last night and I'll have some time on various Saturdays in the next few months that might serve to implement this. In the mean time: talk is cheap! :)
I've reviewed #20, #12 and the appropriate spec. Regarding your notes in #20
Do you think it would make sense to have
[foo]: /url "title"
be its own Node type, like ReferenceDefinition? Something like this maybe:
[foo](/uri "title")
is a Link, which has getDestination, getTitle (not sure about getLabel)
[foo][foo], [foo][] and [foo]
are ReferenceLink extends Link, which have an additional getDefinition which returns the ReferenceDefinition
plus the need for a generic link interface, I was thinking this:
Autolinks only have linkDestinations, which calls into question a functional need for inheritance to Link.
public class Autolink extends Node implements GenericLink {
private String linkDestination;
interface impl just refers to linkDestination...
}
ReferenceDefinitions do break down to exactly the same components as Link, so inheritance is worthwhile from a reduction of code duplication standpoint but I don't know if a ReferenceDefinitionis a Link. So straight inheritance, a class just for type purposes:
public class ReferenceDefinition extends Link implements GenericLink {
not much here...
}
or compositional inheritance:
public class ReferenceDefinition extends Node implements GenericLink {
private Link linkReferenced;
methods call through to linkReferenced..
}
^edit: nope. Don't like that idea after I've thought about it.
or a complete duplicate/copy of Link's functionality. An inheritance decision here might also influence Autolink's implementation.
Then, we'll need something that references the ReferenceDefinition. From the spec:
A link reference definition does not correspond to a structural element of a document. Instead, it defines a label which can be used in reference links...
Emphasis mine, because I've thought of this as a ReferenceLabel
public class ReferenceLabel extends Node {
private ReferenceDefinition referenceDefinition;
...
}
I'm happy to help, bud! Thanks for doing lots of the heavy lifting on this project.
Link as name of interface - I have no problem with that. The purpose of GenericLink was solely to disambiguate it from org.commonmark.node.Link.
Link is implemented by all nodes that result in a rendered link - Good note; makes sense.
getLabel in the interface - Link labels are optional in inline links - Link spec refers to them as link text. Currently in commonmark-java, a Link node has a destination and title and optionally contains a Text child node for link text. A LinkReferenceDefinition as proposed would be implemented similarly.
So, to distill: do you think that the link label of a link reference definition equates to the link text of an inline link?
getLabel may not be the best choice for the interface method name since we're talking about building a link from this object. getLinkText or getText might be more in line with the nature of the work.
Yeah, I see what you're saying. I think the ReferenceLink would have foo as child node(s). But it would also be useful to just have a String representation of it, which allows you to look up the corresponding LinkReferenceDefinition from a Map. Calling that getLabel would be fine. But it wouldn't be on the generic Link interface because not all links have a label.
Anyway, I think as soon as someone starts implementing this, it should become clear and we can discuss details on the PR.
changed the title [-]Preserve link reference definitions as Node & add more information to Link[/-][+]Add more information to Link nodes (for round-trip rendering)[/+]on Jul 12, 2019
Activity
jonsampson commentedon Sep 16, 2017
Hi @robinst,
I've been looking this over since last night and I'll have some time on various Saturdays in the next few months that might serve to implement this. In the mean time: talk is cheap! :)
I've reviewed #20, #12 and the appropriate spec. Regarding your notes in #20
plus the need for a generic link interface, I was thinking this:
Autolinks only havelinkDestinations, which calls into question a functional need for inheritance to Link.ReferenceDefinitions do break down to exactly the same components asLink, so inheritance is worthwhile from a reduction of code duplication standpoint but I don't know if aReferenceDefinitionis aLink. So straight inheritance, a class just for type purposes:or compositional inheritance:public class ReferenceDefinition extends Node implements GenericLink {private Link linkReferenced;
methods call through to linkReferenced..
}
^edit: nope. Don't like that idea after I've thought about it.
or a complete duplicate/copy of Link's functionality. An inheritance decision here might also influence
Autolink's implementation.Then, we'll need something that references the
ReferenceDefinition. From the spec:Emphasis mine, because I've thought of this as a
ReferenceLabelIt's not a link itself, just a pointer.
Thanks for reading!
robinst commentedon Sep 20, 2017
Hey! Thanks for picking this up :).
So, a couple of thoughts:
I think
Linkitself should be the name of the interface (notGenericLink)Linkshould be implemented by all nodes that result in a rendered link. So that means the following:Would result in:
[foo]is aReferenceLink ... implements Link. The last line would be aLinkReferenceDefinitionwhich does not implementLink.I'm not sure
getLabelbelongs on the interface. Link labels are only used for reference links, but not inline links.Does that make sense?
jonsampson commentedon Sep 21, 2017
I'm happy to help, bud! Thanks for doing lots of the heavy lifting on this project.
Linkas name of interface - I have no problem with that. The purpose ofGenericLinkwas solely to disambiguate it fromorg.commonmark.node.Link.Linkis implemented by all nodes that result in a rendered link - Good note; makes sense.getLabelin the interface - Link labels are optional in inline links - Link spec refers to them as link text. Currently in commonmark-java, a Link node has adestinationandtitleand optionally contains aTextchild node for link text. ALinkReferenceDefinitionas proposed would be implemented similarly.So, to distill: do you think that the link label of a link reference definition equates to the link text of an inline link?
Inline Link Example 459 - link is link text
[link](/uri "title")
yields <p><a href="/uri" title="title">link</a></p>
Ref Def Link Example 159 - foo is link label
[foo]: /url "title"
[foo]
yields <p><a href="/url" title="title">foo</a></p>
Proposal: link = foo
getLabelmay not be the best choice for the interface method name since we're talking about building a link from this object.getLinkTextorgetTextmight be more in line with the nature of the work.So now, having written this, I've stumbled across http://spec.commonmark.org/0.28/#reference-link, a sub part of the Link section, which could further inform on how I should visualize this. I hadn't realized there are full reference links, collapsed reference links, and shortcut reference links (which appear to be what I was modelling this stuff based upon). I've got some more reading to do!
robinst commentedon Oct 21, 2017
Yeah, I see what you're saying. I think the
ReferenceLinkwould havefooas child node(s). But it would also be useful to just have a String representation of it, which allows you to look up the correspondingLinkReferenceDefinitionfrom a Map. Calling thatgetLabelwould be fine. But it wouldn't be on the genericLinkinterface because not all links have a label.Anyway, I think as soon as someone starts implementing this, it should become clear and we can discuss details on the PR.
Adapt link reference definition parsing to spec changes
Add LinkReferenceDefinition nodes into document
[-]Preserve link reference definitions as Node & add more information to Link[/-][+]Add more information to Link nodes (for round-trip rendering)[/+]robinst commentedon Jul 12, 2019
The first part of this is now done, see cb43ae9.
robinst commentedon Apr 6, 2024
With the
MarkdownRenderernow merged:The goal of this issue is now nice and clear: Preserve enough details about links in nodes to be able to render them back in their original format.
roxspring commentedon Sep 2, 2026
See #455 for an attempt at addressing the link related round-trip experience.