Skip to content
This repository has been archived by the owner on Aug 7, 2023. It is now read-only.

Support new tree-sitter grammars #1076

Closed
3 tasks done
zmb3 opened this issue Jan 15, 2018 · 13 comments
Closed
3 tasks done

Support new tree-sitter grammars #1076

zmb3 opened this issue Jan 15, 2018 · 13 comments

Comments

@zmb3
Copy link

zmb3 commented Jan 15, 2018

Issue Type

Feature Request

Issue Description

Atom master recently landed support for a new parser: atom/atom#16299

A side effect of this change is that the grammar names have changed, so the default scopes to run eslint are no longer sufficient (note the flow-javascript scope in the debug output below).

Bug Checklist

  • Restart Atom
  • Verify the eslint CLI gives the proper result, while linter-eslint does not
  • Paste the output of the Linter Eslint: Debug command from the Command Palette below
Atom version: 1.25.0-dev-64f46256e
linter-eslint version: 8.4.0
ESLint version: 4.15.0
Hours since last Atom restart: 0.1
Platform: darwin
Using local project ESLint from: /Users/zbergquist/github/go-plus/node_modules/eslint
Current file's scopes: [
  "flow-javascript",
  "program",
  "expression_statement",
  "class",
  "class_body",
  "method_definition",
  "statement_block"
]
linter-eslint configuration: {
  "fixOnSave": true,
  "scopes": [
    "source.js",
    "source.jsx",
    "source.js.jsx",
    "source.babel",
    "source.js-semantic"
  ],
  "lintHtmlFiles": false,
  "useGlobalEslint": false,
  "showRuleIdInMessage": true,
  "disableWhenNoEslintConfig": true,
  "eslintrcPath": "",
  "globalNodePath": "",
  "advancedLocalNodeModules": "",
  "eslintRulesDirs": [],
  "disableEslintIgnore": false,
  "disableFSCache": false,
  "rulesToSilenceWhileTyping": [],
  "rulesToDisableWhileFixing": [],
  "ignoreFixableRulesWhileTyping": false
}
@skylize
Copy link
Contributor

skylize commented Jan 15, 2018

There is a text field in linter-eslint settings for you to add additional scopes as needed.

It's currently near the bottom just above Show Rule Id and Global ESLint checkboxes.

@zmb3
Copy link
Author

zmb3 commented Jan 15, 2018

@skylize yep, I'm aware of it - however for the common case this should just work for users writing JS.

Eventually Atom will move away from textmate grammars entirely, so if linter-eslint doesn't change, every user will have to manually edit settings before things work. I just wanted to put the issue on the radar [well] in advance so you can stay in front of it.

@IanVS
Copy link
Member

IanVS commented Jan 15, 2018

Thanks for the heads-up, @zmb3. What do you expect to be the most commonly used scopes for the popular JS flavors?

@skylize
Copy link
Contributor

skylize commented Jan 15, 2018

Thanks for pointing that out @zmb3. The only information I've seen on it is this article on Atom performance that really only mentions it in passing.

Do you know of any good resources to learn about the changes?

@zmb3
Copy link
Author

zmb3 commented Jan 15, 2018

That’s all the info I have too. I only discovered this because the grammar change also broke go-plus. I’m sure the Atom team will better communicate what changes package authors need to make before this hits a release.

@Arcanemagus
Copy link
Member

Arcanemagus commented Jan 16, 2018

Hmmm, do you have any other language packages installed @zmb3? The new language-javascript should be showing as javascript (instead of source.js), so I'm unsure where the flow-javascript you are seeing is coming from. I'm building atom/atom@5a3e1d2 to check though 😉.

@zmb3
Copy link
Author

zmb3 commented Jan 16, 2018

I believe flow-javascript is coming from the ide-flowtype package.

@skylize
Copy link
Contributor

skylize commented Feb 5, 2018

Reference: PR that added TreeSitter - atom/atom#16299

@skylize
Copy link
Contributor

skylize commented Mar 2, 2018

Tree Sitter parser appears to work as expected out-of-the-box. Pull #1115 adds the default JS scope for Tree Sitter javascript by default. Until that gets released, you can add the javascript scope name yourself to the List of scopes to run ESLint on... in the linter-eslint Settings panel to get support .

For additional non-standard scopes such as flow-javascript mentioned above, you will still need to add them yourself to that same list. This is exactly why we have this setting, so people can add custom scopes based on their local setup.

Closing, as the "feature" part of the issue is effectively addressed by #1115, and the "question" part has been answered. @zmb3 let me know if you feel this issue needs any more attention.

@skylize skylize closed this as completed Mar 2, 2018
@steverandy
Copy link

Is there a plan to release this fix soon?

@skylize
Copy link
Contributor

skylize commented Mar 16, 2018

@Arcanemagus, It's really tiny, but maybe still worth a bump?

@steverandy
In the meantime, the current release will work just fine if you just add javascript to your scopes list in the Settings.

@Arcanemagus
Copy link
Member

Since we currently have major version changes on master, and a few more that I would like to get in before we publish that version I'm not sure the hassle of creating a branch and hotfixing this in there is worth it when those are still behind an experimental flag in Atom.

@skylize
Copy link
Contributor

skylize commented Mar 16, 2018

I was thinking we could just push master as-is. But I was confused about the severity of the existing changes on master.

I certainly agree that with Tree Sitter behind a flag, and with such an incredibly simple workaround, it makes no sense to branch off for a hotfix.

sonicdoe added a commit to ricardofbarros/linter-js-standard that referenced this issue Jun 22, 2018
The `javascript` scope is used by Atom’s experimental Tree-sitter parsing system. See also AtomLinter/linter-eslint#1076.
sonicdoe added a commit to ricardofbarros/linter-js-standard that referenced this issue Jun 22, 2018
The `javascript` scope is used by Atom’s experimental Tree-sitter parsing system. See also AtomLinter/linter-eslint#1076.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants