Skip to content

Conversation

@igrmk
Copy link
Contributor

@igrmk igrmk commented Nov 21, 2019

Github parses underscores not the same way as asterisks if they are inside such_an_identifier
Just look at this repo https://github.com/igrmk/github-markdown-corner-cases
This PR is adding support for such parsing
Asterisks are still parsed the old way

The feature is enabled only if markdown_syntax_github is set to 1

@tpope
Copy link
Owner

tpope commented Nov 23, 2019

A global option is a cop-out. What behaviors in the old implementation are you trying to preserve?

@igrmk
Copy link
Contributor Author

igrmk commented Nov 23, 2019

The problem appears if you use identifiers with underscores in italics.

With current implementation you can toggle italics, bold and bold italics inside the word using underscores. However GitHub ignores underscores inside the word in this case. You can still use asterisks to do it. So some GitHub documents look totally out of sync if you use identifiers with underscores in italics.

Example:

  1. _Italics_Italics_ => Italics_Italics (vim-markdown)
  2. _Italics_Italics_ => Italics_Italics (GitHub)
  3. *Italics*Normal*Italics* => ItalicsNormalItalics (asterisks work the same in both)

First example would cause the rest of the document to be rendered in italics if you use vim-markdown. This is because last underscore would start unclosed italics block.

@tpope
Copy link
Owner

tpope commented Nov 24, 2019

First example would cause the rest of the document to be rendered in italics if you use vim-markdown.

Nobody wants a single _ to highlight the rest of the document in italics. This is not a valid reason to add a configuration option.

I'd say your changes should be the default, but they introduce regressions. Try this:

this, _that_, and these

It works in the current vim-markdown implementation. It works on GitHub. Your implementation highlights the rest of the file. This is probably fixable though, by using \w instead of \S in some cases.

@igrmk
Copy link
Contributor Author

igrmk commented Nov 24, 2019

I've fixed regression and removed configuration option

@tpope tpope merged commit 8a76c84 into tpope:master Dec 1, 2019
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.

2 participants