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

Accept multiple markdown scope names #186

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

burodepeper
Copy link
Contributor

Description of the Change

I made a mistake in #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 #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.

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

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

grammarScopeName = 'source.js'
markdownGrammars.includes(grammarScopeName) // false

Alternate Designs

Not applicable

Benefits

Not applicable

Possible Drawbacks

Not fully tested from within Atom. This patch wouldn't break anything any further, at least...

Applicable Issues

Not applicable

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
```
@rsese
Copy link

rsese commented Dec 21, 2018

Thanks for revisiting this @burodepeper!

Would you be able to add a test for this? The team needs tests in general before they can take a look but also to help make sure future changes in the package don't end up breaking your fix 😄

@burodepeper
Copy link
Contributor Author

burodepeper commented Dec 21, 2018 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants