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

GFM parser: option to disable typographic symbol conversion #462

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
2 participants
@kolyshkin
Contributor

kolyshkin commented Oct 17, 2017

With GFM, we want to be as close as possible to GitHub Flavored Markdown. In particular, GitHub does not replace -- with en-dash, --- with em-dash et cetera, and doing this not only breaks HTML rendering (resulting in, say, –option instead of --option), but also makes header IDs incompatible with those generated by GitHub. So, let's turn automatic typographic symbol conversion by default, and add an option to enable it back.

I have checked that this change makes header IDs the same as they are in GitHub.

For more details on the issue and examples, see #459

Signed-off-by: Kir Kolyshkin kolyshkin@gmail.com

@gettalong

This comment has been minimized.

Owner

gettalong commented Oct 18, 2017

Thanks for your pull request!

Some comments:

  • Please use the "gfm_quirks" configuration option instead of introducing a new one.
  • The default value should not change, i.e. this quirk should not automatically applied.

@gettalong gettalong self-assigned this Oct 18, 2017

@kolyshkin kolyshkin force-pushed the kolyshkin:gfm-idgen branch from f4605a8 to 473426f Oct 18, 2017

@kolyshkin

This comment has been minimized.

Contributor

kolyshkin commented Oct 18, 2017

  • Please use the "gfm_quirks" configuration option instead of introducing a new one.
  • The default value should not change, i.e. this quirk should not automatically applied.

Done. I have two concerns though:

  1. It looks like the value of gfm_quirks is not validated. Is it intentional (i.e. non-recognized options are silently ignored)? To my mind, it would make sense to have at least a warning printed, to figure out a typo or, say, an old version of parser where a specific option is not recognized.

  2. If this option/quirk is not enabled by default, we still have those two problems that I described in the #459. Isn't it your intention to make GFM parser as compatible with GFM as possible?

GFM parser: quirk to disable auto typographic symbol conversion
With GFM, we want to be as close as possible to GitHub Flavored
Markdown. In particular, GitHub does not replace -- with en-dash,
--- with em-dash et cetera, and doing this not only breaks
HTML rendering (resulting in, say, –option instead of --option).

The above replacement also makes header IDs incompatible with those
generated by GitHub, making it way harder to create a document
with intralinks that work both on GitHub and kramdown-generated
HTML.

So, let's introduce another GFM quirk to turn off automatic
typographic symbol conversion. NOTE it is off by default, i.e.
needs to be explicitly enabled.

For more details on the issue and examples, see
#459

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

@kolyshkin kolyshkin force-pushed the kolyshkin:gfm-idgen branch from 473426f to 4d68498 Oct 18, 2017

@kolyshkin kolyshkin changed the title from GFM parser: no default typographic symbol conversion to GFM parser: option to disable typographic symbol conversion Oct 19, 2017

@gettalong

This comment has been minimized.

Owner

gettalong commented Oct 26, 2017

Thanks for the changes! As for your concerns:

  1. Yes, this is intentional so that custom parsers/converter can reuse the option.

  2. Full compatibility with GFM will not be possible, at first because there was no spec and now because it is based on CommonMark. Therefore having this as option is preferable.

@gettalong

This comment has been minimized.

Owner

gettalong commented Oct 26, 2017

I have cherry-picked your commit - the changes will be in the next release.

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