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

Replace markdown grammar with maintained version #6341

Merged
merged 3 commits into from
May 30, 2023

Conversation

wooorm
Copy link
Contributor

@wooorm wooorm commented Mar 23, 2023

Description

This replaces the grammar used to highlight markdown with a maintained one.

I’ll add more sample files later. First wanted to bring this to your attention.

Related to: GH-6339.

Checklist:


Important: the current scope is source.gfm. The new scope is text.md.
There seems to be some Go code to handle text.html.markdown and text.md, which I changed.
The reasons why I prefer changing source.gfm to text.md is that:
a) markdown is closer to a text, markup, language, than to a source language that is “evaluated” or so
b) markdown isn’t a superset of HTML, it has its own complex rules for that, so text.html.markdown isn’t great IMO
c) Since it was (probably) added, GFM now bases itself on CommonMark (CM), bringing it much closer to normal markdown (for example, fenced code blocks are now “normal” markdown). This grammar, other than GFM features, also includes some other extensions (frontmatter, GitHub-specific features, directives, math), which aren’t in GFM

But, I wanted to bring this to attention because:
a) it’s still early days, I can be convinced to use something else
b) source.gfm is used in other existing grammars.

Notably, I see these grammars that use source.gfm:

  • jawee/language-blade (not updated in 2 years)
  • atom/language-ruby (archived)
  • language-viml (@Alhadis)
  • wooorm/markdown-tm-language (that’s me, I can do that!)

I imagine @Alhadis to be able to update this, and I can raise an issue with jawee/language-blade, although it’s not actively maintained, so I’m not sure that can be solved.

@wooorm wooorm requested a review from a team as a code owner March 23, 2023 18:57
@Alhadis
Copy link
Collaborator

Alhadis commented Mar 27, 2023

I prefer changing source.gfm to text.md is that:
a) markdown is closer to a text, markup, language, than to a source language that is “evaluated” or so

Correct. A language's type field is a good indicator of what prefix a TextMate grammar's scope should use: programming and data-type languages should use source.*-scoped grammars, while markup and prose-type languages should use text.*-scoped grammars instead.

It's unclear why the Atom team chose source.gfm for their markdown grammar, but it—along with virtually every other choice of scope-names they used—is an objectively poor choice of scopes that conflicts with established naming conventions.

a) it’s still early days, I can be convinced to use something else

I started a clean rewrite of a markdown grammar several years ago, but never published it (as I was reluctant to share a crappy 2-hour project stub with the world…). I'm sure I'll finish it eventually: I simply wanted to indent code-blocks without screwing up the rest of the document's highlighting:
Figure 1

b) source.gfm is used in other existing grammars.

Atom packages are likely to reference this scope for compatibility with the editor's default Markdown package (source.gfm), while others may support source.gfm and text.md in tandem, whichever the user has available.

Notably, I see these grammars that use source.gfm:
[…] language-viml (@Alhadis)

Only in VimL's injectionSelector field, which does nothing on GitHub because injections only affect grammars bundled in the same package/submodule (a wise restriction, IMHO). Ergo, it's use in language-viml is benign and can be ignored. 👍

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

Note: this PR will not be merged until close to when the next release is made. See here for more details.

@lildude lildude requested a review from a team as a code owner May 30, 2023 09:15
@lildude lildude added this pull request to the merge queue May 30, 2023
Merged via the queue into github-linguist:master with commit 90f1911 May 30, 2023
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants