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

Allow for no whitespace between code fence language id and attributes #63

Merged
merged 2 commits into from
Oct 28, 2019
Merged

Conversation

janosh
Copy link
Contributor

@janosh janosh commented Oct 25, 2019

Closes #62.

build.js Outdated
@@ -80,7 +80,7 @@ const fencedCodeBlockDefinition = (name, identifiers, sourceScope, language, add

return `fenced_code_block_${name}:
begin:
(^|\\G)(\\s*)(\`{3,}|~{3,})\\s*(?i:(${identifiers.join('|')})(\\s+[^\`~]*)?$)
(^|\\G)(\\s*)(\`{3,}|~{3,})\\s*(?i:(${identifiers.join('|')})(\\s*[^\`~]*)?$)
Copy link
Contributor

@mjbvz mjbvz Oct 25, 2019

Choose a reason for hiding this comment

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

I believe this will cause false positive if users write a language name like pythonnotreally (which we should not highlight as python)

Are there specific sets of characters that are used to start a list of attributes? Like {?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

From my experience, it's either { or : but that's probably not a complete list. Still, could be enough for a start.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's start with that. So either require ({, :, or a whitespace) before the attributes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. Not sure that will appease CI, though.

@mjbvz
Copy link
Contributor

mjbvz commented Oct 25, 2019

Thanks. Can you also please add a test case for this new syntax

@mjbvz mjbvz merged commit 870c3d2 into microsoft:master Oct 28, 2019
@mjbvz
Copy link
Contributor

mjbvz commented Oct 28, 2019

Thanks!

@janosh
Copy link
Contributor Author

janosh commented Oct 28, 2019

@mjbvz Thanks for the guidance! Can you estimate when this will be released?

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.

Adding attributes to fenced markdown code blocks breaks syntax highlighting
2 participants