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

Possibly, incorrect parsing of \n #209

Closed
mikeando opened this issue May 1, 2022 · 4 comments
Closed

Possibly, incorrect parsing of \n #209

mikeando opened this issue May 1, 2022 · 4 comments

Comments

@mikeando
Copy link
Contributor

mikeando commented May 1, 2022

The spec has:

A code span begins with a backtick string and ends with a backtick string of equal length. The contents of the code span are the characters between these two backtick strings, normalized in the following ways:

  • First, line endings are converted to spaces.
  • If the resulting string both begins and ends with a space character, but does not consist entirely of space characters, a single space character is removed from the front and back. This allows you to include code that begins or ends with backtick characters, which must be separated by whitespace from the opening or closing backtick strings.

This means that \n should convert to <code> </code>, which is the output from the commonmark.js dingus (https://spec.commonmark.org/dingus/?text=%60%0A%60%0A).

Withcomrak it is instead converted to <code></code>.
I think this makes no difference in the appearance of the final output - but IMO it would be preferable to strictly match the
expected output.

@kivikakk
Copy link
Owner

kivikakk commented May 1, 2022

Interesting, nice discovery! Would happily accept a patch that addressed this. 😊

@mikeando
Copy link
Contributor Author

mikeando commented May 2, 2022

I'd be happy to try to come up with a fix.
Any pointers on where to start? (I've used comrak a bit, but never dug through the code in any detail)

@mikeando
Copy link
Contributor Author

mikeando commented May 2, 2022

Turned out pretty easy to find: have created PR for it at #210

@mikeando
Copy link
Contributor Author

The PR is merged, this can be closed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants