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

HTML comments in middle of block #181

Merged
merged 1 commit into from Nov 20, 2015
Merged

Conversation

mikesamuel
Copy link

Motivation

Comments in Markdown aren't really a thing, but they can come in super-useful. As alluded to in the test-case, MOE can be used to sync between internally developed projects and open-source versions. Part of that requires maintaining internal & external developer docs. MOE directives in comments make that a lot easier, but if they can't appear in the middle of a bullet list that becomes a lot harder.


Current Behavior

src/document.c recognizes <!--...--> style comments as a standalone block.

src/smarty_pants.c seems to recognize them anywhere though I haven't tested.

Changes

This adds a test-case and tweaks document.c to copy HTML comments over the same way it copies tags over.

The HTML comment recognizing code is a bit of a copy/paste job from parse_htmlblock but that version does a bunch of extra work. I can factor out the common code if you'd like.

…k to src/document.c that recognizes them as tags to copy over literally
@mikesamuel
Copy link
Author

commonmark example-567 says the markdown

foo <!-- this is a
comment - with hyphen -->

corresponds to the HTML

<p>foo <!-- this is a
comment - with hyphen --></p>

@mildsunrise
Copy link
Member

The v3 codebase is no longer in development other than for fixes, but/so I think a change like this one is acceptable.

However, your code isn't very legible and it seems to trigger an out-of-bounds read when the closing --> is not found. (i.e. control continues, the rest of the function executes with i > size)

@mildsunrise
Copy link
Member

Sorry, I spoke two early! The conditional on the next line completely resets i so it's okay.
If @devinus agrees, we can merge this.

@mikesamuel
Copy link
Author

I was unaware that there is a newer version. Should I try to effect a similar change in the dev version? Where would I find that?

@mikesamuel
Copy link
Author

Would you like a testcase for -- at the end of input?

@devinus
Copy link
Member

devinus commented Nov 20, 2015

This is a good fix IMO.

@mildsunrise
Copy link
Member

Okay, about to merge.

Would you like a testcase for -- at the end of input?

No, it's good. Our handling of comments isn't HTML compliant anyway.

I was unaware that there is a newer version. Should I try to effect a similar change in the dev version? Where would I find that?

No worries, the dev version targets CommonMark compliance, and it already parses HTML comments, both inline and block.

mildsunrise added a commit that referenced this pull request Nov 20, 2015
HTML comments in middle of block
@mildsunrise mildsunrise merged commit d8d37c3 into hoedown:master Nov 20, 2015
@mildsunrise
Copy link
Member

Released v3.0.6 with this and #174.

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

3 participants