From b9258b5884e25e0fee607cdf4606d9d5a5f3401a Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 2 Mar 2024 20:55:02 -0800 Subject: [PATCH] [grammar-selector] Overhaul grammar display: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * When set to `true`, `hideDuplicateTextMateGrammars` will hide all grammars except whichever one the user has indicated a preference for — via `useTreeSitterParsers` and `useLegacyTreeSitter` settings, whether global or scope-specific. * When set to `false`, `hideDuplicateTextMateGrammars` will show all grammars, even Legacy Tree-sitter. --- .../grammar-selector/lib/grammar-list-view.js | 120 +++++++++---- packages/grammar-selector/package.json | 5 +- .../spec/grammar-selector-spec.js | 165 ++++++++++++++---- 3 files changed, 223 insertions(+), 67 deletions(-) diff --git a/packages/grammar-selector/lib/grammar-list-view.js b/packages/grammar-selector/lib/grammar-list-view.js index aa5c0a0d8..e752a7875 100644 --- a/packages/grammar-selector/lib/grammar-list-view.js +++ b/packages/grammar-selector/lib/grammar-list-view.js @@ -4,6 +4,13 @@ module.exports = class GrammarListView { constructor() { this.autoDetect = { name: 'Auto Detect' }; + this.configSubscription = atom.config.observe( + 'grammar-selector.hideDuplicateTextMateGrammars', + (value) => { + this.hideDuplicateGrammars = value + } + ); + this.selectListView = new SelectListView({ itemsClassList: ['mark-active'], items: [], @@ -20,18 +27,23 @@ module.exports = class GrammarListView { const div = document.createElement('div'); div.classList.add('pull-right'); - if (isTreeSitter(grammar)) { + if (isTreeSitter(grammar) && !this.hideDuplicateGrammars) { + // When we show all grammars, even duplicates, we should add a badge + // to each Tree-sitter grammar to distinguish them in the list. const parser = document.createElement('span'); - let badgeColor = 'badge-success'; - let badgeText = 'Tree-sitter'; + let badgeColor; + let badgeText = isModernTreeSitter(grammar) ? 'Modern Tree-sitter' : 'Legacy Tree-sitter'; + let languageModeConfig = getLanguageModeConfig(); - if (isLegacyTreeSitterMode()) { + if (languageModeConfig === 'node-tree-sitter') { // Color the legacy badge green to represent the user's preference. badgeColor = isLegacyTreeSitter(grammar) ? 'badge-success' : 'badge-warning'; - badgeText = isLegacyTreeSitter(grammar) ? - 'Legacy Tree-sitter' : 'Modern Tree-sitter'; + } else { + // Color the modern badge green to represent the user's preference. + badgeColor = isModernTreeSitter(grammar) ? + 'badge-success' : 'badge-warning'; } parser.classList.add( @@ -40,10 +52,12 @@ module.exports = class GrammarListView { badgeColor ); parser.textContent = badgeText; - parser.setAttribute( - 'title', - '(Recommended) A faster parser with improved syntax highlighting & code navigation support.' - ); + if (isModernTreeSitter(grammar)) { + parser.setAttribute( + 'title', + '(Recommended) A faster parser with improved syntax highlighting & code navigation support.' + ); + } div.appendChild(parser); } @@ -99,6 +113,66 @@ module.exports = class GrammarListView { this.selectListView.reset(); } + getParserPreferenceForScopeName(scopeName) { + let useTreeSitterParsers = atom.config.get( + 'core.useTreeSitterParsers', + { scope: [scopeName] } + ); + let useLegacyTreeSitter = atom.config.get( + 'core.useLegacyTreeSitter', + { scope: [scopeName] } + ); + + if (!useTreeSitterParsers) { + return 'textmate'; + } else if (useLegacyTreeSitter) { + return 'node-tree-sitter'; + } else { + return 'web-tree-sitter'; + } + } + + getParserTypeForGrammar(grammar) { + switch (grammar.constructor.name) { + case 'WASMTreeSitterGrammar': + return 'web-tree-sitter'; + case 'TreeSitterGrammar': + return 'node-tree-sitter'; + default: + return 'textmate'; + } + } + + grammarShouldBeDisplayed(grammar, parserPreference) { + if (!this.hideDuplicateGrammars) return true; + return this.getParserTypeForGrammar(grammar) === parserPreference; + } + + getAllDisplayableGrammars() { + let allGrammars = atom.grammars + .getGrammars({ includeTreeSitter: true }) + .filter(grammar => { + return grammar !== atom.grammars.nullGrammar && grammar.name; + }); + + let parserPreferenceById = new Map(); + + let results = []; + for (let grammar of allGrammars) { + let id = grammar.scopeName; + let preference = parserPreferenceById.get(id); + if (!preference) { + preference = this.getParserPreferenceForScopeName(id); + parserPreferenceById.set(id, preference); + } + if (this.grammarShouldBeDisplayed(grammar, preference)) { + results.push(grammar); + } + } + + return results; + } + async toggle() { if (this.panel != null) { this.cancel(); @@ -113,31 +187,7 @@ module.exports = class GrammarListView { this.currentGrammar = this.autoDetect; } - let grammars = atom.grammars - .getGrammars({ includeTreeSitter: true }) - .filter(grammar => { - return grammar !== atom.grammars.nullGrammar && grammar.name; - }); - - // Don't show legacy Tree-sitter grammars in the selector unless the user - // has opted into it. - if (!isLegacyTreeSitterMode()) { - grammars = grammars.filter(grammar => !isLegacyTreeSitter(grammar)); - } - - if (atom.config.get('grammar-selector.hideDuplicateTextMateGrammars')) { - // Filter out all TextMate grammars for which there is a Tree-sitter - // grammar with the exact same name. - const blacklist = new Set(); - grammars.forEach(grammar => { - if (isTreeSitter(grammar)) { - blacklist.add(grammar.name); - } - }); - grammars = grammars.filter( - grammar => isTreeSitter(grammar) || !blacklist.has(grammar.name) - ); - } + let grammars = this.getAllDisplayableGrammars(); grammars.sort((a, b) => { if (a.scopeName === 'text.plain') { diff --git a/packages/grammar-selector/package.json b/packages/grammar-selector/package.json index f52d01650..1a40f5fde 100644 --- a/packages/grammar-selector/package.json +++ b/packages/grammar-selector/package.json @@ -22,12 +22,13 @@ "showOnRightSideOfStatusBar": { "type": "boolean", "default": true, - "description": "Show the active pane item's language on the right side of Pulsar's status bar, instead of the left." + "description": "Show the active pane item’s language on the right side of Pulsar’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" + "title": "Hide Duplicate Grammars", + "description": "Hides non-preferred grammars when there is more than one grammar. When checked, whichever grammar is preferred for a given scope name (TextMate or Tree-sitter) will be the only one shown. When unchecked, all grammars will always be shown in the list, regardless of the user’s settings." } } } diff --git a/packages/grammar-selector/spec/grammar-selector-spec.js b/packages/grammar-selector/spec/grammar-selector-spec.js index f4f784c32..d8f6563a8 100644 --- a/packages/grammar-selector/spec/grammar-selector-spec.js +++ b/packages/grammar-selector/spec/grammar-selector-spec.js @@ -39,12 +39,14 @@ describe('GrammarSelector', () => { it('displays a list of all the available grammars', async () => { const grammarView = (await getGrammarView(editor)).element; + let allGrammars = atom.grammars + .getGrammars({ includeTreeSitter: true }) + .filter(g => g.name) + // -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 - .getGrammars({ includeTreeSitter: true }) - .filter(g => g.name).length + allGrammars.length ); expect(grammarView.querySelectorAll('li')[0].textContent).toBe( 'Auto Detect' @@ -55,7 +57,9 @@ 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 + if (!atom.config.get('grammar-selector.hideDuplicateTextMateGrammars')) { + expect(grammarView.textContent.includes('Tree-sitter')).toBe(true); // check we are showing and labelling Tree-sitter grammars + } })); describe('when a grammar is selected', () => @@ -204,6 +208,11 @@ describe('GrammarSelector', () => { })); describe('when toggling hideDuplicateTextMateGrammars', () => { + // For continuity reasons, the name of the setting won't be changed; but + // this should now be construed as “hide duplicate grammars” — with the + // grammar selector showing whatever the user-indicated preference is for + // a given grammar. + 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); @@ -259,7 +268,7 @@ describe('GrammarSelector', () => { expect(cppCount).toBe(3); // ensure we actually saw all three grammars }); - it('shows two if false (in proper order when language parser is wasm-tree-sitter)', async () => { + it('shows all three if false (in proper order when language parser is wasm-tree-sitter)', async () => { await atom.packages.activatePackage('language-c'); // punctuation making it sort wrong setConfigForLanguageMode('wasm-tree-sitter'); atom.config.set( @@ -268,16 +277,19 @@ describe('GrammarSelector', () => { ); 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++') { - // first C++ entry should be modern Tree-sitter + // first C++ entry should be legacy Tree-sitter expect(grammar.constructor.name).toBe('WASMTreeSitterGrammar'); cppCount++; } else if (cppCount === 1) { + // next C++ entry should be modern Tree-sitter + expect(grammar.constructor.name).toBe('TreeSitterGrammar'); + cppCount++; + } else if (cppCount === 2) { // immediate next grammar should be the TextMate version expect(name).toBe('C++'); expect(grammar.constructor.name).toBe('Grammar'); @@ -287,10 +299,10 @@ describe('GrammarSelector', () => { } } - expect(cppCount).toBe(2); // ensure we actually saw two grammars + expect(cppCount).toBe(3); // ensure we actually saw two grammars }); - it('shows both if false (in proper order when language parser is textmate)', async () => { + it('shows all three if false (in proper order when language parser is textmate)', async () => { await atom.packages.activatePackage('language-c'); // punctuation making it sort wrong atom.config.set( 'grammar-selector.hideDuplicateTextMateGrammars', @@ -304,20 +316,24 @@ describe('GrammarSelector', () => { const grammar = listItems[i]; const name = grammar.name; if (cppCount === 0 && name === 'C++') { - // first C++ entry should be TextMate + // first C++ entry should be legacy Tree-sitter expect(grammar.constructor.name).toBe('Grammar'); cppCount++; } else if (cppCount === 1) { - // immediate next grammar should be the Tree-sitter version - expect(name).toBe('C++'); + // next C++ entry should be modern Tree-sitter expect(grammar.constructor.name).toBe('WASMTreeSitterGrammar'); cppCount++; + } else if (cppCount === 2) { + // immediate next grammar should be the TextMate version + expect(name).toBe('C++'); + expect(grammar.constructor.name).toBe('TreeSitterGrammar'); + 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 + expect(cppCount).toBe(3); // ensure we actually saw three grammars }); }); @@ -387,17 +403,16 @@ describe('GrammarSelector', () => { .forEach(li => expect(li.textContent).not.toBe(atom.grammars.nullGrammar.name) ); - // Ensure we're showing and labelling tree-sitter grammars… + if (!atom.config.get('grammar-selector.hideDuplicateTextMateGrammars')) { + // Ensure we're showing and labelling tree-sitter grammars. expect(grammarView.textContent.includes('Tree-sitter')).toBe(true); - // …but not old tree-sitter grammars. - expect(grammarView.textContent.includes('Legacy Tree-sitter')).toBe(false); + } }); }); 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 + it('shows only the preferred if true and several exist (and preferred is default)', async () => { atom.config.set('grammar-selector.hideDuplicateTextMateGrammars', true); const grammarView = await getGrammarView(editor); const observedNames = new Map(); @@ -420,7 +435,53 @@ describe('GrammarSelector', () => { } }); - it('shows both if false', async () => { + it('shows only the preferred if true and several exist (and preferred is node-tree-sitter)', async () => { + atom.config.set('grammar-selector.hideDuplicateTextMateGrammars', true); + setConfigForLanguageMode('node-tree-sitter', { scopeSelector: '.source.js' }) + const grammarView = await getGrammarView(editor); + const observedNames = new Map(); + grammarView.element.querySelectorAll('li').forEach(li => { + const name = li.getAttribute('data-grammar'); + if (!observedNames.has(name)) { + observedNames.set(name, 0); + } + observedNames.set(name, observedNames.get(name) + 1); + expect(observedNames.get(name) < 2).toBe(true, `found ${observedNames.get(name)} of ${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).toBe('TreeSitterGrammar'); + } + } + }); + + it('shows only the preferred if true and several exist (and preferred is textmate)', async () => { + atom.config.set('grammar-selector.hideDuplicateTextMateGrammars', true); + setConfigForLanguageMode('textmate', { scopeSelector: '.source.js' }) + const grammarView = await getGrammarView(editor); + const observedNames = new Map(); + grammarView.element.querySelectorAll('li').forEach(li => { + const name = li.getAttribute('data-grammar'); + if (!observedNames.has(name)) { + observedNames.set(name, 0); + } + observedNames.set(name, observedNames.get(name) + 1); + expect(observedNames.get(name) < 2).toBe(true, `found ${observedNames.get(name)} of ${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).toBe('Grammar'); + } + } + }); + + it('shows three if false (in the proper order when language parser is web-tree-sitter)', async () => { await atom.packages.activatePackage('language-c'); // punctuation making it sort wrong atom.config.set( 'grammar-selector.hideDuplicateTextMateGrammars', @@ -437,20 +498,28 @@ describe('GrammarSelector', () => { expect(grammar.constructor.name).toBe('WASMTreeSitterGrammar'); // first C++ entry should be Modern Tree-sitter cppCount++; } else if (cppCount === 1) { + expect(name).toBe('C++'); + expect(grammar.constructor.name).toBe('TreeSitterGrammar'); // first C++ entry should be Modern Tree-sitter + cppCount++; + } else if (cppCount === 2) { 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(name).not.toBe('C++'); } } - expect(cppCount).toBe(2); // ensure we actually saw three grammars + expect(cppCount).toBe(3); // ensure we actually saw three grammars }); }); describe('for every Tree-sitter grammar', () => { it('adds a label to identify it as Tree-sitter', async () => { + atom.config.set( + 'grammar-selector.hideDuplicateTextMateGrammars', + false + ); const grammarView = await getGrammarView(editor); const elements = grammarView.element.querySelectorAll('li'); const listItems = atom.workspace.getModalPanels()[0].item.items; @@ -514,33 +583,35 @@ describe('GrammarSelector', () => { }); describe('when toggling hideDuplicateTextMateGrammars', () => { - it('shows only the Tree-sitter if true and both exist', async () => { + it('shows only the Tree-sitter if true and several exist', async () => { // the main JS grammar has both a TextMate and Tree-sitter implementation - atom.config.set('grammar-selector.hideDuplicateTextMateGrammars', true); + atom.config.set( + 'grammar-selector.hideDuplicateTextMateGrammars', + true + ); const grammarView = await getGrammarView(editor); const observedNames = new Map(); - // Show a maximum of two different kinds of grammar (both tree-sitter - // variants). + grammarView.element.querySelectorAll('li').forEach(li => { const name = li.getAttribute('data-grammar'); if (!observedNames.has(name)) { observedNames.set(name, 0); } observedNames.set(name, observedNames.get(name) + 1); - expect(observedNames.get(name) < 3).toBe(true, `found ${observedNames.get(name)} of ${name}`); + expect(observedNames.get(name) < 2).toBe(true, `found ${observedNames.get(name)} of ${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.includes('TreeSitterGrammar')); + expect(item.constructor.name).toBe('TreeSitterGrammar'); } } }); - it('shows both if false', async () => { - await atom.packages.activatePackage('language-c'); // punctuation making it sort wrong + it('shows all if false', async () => { + await atom.packages.activatePackage('language-c'); atom.config.set( 'grammar-selector.hideDuplicateTextMateGrammars', false @@ -572,7 +643,11 @@ describe('GrammarSelector', () => { }); describe('for every Tree-sitter grammar', () => { - it('adds a label to identify it as Tree-sitter', async () => { + it('adds a label to identify it as Tree-sitter (when showing duplicate grammars)', async () => { + atom.config.set( + 'grammar-selector.hideDuplicateTextMateGrammars', + false + ); const grammarView = await getGrammarView(editor); const elements = grammarView.element.querySelectorAll('li'); const listItems = atom.workspace.getModalPanels()[0].item.items; @@ -597,6 +672,36 @@ describe('GrammarSelector', () => { } } }); + + it('does not add a label to identify it as Tree-sitter (when hiding duplicate grammars)', async () => { + atom.config.set( + 'grammar-selector.hideDuplicateTextMateGrammars', + true + ); + 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++) { + let item = listItems[i]; + let element = elements[i]; + if (item.constructor.name.includes('TreeSitterGrammar')) { + expect( + element.childNodes[1].childNodes[0].className.startsWith( + 'grammar-selector-parser' + ) + ).toBe(false); + } + if (item.constructor.name === 'TreeSitterGrammar') { + expect( + element.childNodes[1].childNodes[0].classList.contains('badge-success') + ).toBe(false); + } else if (item.constructor.name === 'WASMTreeSitterGrammar') { + expect( + element.childNodes[1].childNodes[0].classList.contains('badge-warning') + ).toBe(false); + } + } + }); }); });