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

Show Tree-sitter grammars in grammar selector #18738

Merged
merged 37 commits into from Jun 21, 2019
Merged
Changes from 1 commit
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
44eeca6
Return tree sitter grammars too
Aerijo Jan 24, 2019
da804a2
Make including tree sitter grammars optional
Aerijo Jan 24, 2019
cb27305
Merge branch 'master' of github.com:atom/atom
Aerijo Feb 5, 2019
4c617cb
Add “Tree-sitter” badge
simurai Feb 13, 2019
4abfa5f
Add description
simurai Feb 13, 2019
2aef2da
Remove console.log
simurai Feb 13, 2019
5ff44a1
Merge branch 'master' of https://github.com/atom/atom
Aerijo Feb 17, 2019
8d4b9c2
Allow hiding duplicates + stable sort TS
Aerijo Feb 17, 2019
e9e52f5
Make method to assign specific grammar
Aerijo Feb 17, 2019
2cb9bc4
Fix specs
Aerijo Feb 18, 2019
321b493
add specs
Aerijo Feb 18, 2019
8dcdae9
merge upstream
Aerijo Mar 4, 2019
424c712
fix lints
Aerijo Mar 4, 2019
cb501f8
Better tests and enable returning Tree-sitter by default
Aerijo Mar 4, 2019
d277c94
docs and minor tweaks
Aerijo Mar 4, 2019
0312f53
fix specs
Aerijo Mar 4, 2019
8a488b2
fix lint
Aerijo Mar 4, 2019
391bf71
fix serialisation spec
Aerijo Mar 5, 2019
f4887f7
actually fix it
Aerijo Mar 5, 2019
814c23b
this?
Aerijo Mar 5, 2019
0580aa1
Merge commit '677bbb7f0b8754787ff9e7bfab4602ba82e13b0b' into pr18738
rafeca May 31, 2019
41840ab
Merge commit '1d9a4cafcf6cc288d675512db8fd984e13aab869' into pr18738
rafeca May 31, 2019
66f7f17
Reformat all JS files using prettier
rafeca May 31, 2019
f141ca3
Merge branch 'master' of github.com:atom/atom
Aerijo Jun 12, 2019
14f5f99
go with default include approach
Aerijo Jun 15, 2019
29a7b77
fix lint
Aerijo Jun 15, 2019
7d5a52f
clean up duplicate removal
Aerijo Jun 20, 2019
5b344ae
fix lint
Aerijo Jun 20, 2019
4cc1afb
imitate public interface
Aerijo Jun 20, 2019
2cc3bff
fix lint
Aerijo Jun 20, 2019
4bcb519
Merge branch 'master' of github.com:atom/atom
Aerijo Jun 20, 2019
3e1097f
Merge remote-tracking branch 'origin/master' into Aerijo
nathansobo Jun 20, 2019
a92c86f
Default to no Tree-sitter grammars
Aerijo Jun 21, 2019
cec3b05
restore tree-sitter in grammar iterator
Aerijo Jun 21, 2019
166d968
adjust specs
Aerijo Jun 21, 2019
9e1e647
fix lint
Aerijo Jun 21, 2019
5f220a8
fix spec
Aerijo Jun 21, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -20,7 +20,7 @@ class GrammarListView {
const div = document.createElement('div')
div.classList.add('pull-right')

if (grammar.constructor.name === "TreeSitterGrammar") {
if (isTreeSitter(grammar)) {
This conversation was marked as resolved by Aerijo

This comment has been minimized.

Copy link
@Aerijo

Aerijo Feb 17, 2019

Author Member

This is used a few times, and as a function adds better separation of the idea from the implementation

const parser = document.createElement('span')
parser.classList.add('grammar-selector-parser', 'badge', 'badge-success')
parser.textContent = 'Tree-sitter'
@@ -82,32 +82,64 @@ class GrammarListView {
async toggle () {
if (this.panel != null) {
this.cancel()
} else if (atom.workspace.getActiveTextEditor()) {
this.editor = atom.workspace.getActiveTextEditor()
return
}

const editor = atom.workspace.getActiveTextEditor()
This conversation was marked as resolved by Aerijo

This comment has been minimized.

Copy link
@Aerijo

Aerijo Feb 17, 2019

Author Member

It's trivial, but the original felt wasteful compared to just keeping the editor the first time. It's also technically more correct perhaps? Can the editor change between two consecutive steps?

if (editor) {
this.editor = editor
this.currentGrammar = this.editor.getGrammar()
if (this.currentGrammar === atom.grammars.nullGrammar) {
this.currentGrammar = this.autoDetect
}

const grammars = atom.grammars.getGrammars(true).filter((grammar) => {
let grammars = atom.grammars.getGrammars({includeTreeSitter: true}).filter(grammar => {
return grammar !== atom.grammars.nullGrammar && grammar.name
})

if (atom.config.get("grammar-selector.hideDuplicateTextMateGrammars")) {
const oldGrammars = grammars
This conversation was marked as resolved by Aerijo

This comment has been minimized.

Copy link
@Aerijo

Aerijo Feb 17, 2019

Author Member

This is a mess, but seems to work. Open for improvements.

grammars = []
const blacklist = new Set()
for (const grammar of oldGrammars) {
if (isTreeSitter(grammar)) {
blacklist.add(grammar.name)
grammars.push(grammar)
}
}
atom.grammars.getGrammars({includeTreeSitter: false}).forEach(grammar => {
if (grammar !== atom.grammars.nullGrammar && grammar.name && !blacklist.has(grammar.name)) {
grammars.push(grammar)
}
})
}

grammars.sort((a, b) => {
if (a.scopeName === 'text.plain') {
return -1
} else if (b.scopeName === 'text.plain') {
return 1
} else if (a.name) {
return a.name.localeCompare(b.name)
} else if (a.scopeName) {
return a.scopeName.localeCompare(b.scopeName)
} else {
return 1
} else if (a.name === b.name) {
This conversation was marked as resolved by Aerijo

This comment has been minimized.

Copy link
@Aerijo

Aerijo Feb 17, 2019

Author Member

Equivalent logic (besides TS check), as we filtered out grammars with no name already.

return compareGrammarType(a, b)
}
return a.name.localeCompare(b.name)
})
grammars.unshift(this.autoDetect)
await this.selectListView.update({items: grammars})
this.attach()
}
}
}

function isTreeSitter (grammar) {
return grammar.constructor.name === "TreeSitterGrammar"
}

function compareGrammarType (a, b) {
This conversation was marked as resolved by Aerijo

This comment has been minimized.

Copy link
@Aerijo

Aerijo Feb 17, 2019

Author Member

Separate function to make extending to multiple grammar types easier and not bloat the main sort function

if (isTreeSitter(a)) {
return -1
} else if (isTreeSitter(b)) {
return 1
}
return 0
}
@@ -37,6 +37,11 @@
"type": "boolean",
"default": true,
"description": "Show the active pane item's language on the right side of Atom's status bar, instead of the left."
},
"hideDuplicateTextMateGrammars": {
"type": "boolean",
"default": true,
"description": "Hides the TextMate grammar when there is an existing Tree-sitter grammar"
}
}
}
@@ -549,10 +549,12 @@ class GrammarRegistry {
// Extended: Get all the grammars in this registry.
//
// Returns a non-empty {Array} of {Grammar} instances.
getGrammars (includeTreesitter = false) {
let grammars = this.textmateRegistry.getGrammars()
if (includeTreesitter) grammars = grammars.concat(Object.values(this.treeSitterGrammarsById))
return grammars
getGrammars (params = {includeTreeSitter: false}) {

This comment has been minimized.

Copy link
@Aerijo

Aerijo Feb 17, 2019

Author Member

Turned into options object so we can add / change settings later without breaking changes

Maybe add TM-TS duplicate removal here? It would be slightly easier, and allow it to be used by all packages instead of making them do it themselves

let tmGrammars = this.textmateRegistry.getGrammars()
if (!params.includeTreeSitter) return tmGrammars

let tsGrammars = Object.values(this.treeSitterGrammarsById)
return tsGrammars.concat(tmGrammars)
}

scopeForId (id) {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.