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

Fixes for the LaTeX renderer #182

Merged
merged 4 commits into from
Jan 7, 2017
Merged

Fixes for the LaTeX renderer #182

merged 4 commits into from
Jan 7, 2017

Conversation

Doeme
Copy link
Contributor

@Doeme Doeme commented Jan 7, 2017

The first fix corrects ligature handling with dashes, (- are manual hyphenations), the second fixes a double-output of urls.

Dominik Schmidt added 2 commits January 7, 2017 17:10
@@ -389,7 +389,6 @@ static int S_render_node(cmark_renderer *renderer, cmark_node *node,
switch (get_link_type(node)) {
case URL_AUTOLINK:
LIT("\\url{");
OUT(url, false, URL);
Copy link
Member

@jgm jgm Jan 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes the double rendering, but we lose proper escaping for the URL. (That is what the URL in OUT accomplishes.) The correct fix would be to keep this line and somehow suppress the output of children of this node. To do this, return 0 (that stops processing of the contents of the node):

-        break;
+        LIT("}");
+        return 0; // don't process contents or we get double render

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, ok.
I've improved this in 1c0d474. Thanks for looking over it!

This reverts commit 8fb1f1c
and adds the proper solution to the problem.

With commit 8fb1f1c double rendering is fixed, but the url isn't
escaped anymore, so I discarded the wrong copy.
We now return 0 from the function in case of a single link,
which stops processing the contents of the node.
@jgm jgm merged commit 5eb01d2 into commonmark:master Jan 7, 2017
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.

None yet

2 participants