Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Also accept text.md as a valid Markdown scope#177

Merged
daviwil merged 1 commit intoatom:masterfrom
burodepeper:patch-1
Sep 6, 2018
Merged

Also accept text.md as a valid Markdown scope#177
daviwil merged 1 commit intoatom:masterfrom
burodepeper:patch-1

Conversation

@burodepeper
Copy link
Contributor

Requirements

  • Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • All new code requires tests to ensure against regressions

Description of the Change

Appended an if statement to allow for either source.gfm or text.md to be valid scope names for Markdown files.

Alternate Designs

Ideally, there'd be some standard to detect whether content is Markdown without relying on a scope name, which is a little overkill for this feature. I think the proposed change is acceptable.

Benefits

language-gfm isn't the only Markdown grammar. This patch helps serve a larger audience.

Possible Drawbacks

None?

Applicable Issues

@daviwil
Copy link
Contributor

daviwil commented Sep 6, 2018

Thanks a lot @burodepeper!

@daviwil daviwil merged commit 84ea711 into atom:master Sep 6, 2018
burodepeper added a commit to burodepeper/whitespace that referenced this pull request Dec 19, 2018
I made a mistake in atom#177.

For whatever reason, I wrote a quick patch in my browser, and obviously, this code would never yield anything useful, _because it is just silly javascript_. The discussion in atom#185 made me verify my patch. This new patch should actually do something, while I have to admit that I've only verified it via Chrome's console. My test below seems valid enough.

```js
const markdownGrammars = ['source.gfm', 'text.md']

let grammarScopeName = 'text.md'
markdownGrammars.includes(grammarScopeName) // true

grammarScopeName = 'source.js'
markdownGrammars.includes(grammarScopeName) // false
```
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.

2 participants