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 26 commits
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

@@ -18,9 +18,25 @@ module.exports = class GrammarListView {

const div = document.createElement('div');
div.classList.add('pull-right');

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';
parser.setAttribute(
'title',
'(Recommended) A faster parser with improved syntax highlighting & code navigation support.'
);
div.appendChild(parser);
}

if (grammar.scopeName) {
const scopeName = document.createElement('scopeName');
scopeName.classList.add('key-binding'); // It will be styled the same as the keybindings in the command palette
scopeName.classList.add('badge', 'badge-info');
scopeName.textContent = grammar.scopeName;
div.appendChild(scopeName);
element.appendChild(div);
@@ -33,7 +49,7 @@ module.exports = class GrammarListView {
if (grammar === this.autoDetect) {
atom.textEditors.clearGrammarOverride(this.editor);
} else {
atom.textEditors.setGrammarOverride(this.editor, grammar.scopeName);
atom.grammars.assignGrammar(this.editor, grammar);
}
},
didCancelSelection: () => {
@@ -72,32 +88,68 @@ module.exports = 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();
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().filter(grammar => {
let grammars = atom.grammars.getGrammars().filter(grammar => {
return grammar !== atom.grammars.nullGrammar && grammar.name;
});

if (atom.config.get('grammar-selector.hideDuplicateTextMateGrammars')) {

This comment has been minimized.

Copy link
@nathansobo

nathansobo Jun 19, 2019

Contributor

In this case, do you really need the second call to atom.grammars.getGrammars({ textMateOnly: true }? It seems like you could simply iterate through the grammars array twice, once to populate the blacklist of Tree Sitter grammars and then a second time to filter. It's not a huge deal though.

This comment has been minimized.

Copy link
@Aerijo

Aerijo Jun 20, 2019

Author Member

Yeah, cleaned it up

const oldGrammars = grammars;
grammars = [];
const blacklist = new Set();
for (const grammar of oldGrammars) {
if (isTreeSitter(grammar)) {
blacklist.add(grammar.name);
grammars.push(grammar);
}
}
atom.grammars.getGrammars({ textMateOnly: true }).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) {
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"
}
}
}
@@ -7,6 +7,7 @@ describe('GrammarSelector', () => {
beforeEach(async () => {
jasmine.attachToDOM(atom.views.getView(atom.workspace));
atom.config.set('grammar-selector.showOnRightSideOfStatusBar', false);
atom.config.set('grammar-selector.hideDuplicateTextMateGrammars', false);

await atom.packages.activatePackage('status-bar');
await atom.packages.activatePackage('grammar-selector');
@@ -27,21 +28,13 @@ describe('GrammarSelector', () => {

describe('when grammar-selector:show is triggered', () =>
it('displays a list of all the available grammars', async () => {
atom.commands.dispatch(editor.getElement(), 'grammar-selector:show');
await SelectListView.getScheduler().getNextUpdatePromise();

const grammarView = atom.workspace.getModalPanels()[0].getItem().element;
// TODO: Remove once Atom 1.23 reaches stable
if (parseFloat(atom.getVersion()) >= 1.23) {
// Do not take into account the two JS regex grammars or language-with-no-name
expect(grammarView.querySelectorAll('li').length).toBe(
atom.grammars.grammars.length - 3
);
} else {
expect(grammarView.querySelectorAll('li').length).toBe(
atom.grammars.grammars.length - 1
);
}
const grammarView = (await getGrammarView(editor)).element;

// -1 for removing nullGrammar, +1 for adding "Auto Detect"
// Tree-sitter names the regex and JSDoc grammars
expect(grammarView.querySelectorAll('li').length).toBe(
atom.grammars.grammars.filter(g => g.name).length
);
expect(grammarView.querySelectorAll('li')[0].textContent).toBe(
'Auto Detect'
);
@@ -51,42 +44,31 @@ describe('GrammarSelector', () => {
.forEach(li =>
expect(li.textContent).not.toBe(atom.grammars.nullGrammar.name)
);
expect(grammarView.textContent.includes('Tree-sitter')).toBe(true); // check we are showing and labelling Tree-sitter grammars
}));

describe('when a grammar is selected', () =>
it('sets the new grammar on the editor', async () => {
atom.commands.dispatch(editor.getElement(), 'grammar-selector:show');
await SelectListView.getScheduler().getNextUpdatePromise();

const grammarView = atom.workspace.getModalPanels()[0].getItem();
const grammarView = await getGrammarView(editor);
grammarView.props.didConfirmSelection(textGrammar);
expect(editor.getGrammar()).toBe(textGrammar);
}));

describe('when auto-detect is selected', () =>
it('restores the auto-detected grammar on the editor', async () => {
atom.commands.dispatch(editor.getElement(), 'grammar-selector:show');
await SelectListView.getScheduler().getNextUpdatePromise();

let grammarView = atom.workspace.getModalPanels()[0].getItem();
let grammarView = await getGrammarView(editor);
grammarView.props.didConfirmSelection(textGrammar);
expect(editor.getGrammar()).toBe(textGrammar);

atom.commands.dispatch(editor.getElement(), 'grammar-selector:show');
await SelectListView.getScheduler().getNextUpdatePromise();

grammarView = atom.workspace.getModalPanels()[0].getItem();
grammarView = await getGrammarView(editor);
grammarView.props.didConfirmSelection(grammarView.items[0]);
expect(editor.getGrammar()).toBe(jsGrammar);
}));

describe("when the editor's current grammar is the null grammar", () =>
it('displays Auto Detect as the selected grammar', async () => {
editor.setGrammar(atom.grammars.nullGrammar);
atom.commands.dispatch(editor.getElement(), 'grammar-selector:show');
await SelectListView.getScheduler().getNextUpdatePromise();

const grammarView = atom.workspace.getModalPanels()[0].getItem().element;
const grammarView = (await getGrammarView(editor)).element;
expect(grammarView.querySelector('li.active').textContent).toBe(
'Auto Detect'
);
@@ -97,10 +79,7 @@ describe('GrammarSelector', () => {
editor = await atom.workspace.open();
expect(editor.getGrammar()).not.toBe(jsGrammar);

atom.commands.dispatch(editor.getElement(), 'grammar-selector:show');
await SelectListView.getScheduler().getNextUpdatePromise();

const grammarView = atom.workspace.getModalPanels()[0].getItem();
const grammarView = await getGrammarView(editor);
grammarView.props.didConfirmSelection(jsGrammar);
expect(editor.getGrammar()).toBe(jsGrammar);
}));
@@ -199,6 +178,73 @@ describe('GrammarSelector', () => {
);
}));

describe('when toggling hideDuplicateTextMateGrammars', () => {
it('shows only the Tree-sitter if true and both exist', async () => {
// the main JS grammar has both a TextMate and Tree-sitter implementation
atom.config.set('grammar-selector.hideDuplicateTextMateGrammars', true);
const grammarView = await getGrammarView(editor);
const observedNames = new Set();
grammarView.element.querySelectorAll('li').forEach(li => {
const name = li.getAttribute('data-grammar');
expect(observedNames.has(name)).toBe(false);
observedNames.add(name);
});

// check the seen JS is actually the Tree-sitter one
const list = atom.workspace.getModalPanels()[0].item;
for (const item of list.items) {
if (item.name === 'JavaScript') {
expect(item.constructor.name === 'TreeSitterGrammar');
}
}
});

it('shows both if false', async () => {
await atom.packages.activatePackage('language-c'); // punctuation making it sort wrong
atom.config.set(
'grammar-selector.hideDuplicateTextMateGrammars',
false
);
await getGrammarView(editor);
let cppCount = 0;

const listItems = atom.workspace.getModalPanels()[0].item.items;
for (let i = 0; i < listItems.length; i++) {
const grammar = listItems[i];
const name = grammar.name;
if (cppCount === 0 && name === 'C++') {
expect(grammar.constructor.name).toBe('TreeSitterGrammar'); // first C++ entry should be Tree-sitter
cppCount++;
} else if (cppCount === 1) {
expect(name).toBe('C++');
expect(grammar.constructor.name).toBe('Grammar'); // immediate next grammar should be the TextMate version
cppCount++;
} else {
expect(name).not.toBe('C++'); // there should not be any other C++ grammars
}
}

expect(cppCount).toBe(2); // ensure we actually saw both grammars
});
});

describe('for every Tree-sitter grammar', () => {
it('adds a label to identify it as Tree-sitter', async () => {
const grammarView = await getGrammarView(editor);
const elements = grammarView.element.querySelectorAll('li');
const listItems = atom.workspace.getModalPanels()[0].item.items;
for (let i = 0; i < listItems.length; i++) {
if (listItems[i].constructor.name === 'TreeSitterGrammar') {
expect(
elements[i].childNodes[1].childNodes[0].className.startsWith(
'grammar-selector-parser'
)
).toBe(true);
}
}
});
});

describe('when clicked', () =>
it('shows the grammar selector modal', () => {
const eventHandler = jasmine.createSpy('eventHandler');
@@ -224,3 +270,9 @@ function getTooltipText(element) {
const [tooltip] = atom.tooltips.findTooltips(element);
return tooltip.getTitle();
}

async function getGrammarView(editor) {
atom.commands.dispatch(editor.getElement(), 'grammar-selector:show');
await SelectListView.getScheduler().getNextUpdatePromise();
return atom.workspace.getModalPanels()[0].getItem();
}
@@ -4,3 +4,7 @@
.grammar-status a:hover {
color: @text-color;
}

.grammar-selector-parser {
margin-right: @component-padding;
}
@@ -69,6 +69,23 @@ describe('GrammarRegistry', () => {
});
});

describe('.assignGrammar(buffer, grammar)', () => {
it('allows a TextMate grammar to be assigned directly, even when Tree-sitter is permitted', () => {
grammarRegistry.loadGrammarSync(
require.resolve(
'language-javascript/grammars/tree-sitter-javascript.cson'
)
);
const tmGrammar = grammarRegistry.loadGrammarSync(
require.resolve('language-javascript/grammars/javascript.cson')
);

const buffer = new TextBuffer();
expect(grammarRegistry.assignGrammar(buffer, tmGrammar)).toBe(true);
expect(buffer.getLanguageMode().getGrammar()).toBe(tmGrammar);
});
});

describe('.grammarForId(languageId)', () => {
it('returns a text-mate grammar when `core.useTreeSitterParsers` is false', () => {
atom.config.set('core.useTreeSitterParsers', false, {
@@ -859,6 +876,25 @@ describe('GrammarRegistry', () => {
expect(buffer2Copy.getLanguageMode().getLanguageId()).toBe('source.js');
});
});

describe('when working with grammars', () => {
beforeEach(async () => {
await atom.packages.activatePackage('language-javascript');
});

it('returns both Tree-sitter and TextMate grammars by default', async () => {
const allGrammars = atom.grammars.getGrammars();
const tmGrammars = atom.grammars.getGrammars({ textMateOnly: true });
expect(allGrammars.length).toBeGreaterThan(tmGrammars.length);
});

it('executes the foreach callback on both Tree-sitter and TextMate grammars', async () => {
const numAllGrammars = atom.grammars.getGrammars().length;
let i = 0;
atom.grammars.forEachGrammar(() => i++);
expect(i).toBe(numAllGrammars);
});
});
});

function retainedBufferCount(grammarRegistry) {
@@ -1999,10 +1999,13 @@ describe('Workspace', () => {
.sort()
).toEqual([
'source.coffee',
'source.js', // Tree-sitter grammars also load
'source.js',
'source.js.regexp',
'source.js.regexp',
'source.js.regexp.replacement',
'source.jsdoc',
'source.jsdoc',
'source.litcoffee',
'text.plain.null-grammar',
'text.todo'
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.