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] fix list punctuation #803

Merged
merged 12 commits into from
Feb 10, 2017

Conversation

keith-hall
Copy link
Collaborator

Before this change, list punctuation was only scoped for the first item in the list. Now, all list punctuation will be correctly scoped.
Fixes #264

@@ -8,14 +8,25 @@ file_extensions:
- markdown
- markdn
scope: text.html.markdown
variables:
separator_line: |-
(?x)
Copy link
Collaborator

@FichteFoll FichteFoll Feb 10, 2017

Choose a reason for hiding this comment

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

I suggest always wrapping variables in non-capturing groups and multi-line expressions in (?x:...) groups, unless they represent a set. That way you can include them anywhere without having to worry about the including pattern being in extended more or not.

For example, in contexts.main[1].match you are already using extended mode, but if you were not the result could be different and unexpected, since extended mode would be active after you included this variable.
However, in context.separator[0].match you do not initially use extended mode, but if you were to use any spaces after the inclusion of this variable it would be ignored.

The reason why I wrap everything in a group is because it allows to treat variables atomicly. You can add quantifiers after variables and they mean exactly what you think they would (what the variable matches can be repeated) instead of potentially only repeating the last token of the variable's contents. For example, {{block_quote}}+ (which makes no sense, but for the example's sake) would expand to [ ]{,3}.+ and not (?:[ ]{,3}>.)+.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that's great advice, thanks @FichteFoll - I'll update my variables :)

@keith-hall
Copy link
Collaborator Author

I've fixed a few bugs mentioned at #266, added more tests, and started reworking the regex patterns that cause ST to use Oniguruma (#481). I think further improvements should be part of another PR now, as they start to get more complicated (the "italic" and "bold" regexes look intimidating, plus the lookbehinds and \Gs might be more involved to remove).

@wbond
Copy link
Member

wbond commented Feb 10, 2017

Awesome!

@wbond wbond merged commit 70e37f6 into sublimehq:master Feb 10, 2017
@keith-hall keith-hall deleted the markdown_list_punctuation branch February 10, 2017 14:51
@wbond wbond mentioned this pull request Feb 10, 2017
12 tasks
@jrappen
Copy link
Contributor

jrappen commented Feb 10, 2017

Thank you!

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.

4 participants