From 76ac2cf81ca29f6203c9854a19e906fe6c5471c0 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 6 Jan 2024 15:46:53 -0800 Subject: [PATCH 01/26] =?UTF-8?q?Make=20`useExperimentalModernTreeSitter`?= =?UTF-8?q?=20the=20default=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …and create `useLegacyTreeSitter` for those who want to opt into the previous default behavior. (Legacy Tree-sitter grammars will soon be removed, but this is a step toward that future!) --- .../autocomplete-css/spec/provider-spec.js | 6 +-- packages/autocomplete-html/spec/.eslintrc.js | 12 ++++++ .../grammar-selector/lib/grammar-list-view.js | 28 +++++++------- packages/grammar-selector/spec/.eslintrc.js | 8 ++++ .../spec/grammar-selector-spec.js | 4 +- packages/language-clojure/spec/.eslintrc.js | 9 +++++ .../language-clojure/spec/clojure-spec.js | 1 - .../language-clojure/spec/tokenizer-spec.js | 2 - .../language-html/spec/tree-sitter-spec.js | 2 +- packages/language-java/spec/.eslintrc.js | 12 ++++++ .../spec/tree-sitter-java-spec.js | 2 +- packages/language-ruby/spec/.eslintrc.js | 15 ++++++++ packages/language-ruby/spec/tokenizer-spec.js | 2 +- .../language-ruby/spec/tree-sitter-spec.js | 2 +- .../spec/wasm-tree-sitter-spec.js | 1 - spec/grammar-registry-spec.js | 4 +- spec/scope-resolver-spec.js | 5 +-- spec/tree-sitter-language-mode-spec.js | 2 +- spec/wasm-tree-sitter-language-mode-spec.js | 1 - src/config-schema.js | 7 ++-- src/grammar-registry.js | 38 +++++++++---------- 21 files changed, 106 insertions(+), 57 deletions(-) create mode 100644 packages/autocomplete-html/spec/.eslintrc.js create mode 100644 packages/grammar-selector/spec/.eslintrc.js create mode 100644 packages/language-clojure/spec/.eslintrc.js create mode 100644 packages/language-java/spec/.eslintrc.js create mode 100644 packages/language-ruby/spec/.eslintrc.js diff --git a/packages/autocomplete-css/spec/provider-spec.js b/packages/autocomplete-css/spec/provider-spec.js index 1249ee72cb..cd85ef374a 100644 --- a/packages/autocomplete-css/spec/provider-spec.js +++ b/packages/autocomplete-css/spec/provider-spec.js @@ -33,7 +33,7 @@ const packagesToTest = { const wait = (ms) => new Promise(resolve => setTimeout(resolve, ms)); -const whenEditorReady = function(editor) { +const whenEditorReady = function (editor) { const languageMode = editor.getBuffer().getLanguageMode(); if (!languageMode.constructor.name.includes('TreeSitter')) { return Promise.resolve(); @@ -105,8 +105,6 @@ describe("CSS property name and value autocompletions", async () => { await atom.workspace.open(packagesToTest[packageLabel].file); editor = atom.workspace.getActiveTextEditor(); await whenEditorReady(editor); - console.warn('USING TREE SITTER?!?', packageLabel, meta.useTreeSitter); - atom.config.set('core.useExperimentalModernTreeSitter', meta.useTreeSitter ?? false); atom.config.set('core.useTreeSitterParsers', meta.useTreeSitter ?? false); }); @@ -738,7 +736,7 @@ div:nth { }) ); - Object.keys(packagesToTest).forEach(function(packageLabel) { + Object.keys(packagesToTest).forEach(function (packageLabel) { if (packagesToTest[packageLabel].name !== 'language-css') { describe(`${packageLabel} files`, async () => { beforeEach(async () => { diff --git a/packages/autocomplete-html/spec/.eslintrc.js b/packages/autocomplete-html/spec/.eslintrc.js new file mode 100644 index 0000000000..b62afc93cd --- /dev/null +++ b/packages/autocomplete-html/spec/.eslintrc.js @@ -0,0 +1,12 @@ +module.exports = { + env: { jasmine: true }, + globals: { + "waitsForPromise": true + }, + rules: { + "node/no-unpublished-require": "off", + "node/no-extraneous-require": "off", + "no-unused-vars": "off", + "no-empty": "off" + } +}; diff --git a/packages/grammar-selector/lib/grammar-list-view.js b/packages/grammar-selector/lib/grammar-list-view.js index c54397375c..8980a18ce7 100644 --- a/packages/grammar-selector/lib/grammar-list-view.js +++ b/packages/grammar-selector/lib/grammar-list-view.js @@ -26,10 +26,10 @@ module.exports = class GrammarListView { let badgeColor = 'badge-success'; let badgeText = 'Tree-sitter'; - if (isExperimentalTreeSitterMode()) { - badgeColor = isModernTreeSitter(grammar) ? + if (isLegacyTreeSitterMode()) { + badgeColor = isLegacyTreeSitter(grammar) ? 'badge-success' : 'badge-warning'; - badgeText = isModernTreeSitter(grammar) ? + badgeText = isLegacyTreeSitter(grammar) ? 'Tree-sitter' : 'Legacy Tree-sitter'; } @@ -118,13 +118,15 @@ module.exports = class GrammarListView { return grammar !== atom.grammars.nullGrammar && grammar.name; }); - // Don't show modern tree-sitter grammars in the selector unless the user + // Don't show legacy Tree-sitter grammars in the selector unless the user // has opted into it. - if (!isExperimentalTreeSitterMode()) { - grammars = grammars.filter(grammar => !isModernTreeSitter(grammar)); + 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)) { @@ -155,24 +157,24 @@ module.exports = class GrammarListView { function getLanguageModeConfig() { let isTreeSitterMode = atom.config.get('core.useTreeSitterParsers'); - let isExperimental = atom.config.get('core.useExperimentalModernTreeSitter'); + let isLegacy = atom.config.get('core.useLegacyTreeSitter'); if (!isTreeSitterMode) return 'textmate'; - return isExperimental ? 'wasm-tree-sitter' : 'node-tree-sitter'; + return isLegacy ? 'node-tree-sitter' : 'wasm-tree-sitter'; } -function isExperimentalTreeSitterMode() { - return getLanguageModeConfig() === 'wasm-tree-sitter'; +function isLegacyTreeSitterMode() { + return getLanguageModeConfig() === 'node-tree-sitter'; } function isTreeSitter(grammar) { - return isOldTreeSitter(grammar) || isModernTreeSitter(grammar); + return isLegacyTreeSitter(grammar) || isModernTreeSitter(grammar); } function isModernTreeSitter(grammar) { return grammar.constructor.name === 'WASMTreeSitterGrammar'; } -function isOldTreeSitter(grammar) { +function isLegacyTreeSitter(grammar) { return grammar.constructor.name === 'TreeSitterGrammar'; } @@ -183,6 +185,6 @@ function compareGrammarType(a, b) { function getGrammarScore(grammar) { let languageParser = getLanguageModeConfig(); if (isModernTreeSitter(grammar)) { return -2; } - if (isOldTreeSitter(grammar)) { return -1; } + if (isLegacyTreeSitter(grammar)) { return -1; } return languageParser === 'textmate' ? -3 : 0; } diff --git a/packages/grammar-selector/spec/.eslintrc.js b/packages/grammar-selector/spec/.eslintrc.js new file mode 100644 index 0000000000..746f7afb7a --- /dev/null +++ b/packages/grammar-selector/spec/.eslintrc.js @@ -0,0 +1,8 @@ +module.exports = { + env: { jasmine: true }, + rules: { + "node/no-unpublished-require": "off", + "no-unused-vars": "off", + "no-empty": "off" + } +}; diff --git a/packages/grammar-selector/spec/grammar-selector-spec.js b/packages/grammar-selector/spec/grammar-selector-spec.js index 4f13eadbe8..118c09dd1c 100644 --- a/packages/grammar-selector/spec/grammar-selector-spec.js +++ b/packages/grammar-selector/spec/grammar-selector-spec.js @@ -3,9 +3,9 @@ const SelectListView = require('atom-select-list'); function setConfigForLanguageMode(mode) { let useTreeSitterParsers = mode !== 'textmate'; - let useExperimentalModernTreeSitter = mode === 'wasm-tree-sitter'; + let useLegacyTreeSitter = mode === 'node-tree-sitter'; atom.config.set('core.useTreeSitterParsers', useTreeSitterParsers); - atom.config.set('core.useExperimentalModernTreeSitter', useExperimentalModernTreeSitter); + atom.config.set('core.useLegacyTreeSitter', useLegacyTreeSitter); } describe('GrammarSelector', () => { diff --git a/packages/language-clojure/spec/.eslintrc.js b/packages/language-clojure/spec/.eslintrc.js new file mode 100644 index 0000000000..5226d69213 --- /dev/null +++ b/packages/language-clojure/spec/.eslintrc.js @@ -0,0 +1,9 @@ +module.exports = { + env: { jasmine: true }, + rules: { + "node/no-unpublished-require": "off", + "node/no-extraneous-require": "off", + "no-unused-vars": "off", + "no-empty": "off" + } +}; diff --git a/packages/language-clojure/spec/clojure-spec.js b/packages/language-clojure/spec/clojure-spec.js index bf7cb0a465..8f58b9a3ae 100644 --- a/packages/language-clojure/spec/clojure-spec.js +++ b/packages/language-clojure/spec/clojure-spec.js @@ -4,7 +4,6 @@ describe("Clojure grammar", function() { beforeEach(function() { atom.config.set('core.useTreeSitterParsers', false); - atom.config.set('core.useExperimentalModernTreeSitter', false); waitsForPromise(() => atom.packages.activatePackage("language-clojure")); diff --git a/packages/language-clojure/spec/tokenizer-spec.js b/packages/language-clojure/spec/tokenizer-spec.js index 9699663208..a12e608511 100644 --- a/packages/language-clojure/spec/tokenizer-spec.js +++ b/packages/language-clojure/spec/tokenizer-spec.js @@ -3,9 +3,7 @@ const path = require('path'); function setConfigForLanguageMode(mode) { let useTreeSitterParsers = mode !== 'textmate'; - let useExperimentalModernTreeSitter = mode === 'modern-tree-sitter'; atom.config.set('core.useTreeSitterParsers', useTreeSitterParsers); - atom.config.set('core.useExperimentalModernTreeSitter', useExperimentalModernTreeSitter); } describe('Clojure grammars', () => { diff --git a/packages/language-html/spec/tree-sitter-spec.js b/packages/language-html/spec/tree-sitter-spec.js index 8fd99ae7b7..973dd45973 100644 --- a/packages/language-html/spec/tree-sitter-spec.js +++ b/packages/language-html/spec/tree-sitter-spec.js @@ -3,7 +3,7 @@ const dedent = require('dedent'); describe('Tree-sitter HTML grammar', () => { beforeEach(async () => { atom.config.set('core.useTreeSitterParsers', true); - atom.config.set('core.useExperimentalModernTreeSitter', false); + atom.config.set('core.useLegacyTreeSitter', true); await atom.packages.activatePackage('language-html'); }); diff --git a/packages/language-java/spec/.eslintrc.js b/packages/language-java/spec/.eslintrc.js new file mode 100644 index 0000000000..0e63132582 --- /dev/null +++ b/packages/language-java/spec/.eslintrc.js @@ -0,0 +1,12 @@ +module.exports = { + env: { jasmine: true }, + globals: { + waitsForPromise: true + }, + rules: { + "node/no-unpublished-require": "off", + "node/no-extraneous-require": "off", + "no-unused-vars": "off", + "no-empty": "off" + } +}; diff --git a/packages/language-java/spec/tree-sitter-java-spec.js b/packages/language-java/spec/tree-sitter-java-spec.js index b187082fde..af12e6a044 100644 --- a/packages/language-java/spec/tree-sitter-java-spec.js +++ b/packages/language-java/spec/tree-sitter-java-spec.js @@ -8,7 +8,7 @@ describe('Tree-sitter based Java grammar', function() { beforeEach(function() { atom.config.set('core.useTreeSitterParsers', true); - atom.config.set('core.useExperimentalModernTreeSitter', false); + atom.config.set('core.useLegacyTreeSitter', true); waitsForPromise(() => atom.packages.activatePackage('language-java')); diff --git a/packages/language-ruby/spec/.eslintrc.js b/packages/language-ruby/spec/.eslintrc.js new file mode 100644 index 0000000000..36c4dcd826 --- /dev/null +++ b/packages/language-ruby/spec/.eslintrc.js @@ -0,0 +1,15 @@ +module.exports = { + env: { jasmine: true }, + globals: { + waitsForPromise: true, + runGrammarTests: true, + runFoldsTests: true, + normalizeTreeSitterTextData: true + }, + rules: { + "node/no-unpublished-require": "off", + "node/no-extraneous-require": "off", + "no-unused-vars": "off", + "no-empty": "off" + } +}; diff --git a/packages/language-ruby/spec/tokenizer-spec.js b/packages/language-ruby/spec/tokenizer-spec.js index 5960207d3f..7cf8d71fdb 100644 --- a/packages/language-ruby/spec/tokenizer-spec.js +++ b/packages/language-ruby/spec/tokenizer-spec.js @@ -16,7 +16,7 @@ describe('Ruby grammars', () => { xit('tokenizes the editor using node tree-sitter parser', async () => { atom.config.set('core.useTreeSitterParsers', true); - atom.config.set('core.useExperimentalModernTreeSitter', false); + atom.config.set('core.useLegacyTreeSitter', true); await runGrammarTests(path.join(__dirname, 'fixtures', 'textmate-grammar.rb'), /#/) }); }); diff --git a/packages/language-ruby/spec/tree-sitter-spec.js b/packages/language-ruby/spec/tree-sitter-spec.js index 7fce8a75f3..01d243dc5d 100644 --- a/packages/language-ruby/spec/tree-sitter-spec.js +++ b/packages/language-ruby/spec/tree-sitter-spec.js @@ -3,7 +3,7 @@ const dedent = require('dedent'); describe('Tree-sitter Ruby grammar', () => { beforeEach(async () => { atom.config.set('core.useTreeSitterParsers', true); - atom.config.set('core.useExperimentalModernTreeSitter', false); + atom.config.set('core.useLegacyTreeSitter', true); await atom.packages.activatePackage('language-ruby'); }); diff --git a/packages/language-ruby/spec/wasm-tree-sitter-spec.js b/packages/language-ruby/spec/wasm-tree-sitter-spec.js index 711b5aec1c..10755671a6 100644 --- a/packages/language-ruby/spec/wasm-tree-sitter-spec.js +++ b/packages/language-ruby/spec/wasm-tree-sitter-spec.js @@ -7,7 +7,6 @@ xdescribe('WASM Tree-sitter Ruby grammar', () => { beforeEach(async () => { await atom.packages.activatePackage('language-ruby'); atom.config.set('core.useTreeSitterParsers', true); - atom.config.set('core.useExperimentalModernTreeSitter', true); }); it('tokenizes symbols', async () => { diff --git a/spec/grammar-registry-spec.js b/spec/grammar-registry-spec.js index 66b3a78cb8..fb5c33b420 100644 --- a/spec/grammar-registry-spec.js +++ b/spec/grammar-registry-spec.js @@ -11,9 +11,9 @@ const { OnigScanner } = SecondMate; // Expects one of `textmate`, `node-tree-sitter`, or `wasm-tree-sitter`. function setConfigForLanguageMode(mode, options = {}) { let useTreeSitterParsers = mode !== 'textmate'; - let useExperimentalModernTreeSitter = mode === 'wasm-tree-sitter'; + let useLegacyTreeSitter = mode === 'node-tree-sitter'; atom.config.set('core.useTreeSitterParsers', useTreeSitterParsers, options); - atom.config.set('core.useExperimentalModernTreeSitter', useExperimentalModernTreeSitter, options); + atom.config.set('core.useLegacyTreeSitter', useLegacyTreeSitter, options); } describe('GrammarRegistry', () => { diff --git a/spec/scope-resolver-spec.js b/spec/scope-resolver-spec.js index f3701241d8..ec6fc7f7aa 100644 --- a/spec/scope-resolver-spec.js +++ b/spec/scope-resolver-spec.js @@ -93,7 +93,7 @@ function rangeFromDescriptor(rawRange) { } describe('ScopeResolver', () => { - let editor, buffer, grammar, scopeResolver; + let editor, buffer, grammar; beforeEach(async () => { grammar = new WASMTreeSitterGrammar(atom.grammars, jsGrammarPath, jsConfig); @@ -101,7 +101,6 @@ describe('ScopeResolver', () => { buffer = editor.getBuffer(); atom.grammars.addGrammar(grammar); atom.config.set('core.useTreeSitterParsers', true); - atom.config.set('core.useExperimentalModernTreeSitter', true); }); afterEach(() => { @@ -126,7 +125,7 @@ describe('ScopeResolver', () => { let { scopeResolver, captures } = await getAllCaptures(grammar, languageMode); for (let capture of captures) { - let { node, name } = capture; + let { node } = capture; let range = scopeResolver.store(capture); expect(stringForNodeRange(range)) .toBe(stringForNodeRange(node)); diff --git a/spec/tree-sitter-language-mode-spec.js b/spec/tree-sitter-language-mode-spec.js index 32a1e2c123..889e7e30a7 100644 --- a/spec/tree-sitter-language-mode-spec.js +++ b/spec/tree-sitter-language-mode-spec.js @@ -42,7 +42,7 @@ describe('TreeSitterLanguageMode', () => { buffer = editor.getBuffer(); editor.displayLayer.reset({ foldCharacter: '…' }); atom.config.set('core.useTreeSitterParsers', true); - atom.config.set('core.useExperimentalModernTreeSitter', false); + atom.config.set('core.useLegacyTreeSitter', true); }); describe('highlighting', () => { diff --git a/spec/wasm-tree-sitter-language-mode-spec.js b/spec/wasm-tree-sitter-language-mode-spec.js index 9937880c8c..dcc78a60e9 100644 --- a/spec/wasm-tree-sitter-language-mode-spec.js +++ b/spec/wasm-tree-sitter-language-mode-spec.js @@ -66,7 +66,6 @@ describe('WASMTreeSitterLanguageMode', () => { buffer = editor.getBuffer(); editor.displayLayer.reset({ foldCharacter: '…' }); atom.config.set('core.useTreeSitterParsers', true); - atom.config.set('core.useExperimentalModernTreeSitter', true); }); afterEach(() => { diff --git a/src/config-schema.js b/src/config-schema.js index 29108d6a96..52ed1c9652 100644 --- a/src/config-schema.js +++ b/src/config-schema.js @@ -360,13 +360,14 @@ const configSchema = { useTreeSitterParsers: { type: 'boolean', default: true, + title: 'Use Tree-sitter Parsers', description: 'Use Tree-sitter parsers for supported languages.' }, - useExperimentalModernTreeSitter: { + useLegacyTreeSitter: { type: 'boolean', default: false, - title: 'Use Modern Tree-Sitter Implementation', - description: 'Experimental: Use the new query-file-based Tree-sitter system instead of the legacy system from Atom. (This system will eventually replace the legacy system.) Has no effect unless "Use Tree Sitter Parsers" is also checked.' + title: 'Use legacy Tree-sitter Implementation', + description: 'Opt into the legacy Atom Tree-sitter system instead of the modern system added by Pulsar. (This legacy system will soon be removed.) Has no effect unless “Use Tree-sitter Parsers” is also checked.' }, colorProfile: { description: diff --git a/src/grammar-registry.js b/src/grammar-registry.js index f4852d70e2..c5eeeb8d2b 100644 --- a/src/grammar-registry.js +++ b/src/grammar-registry.js @@ -46,7 +46,7 @@ module.exports = class GrammarRegistry { this.textmateRegistry.onDidUpdateGrammar(grammarAddedOrUpdated); let onLanguageModeChange = () => { - this.grammarScoresByBuffer.forEach((score, buffer) => { + this.grammarScoresByBuffer.forEach((_score, buffer) => { if (!this.languageOverridesByBufferId.has(buffer.id)) { this.autoAssignLanguageMode(buffer); } @@ -55,7 +55,7 @@ module.exports = class GrammarRegistry { this.subscriptions.add( this.config.onDidChange('core.useTreeSitterParsers', onLanguageModeChange), - this.config.onDidChange('core.useExperimentalModernTreeSitter', onLanguageModeChange) + this.config.onDidChange('core.useLegacyTreeSitter', onLanguageModeChange) ); } @@ -253,10 +253,10 @@ module.exports = class GrammarRegistry { scope = new ScopeDescriptor({ scopes: [scope] }) } let useTreeSitterParsers = this.config.get('core.useTreeSitterParsers', { scope }); - let useExperimentalModernTreeSitter = this.config.get('core.useExperimentalModernTreeSitter', { scope }); + let useLegacyTreeSitter = this.config.get('core.useLegacyTreeSitter', { scope }); if (!useTreeSitterParsers) return 'textmate'; - return useExperimentalModernTreeSitter ? 'wasm-tree-sitter' : 'node-tree-sitter'; + return useLegacyTreeSitter ? 'node-tree-sitter' : 'wasm-tree-sitter'; } // Extended: Returns a {Number} representing how well the grammar matches the @@ -288,19 +288,15 @@ module.exports = class GrammarRegistry { if (isNewTreeSitter) { if (parserConfig === 'wasm-tree-sitter') { score += 0.1; - } else { - // Never automatically switch to a new Tree-sitter grammar unless the - // user has opted into the experimental setting. - score = -Infinity; } } else if (isOldTreeSitter) { if (parserConfig === 'node-tree-sitter') { score += 0.1; } else if (parserConfig === 'wasm-tree-sitter') { - // In experimental new-tree-sitter mode, we still would rather fall - // back to an old-tree-sitter grammar than a TM grammar. Bump the + // If `useLegacyTreeSitter` isn't checked, we probably still prefer a + // legacy Tree-sitter grammar over a TextMate-style grammar. Bump the // score, but just a bit less than we'd bump it if this were a - // new-tree-sitter grammar. + // modern Tree-sitter grammar. score += 0.09; } } @@ -770,17 +766,17 @@ module.exports = class GrammarRegistry { let result = this.textmateRegistry.getGrammars(); if (!(params && params.includeTreeSitter)) return result; - let includeModernTreeSitterGrammars = - atom.config.get('core.useExperimentalModernTreeSitter') === true; + let includeLegacyTreeSitterGrammars = + atom.config.get('core.useLegacyTreeSitter') === true; - const tsGrammars = Object.values(this.treeSitterGrammarsById) + let modernTsGrammars = Object.values(this.wasmTreeSitterGrammarsById) .filter(g => g.scopeName); - result = result.concat(tsGrammars); + result = result.concat(modernTsGrammars); - if (includeModernTreeSitterGrammars) { - let modernTsGrammars = Object.values(this.wasmTreeSitterGrammarsById) + if (includeLegacyTreeSitterGrammars) { + const legacyTsGrammars = Object.values(this.treeSitterGrammarsById) .filter(g => g.scopeName); - result = result.concat(modernTsGrammars); + result = result.concat(legacyTsGrammars); } return result; @@ -790,8 +786,10 @@ module.exports = class GrammarRegistry { return this.textmateRegistry.scopeForId(id); } - // TODO: why is this being used? Can we remove it soon? - treeSitterGrammarForLanguageString(languageString, type = 'original') { + // Match up a language string (of the sort generated by an injection point) + // with a grammar. Checks the `injectionRegex` property on grammars and + // returns the one with the longest match. + treeSitterGrammarForLanguageString(languageString, type = 'wasm') { let longestMatchLength = 0; let grammarWithLongestMatch = null; let table = type === 'original' ? this.treeSitterGrammarsById : this.wasmTreeSitterGrammarsById; From a6b5f23b2668447845b115699f3571d0fee4a50f Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 6 Jan 2024 16:24:44 -0800 Subject: [PATCH 02/26] =?UTF-8?q?Get=20the=20`grammar-selector`=20tests=20?= =?UTF-8?q?passing=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …including a lot more tests for the new `useLegacyTreeSitter` setting. --- .../grammar-selector/lib/grammar-list-view.js | 11 +- .../spec/grammar-selector-spec.js | 175 ++++++++++++++++-- 2 files changed, 172 insertions(+), 14 deletions(-) diff --git a/packages/grammar-selector/lib/grammar-list-view.js b/packages/grammar-selector/lib/grammar-list-view.js index 8980a18ce7..aa5c0a0d8f 100644 --- a/packages/grammar-selector/lib/grammar-list-view.js +++ b/packages/grammar-selector/lib/grammar-list-view.js @@ -27,10 +27,11 @@ module.exports = class GrammarListView { let badgeText = 'Tree-sitter'; if (isLegacyTreeSitterMode()) { + // Color the legacy badge green to represent the user's preference. badgeColor = isLegacyTreeSitter(grammar) ? 'badge-success' : 'badge-warning'; badgeText = isLegacyTreeSitter(grammar) ? - 'Tree-sitter' : 'Legacy Tree-sitter'; + 'Legacy Tree-sitter' : 'Modern Tree-sitter'; } parser.classList.add( @@ -184,7 +185,11 @@ function compareGrammarType(a, b) { function getGrammarScore(grammar) { let languageParser = getLanguageModeConfig(); - if (isModernTreeSitter(grammar)) { return -2; } - if (isLegacyTreeSitter(grammar)) { return -1; } + if (isModernTreeSitter(grammar)) { + return languageParser === 'node-tree-sitter' ? -1 : -2; + } + if (isLegacyTreeSitter(grammar)) { + return languageParser === 'node-tree-sitter' ? -2 : -1; + } return languageParser === 'textmate' ? -3 : 0; } diff --git a/packages/grammar-selector/spec/grammar-selector-spec.js b/packages/grammar-selector/spec/grammar-selector-spec.js index 118c09dd1c..00dd36bfdc 100644 --- a/packages/grammar-selector/spec/grammar-selector-spec.js +++ b/packages/grammar-selector/spec/grammar-selector-spec.js @@ -224,7 +224,7 @@ describe('GrammarSelector', () => { } }); - it('shows both if false (in proper order when language parser is node-tree-sitter)', async () => { + it('shows all three if false (in proper order when language parser is node-tree-sitter)', async () => { await atom.packages.activatePackage('language-c'); // punctuation making it sort wrong setConfigForLanguageMode('node-tree-sitter'); atom.config.set( @@ -239,10 +239,14 @@ describe('GrammarSelector', () => { const grammar = listItems[i]; const name = grammar.name; if (cppCount === 0 && name === 'C++') { - // first C++ entry should be Tree-sitter + // first C++ entry should be legacy Tree-sitter expect(grammar.constructor.name).toBe('TreeSitterGrammar'); cppCount++; } else if (cppCount === 1) { + // 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('Grammar'); @@ -252,7 +256,38 @@ describe('GrammarSelector', () => { } } - expect(cppCount).toBe(2); // ensure we actually saw both grammars + 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 () => { + await atom.packages.activatePackage('language-c'); // punctuation making it sort wrong + setConfigForLanguageMode('wasm-tree-sitter'); + 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++') { + // first C++ entry should be modern Tree-sitter + expect(grammar.constructor.name).toBe('WASMTreeSitterGrammar'); + cppCount++; + } else if (cppCount === 1) { + // immediate next grammar should be the TextMate version + expect(name).toBe('C++'); + expect(grammar.constructor.name).toBe('Grammar'); + cppCount++; + } else { + expect(name).not.toBe('C++'); // there should not be any other C++ grammars + } + } + + expect(cppCount).toBe(2); // ensure we actually saw two grammars }); it('shows both if false (in proper order when language parser is textmate)', async () => { @@ -275,7 +310,7 @@ describe('GrammarSelector', () => { } else if (cppCount === 1) { // immediate next grammar should be the Tree-sitter version expect(name).toBe('C++'); - expect(grammar.constructor.name).toBe('TreeSitterGrammar'); + expect(grammar.constructor.name).toBe('WASMTreeSitterGrammar'); cppCount++; } else { expect(name).not.toBe('C++'); // there should not be any other C++ grammars @@ -333,7 +368,125 @@ describe('GrammarSelector', () => { }); describe('when grammar-selector:show is triggered', () => { - it('displays a list of all the available grammars', async () => { + it('displays a list of all the available grammars ', async () => { + 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 + .getGrammars({ includeTreeSitter: true }) + .filter(g => g.name).length + ); + expect(grammarView.querySelectorAll('li')[0].textContent).toBe( + 'Auto Detect' + ); + expect(grammarView.textContent.includes('source.a')).toBe(false); + grammarView + .querySelectorAll('li') + .forEach(li => + expect(li.textContent).not.toBe(atom.grammars.nullGrammar.name) + ); + // 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 + atom.config.set('grammar-selector.hideDuplicateTextMateGrammars', true); + const grammarView = await getGrammarView(editor); + const observedNames = new Map(); + // Show a maximum of one grammar (the modern tree-sitter variant). + 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('WASMTreeSitterGrammar'); + } + } + }); + + 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('WASMTreeSitterGrammar'); // first C++ entry should be Modern 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 three 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++) { + 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(true); + } + if (item.constructor.name === 'TreeSitterGrammar') { + expect( + element.childNodes[1].childNodes[0].classList.contains('badge-warning') + ).toBe(true); + } else if (item.constructor.name === 'WASMTreeSitterGrammar') { + expect( + element.childNodes[1].childNodes[0].classList.contains('badge-success') + ).toBe(true); + } + } + }); + }); + + }); + + // We will not need these tests for long. + describe('when language parser is "node-tree-sitter"', () => { + beforeEach(() => { + setConfigForLanguageMode('node-tree-sitter'); + }); + + describe('when grammar-selector:show is triggered', () => { + it('displays a list of all the available grammars ', async () => { const grammarView = (await getGrammarView(editor)).element; // -1 for removing nullGrammar, +1 for adding "Auto Detect" @@ -354,9 +507,10 @@ describe('GrammarSelector', () => { ); // Ensure we're showing and labelling tree-sitter grammars… expect(grammarView.textContent.includes('Tree-sitter')).toBe(true); - // …and old tree-sitter grammars. + // …and also old tree-sitter grammars. expect(grammarView.textContent.includes('Legacy Tree-sitter')).toBe(true); }); + }); describe('when toggling hideDuplicateTextMateGrammars', () => { @@ -399,11 +553,10 @@ describe('GrammarSelector', () => { const grammar = listItems[i]; const name = grammar.name; if (cppCount === 0 && name === 'C++') { - expect(grammar.constructor.name).toBe('WASMTreeSitterGrammar'); // first C++ entry should be Modern Tree-sitter + expect(grammar.constructor.name).toBe('TreeSitterGrammar'); // first C++ entry should be Modern Tree-sitter cppCount++; } else if (cppCount === 1) { - expect(name).toBe('C++'); - expect(grammar.constructor.name).toBe('TreeSitterGrammar'); // second C++ entry should be Legacy Tree-sitter + expect(grammar.constructor.name).toBe('WASMTreeSitterGrammar'); // next C++ entry should be legacy Tree-sitter cppCount++; } else if (cppCount === 2) { expect(name).toBe('C++'); @@ -435,11 +588,11 @@ describe('GrammarSelector', () => { } if (item.constructor.name === 'TreeSitterGrammar') { expect( - element.childNodes[1].childNodes[0].classList.contains('badge-warning') + element.childNodes[1].childNodes[0].classList.contains('badge-success') ).toBe(true); } else if (item.constructor.name === 'WASMTreeSitterGrammar') { expect( - element.childNodes[1].childNodes[0].classList.contains('badge-success') + element.childNodes[1].childNodes[0].classList.contains('badge-warning') ).toBe(true); } } From 2dfa4fd792e8f0cd922f713f20968504be44e7d7 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 6 Jan 2024 16:28:16 -0800 Subject: [PATCH 03/26] =?UTF-8?q?Enable=20legacy=20flag=20for=20`autocompl?= =?UTF-8?q?ete-html`=20tests=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …but also these specs need to be rewritten! --- packages/autocomplete-html/spec/provider-spec.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/autocomplete-html/spec/provider-spec.js b/packages/autocomplete-html/spec/provider-spec.js index ee3588df45..459da47a38 100644 --- a/packages/autocomplete-html/spec/provider-spec.js +++ b/packages/autocomplete-html/spec/provider-spec.js @@ -33,6 +33,7 @@ describe('HTML autocompletions', () => { } beforeEach(() => { + atom.config.set('core.useLegacyTreeSitter', true) waitsForPromise(() => atom.packages.activatePackage('autocomplete-html')) waitsForPromise(() => atom.packages.activatePackage('language-html')) waitsForPromise(() => atom.workspace.open('test.html')) From a12e75b8c1ac29661cd4b675d99a1a0da3c84f90 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sat, 6 Jan 2024 17:43:52 -0800 Subject: [PATCH 04/26] Get `autocomplete-html` specs passing in modern Tree-sitter mode --- packages/autocomplete-html/lib/main.js | 3 +- .../lib/text-mate-provider.js | 26 ++- .../autocomplete-html/spec/provider-spec.js | 153 ++++++++++++------ .../grammars/tree-sitter-html/highlights.scm | 7 +- 4 files changed, 138 insertions(+), 51 deletions(-) diff --git a/packages/autocomplete-html/lib/main.js b/packages/autocomplete-html/lib/main.js index ee0da53861..e2e16c4f60 100644 --- a/packages/autocomplete-html/lib/main.js +++ b/packages/autocomplete-html/lib/main.js @@ -9,7 +9,8 @@ const provider = { getSuggestions (request) { try { - if (request.editor.getBuffer().getLanguageMode().tree) { + let languageMode = request.editor.getBuffer().getLanguageMode(); + if (languageMode.constructor.name === 'TreeSitterLanguageMode') { return getSuggestionsWithTreeSitter(request) } else { return getSuggestionsWithTextMate(request) diff --git a/packages/autocomplete-html/lib/text-mate-provider.js b/packages/autocomplete-html/lib/text-mate-provider.js index 8ed5965527..479176e186 100644 --- a/packages/autocomplete-html/lib/text-mate-provider.js +++ b/packages/autocomplete-html/lib/text-mate-provider.js @@ -50,7 +50,7 @@ function isTagStart ({prefix, scopeDescriptor, bufferPosition, editor}) { function isAttributeStart ({prefix, scopeDescriptor, bufferPosition, editor}) { const scopes = scopeDescriptor.getScopesArray() if (!getPreviousAttribute(editor, bufferPosition) && prefix && !prefix.trim()) { - return hasTagScope(scopes) + return hasTagScope(scopes) || afterTagScope(editor, bufferPosition) } const previousBufferPosition = [bufferPosition.row, Math.max(0, bufferPosition.column - 1)] @@ -68,6 +68,25 @@ function isAttributeStart ({prefix, scopeDescriptor, bufferPosition, editor}) { ) } +// This fixes the +// +//
` on the tag, so we should +// move back to the nearest text and try to read the scopes from there. +// Designed to work no matter how many spaces there are between the end of the +// tag name and the cursor. +function afterTagScope (editor, bufferPosition) { + let cursor = editor.getCursors().find(cursor => { + return cursor.getBufferPosition().isEqual(bufferPosition) + }) + if (!cursor) return false; + let position = cursor.getPreviousWordBoundaryBufferPosition(); + position = position.translate([0, -1]); + let scopes = editor.scopeDescriptorForBufferPosition(position); + return scopes.getScopesArray().some(t => t.startsWith('entity.name.tag')); +} + function isAttributeValueStart ({scopeDescriptor, bufferPosition, editor}) { const scopes = scopeDescriptor.getScopesArray() @@ -75,6 +94,11 @@ function isAttributeValueStart ({scopeDescriptor, bufferPosition, editor}) { const previousScopes = editor.scopeDescriptorForBufferPosition(previousBufferPosition) const previousScopesArray = previousScopes.getScopesArray() + // This is an unambiguous case — if the cursor is on the right side of the + // opening quote, then we must be in the right place. + if (previousScopesArray.includes('punctuation.definition.string.begin.html')) + return true + // autocomplete here: attribute="|" // not here: attribute=|"" // or here: attribute=""| diff --git a/packages/autocomplete-html/spec/provider-spec.js b/packages/autocomplete-html/spec/provider-spec.js index 459da47a38..89afcf528e 100644 --- a/packages/autocomplete-html/spec/provider-spec.js +++ b/packages/autocomplete-html/spec/provider-spec.js @@ -1,5 +1,5 @@ describe('HTML autocompletions', () => { - let editor, provider + let editor, provider, languageMode function getCompletions () { const cursor = editor.getLastCursor() @@ -33,51 +33,63 @@ describe('HTML autocompletions', () => { } beforeEach(() => { - atom.config.set('core.useLegacyTreeSitter', true) waitsForPromise(() => atom.packages.activatePackage('autocomplete-html')) waitsForPromise(() => atom.packages.activatePackage('language-html')) waitsForPromise(() => atom.workspace.open('test.html')) + waitsForPromise(() => { + let editor = atom.workspace.getActiveTextEditor() + let languageMode = editor.getBuffer().getLanguageMode() + return languageMode.ready + }) runs(() => provider = atom.packages.getActivePackage('autocomplete-html').mainModule.getProvider()) runs(() => editor = atom.workspace.getActiveTextEditor()) + runs(() => languageMode = editor.getBuffer().getLanguageMode()) }) - it('returns no completions when not at the start of a tag', () => { + it('returns no completions when not at the start of a tag', async () => { editor.setText('') + await languageMode.atTransactionEnd() expect(getCompletions().length).toBe(0) editor.setText('d') editor.setCursorBufferPosition([0, 0]) + await languageMode.atTransactionEnd() + expect(getCompletions().length).toBe(0) + editor.setCursorBufferPosition([0, 1]) expect(getCompletions().length).toBe(0) }) - it('returns no completions in style tags', () => { + it('returns no completions in style tags', async () => { editor.setText(`\ \ ` ) + await languageMode.atTransactionEnd() editor.setCursorBufferPosition([1, 1]) expect(getCompletions().length).toBe(0) }) - it('returns no completions in script tags', () => { + it('returns no completions in script tags', async () => { editor.setText(`\ \ ` ) + await languageMode.atTransactionEnd() editor.setCursorBufferPosition([1, 1]) expect(getCompletions().length).toBe(0) }) - it('autcompletes tag names without a prefix', () => { + it('autcompletes tag names without a prefix', async () => { editor.setText('<') editor.setCursorBufferPosition([0, 1]) + await languageMode.atTransactionEnd() const completions = getCompletions() expect(completions.length).toBeGreaterThan(113) // Fun Fact last check this was 232 @@ -91,9 +103,10 @@ describe('HTML autocompletions', () => { } }) - it('autocompletes tag names with a prefix', () => { + it('autocompletes tag names with a prefix', async () => { editor.setText(' { expect(isValueInCompletions('dt', completions)).toBe(true) }) - it("does not autocomplete tag names if there's a space after the <", () => { + it("does not autocomplete tag names if there's a space after the <", async () => { editor.setText('< ') editor.setCursorBufferPosition([0, 2]) + await languageMode.atTransactionEnd() let completions = getCompletions() expect(completions.length).toBe(0) editor.setText('< h') editor.setCursorBufferPosition([0, 2]) + await languageMode.atTransactionEnd() completions = getCompletions() expect(completions.length).toBe(0) }) - it('does not provide a descriptionMoreURL if the tag does not have a unique description', () => { + it('does not provide a descriptionMoreURL if the tag does not have a unique description', async () => { // isindex does not have an associated MDN page as of March 25, 2023 editor.setText(' { expect(completions[loc].descriptionMoreURL).toBeNull() }) - it('autocompletes attribute names without a prefix', () => { + it('autocompletes attribute names without a prefix', async () => { editor.setText('
{ editor.setText(' { editor.setText('
') editor.setCursorBufferPosition([0, 5]) + await languageMode.atTransactionEnd() completions = getCompletions() expect(completions.length).toBeGreaterThan(0) @@ -194,6 +213,7 @@ describe('HTML autocompletions', () => { editor.setText('
') editor.setCursorBufferPosition([0, 5]) + await languageMode.atTransactionEnd() completions = getCompletions() expect(completions.length).toBeGreaterThan(0) @@ -202,9 +222,10 @@ describe('HTML autocompletions', () => { } }) - it('autocompletes attribute names with a prefix', () => { + it('autocompletes attribute names with a prefix', async () => { editor.setText('
{ editor.setText('
{ editor.setText('
') editor.setCursorBufferPosition([0, 6]) + await languageMode.atTransactionEnd() completions = getCompletions() expect(completions.length).toBeGreaterThan(3) @@ -239,6 +262,7 @@ describe('HTML autocompletions', () => { editor.setText('
') editor.setCursorBufferPosition([0, 6]) + await languageMode.atTransactionEnd() completions = getCompletions() expect(completions.length).toBeGreaterThan(3) @@ -249,6 +273,7 @@ describe('HTML autocompletions', () => { editor.setText(' { editor.setText(' { + it('autocompletes attribute names without a prefix surrounded by whitespace', async () => { editor.setText(' { + it("respects the 'flag' type when autocompleting attribute names", async () => { editor.setText(' { + it('does not autocomplete attribute names outside of a tag', async () => { editor.setText('') editor.setCursorBufferPosition([0, 0]) @@ -325,18 +358,20 @@ describe('HTML autocompletions', () => { expect(getCompletions().length).toBe(0) }) - it('does not throw when a local attribute is not in the attributes list', () => { + it('does not throw when a local attribute is not in the attributes list', async () => { // Some tags, like body, have local attributes that are not present in the top-level attributes array editor.setText(' { + it('does not provide a descriptionMoreURL if the attribute does not have a unique description', async () => { editor.setText(' { expect(completions[loc].descriptionMoreURL).toBeNull() }) - it('autocompletes attribute values without a prefix', () => { + it('autocompletes attribute values without a prefix', async () => { editor.setText(' { expect(completions[1].text).toBe('slide') expect(completions[2].text).toBe('alternate') - editor.setText(' { expect(completions[2].text).toBe('alternate') }) - it('autocompletes attribute values with a prefix', () => { + it('autocompletes attribute values with a prefix', async () => { editor.setText(' { editor.setText(' { editor.setText(' { expect(completions[5].text).toBe('es') }) - it('autocompletes ambiguous attribute values', () => { + it('autocompletes ambiguous attribute values', async () => { editor.setText('