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

[Markdown] support lists in blockquotes, code fences, multiline inline code and multiline emphasis #806

Merged
merged 30 commits into from
Mar 21, 2017

Conversation

keith-hall
Copy link
Collaborator

This PR simplifies a few contexts, removes a few incompatible regexs (mainly \G) and adds support for lists in blockquotes, plus includes a few more tests.

@keith-hall keith-hall force-pushed the markdown branch 3 times, most recently from e83517a to 4aac501 Compare February 13, 2017 10:34
@keith-hall
Copy link
Collaborator Author

I've got this to the state where the only incompatible regex patterns are two \Gs :)
Note that I have scoped .begin and .end which could affect color schemes - I can remove these changes if needed.

- match: '\1'
scope: punctuation.definition.raw.end.markdown
pop: true
- match: '{{backticks}}'
Copy link
Collaborator

@FichteFoll FichteFoll Feb 13, 2017

Choose a reason for hiding this comment

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

I suggest this instead:

contexts:
  raw:
    - match: (`+)
      scope: punctuation.definition.raw.begin.markdown
      push:
        - meta_scope: markup.raw.inline.markdown
        - match: \1(?!`)
          scope: punctuation.definition.raw.end.markdown
          pop: true
        - match: '`+'
        - match: ^\s*$\n?
          scope: invalid.illegal.non-terminated.raw.markdown
          pop: true

This allows multi-line inline raw sequences again and isn't explicit about the number of backticks allowed, which is arbitrary. I don't know if this actually represents how markdown parsers work, but some testing here on github seemed to confirm this behavior.

Should also exit on empty lines. See:

test `test

test`

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@FichteFoll thanks, I have just pushed a new commit with your proposal :)

@keith-hall
Copy link
Collaborator Author

I've done some testing at http://spec.commonmark.org/dingus/ and made the backtick handling more consistent with how that works. This includes support for code fences (though it doesn't highlight the language contained inside the fence yet, I'll leave that for another PR, as I wonder if it would give us more with_prototype recursion limit warnings).

@keith-hall keith-hall changed the title [Markdown] support lists in blockquotes [Markdown] support lists in blockquotes, code fences and multiline inline code Feb 14, 2017
@keith-hall
Copy link
Collaborator Author

keith-hall commented Feb 21, 2017

Performance comparison (24 KiB syntax test file):

This PR:

Syntax "Packages/Markdown/Markdown.sublime-syntax" took an average of 5.0ms over 10 runs

Before this PR:

Syntax "Packages/Markdown/Markdown.sublime-syntax" took an average of 7.2ms over 10 runs

@keith-hall keith-hall changed the title [Markdown] support lists in blockquotes, code fences and multiline inline code [Markdown] support lists in blockquotes, code fences, multiline inline code and multiline emphasis Feb 21, 2017
@keith-hall
Copy link
Collaborator Author

Performance update & comparisons against some Markdown syntaxes on Package Control, for the syntax test file in this PR, which is now 40 KiB:

Syntax "Packages/Markdown/Markdown.sublime-syntax" took an average of 10.4ms over 10 runs (this PR)

Syntax "Packages/Markdown/Markdown.sublime-syntax" took an average of 11.2ms over 10 runs (before this PR - I guess the smaller difference now is due to all the added constructs it lexes in this PR)

Syntax "Packages/Markdown Extended/Syntaxes/Markdown Extended.sublime-syntax" took an average of 12.3ms over 10 runs

Syntax "Packages/MarkdownHighlighting/Markdown.sublime-syntax" took an average of 12.8ms over 10 runs

Syntax "Packages/MarkdownEditing/Markdown (Standard).tmLanguage" took an average of 11.3ms over 10 runs

Syntax "Packages/MarkdownEditing/Markdown.tmLanguage" took an average of 12.9ms over 10 runs

@wbond wbond merged commit ac5c5d8 into sublimehq:master Mar 21, 2017
@wbond
Copy link
Member

wbond commented Mar 21, 2017

Thank you very much for all of this work!

@keith-hall keith-hall deleted the markdown branch March 21, 2017 13:34
@wbond wbond mentioned this pull request Mar 21, 2017
12 tasks
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.

3 participants