From 351f96d5ddd30e52bca757828ac12c9fce66c4c1 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 6 Nov 2017 09:36:06 -0800 Subject: [PATCH] Iterate on GrammarRegistry APIs Signed-off-by: Nathan Sobo --- spec/grammar-registry-spec.js | 140 ++++++++++++++++++++++++++++++ spec/text-editor-registry-spec.js | 66 -------------- src/grammar-registry.js | 92 +++++++++++++------- src/project.js | 4 +- src/text-editor.js | 4 +- src/tokenized-buffer.js | 4 + 6 files changed, 210 insertions(+), 100 deletions(-) create mode 100644 spec/grammar-registry-spec.js diff --git a/spec/grammar-registry-spec.js b/spec/grammar-registry-spec.js new file mode 100644 index 000000000..75828cb35 --- /dev/null +++ b/spec/grammar-registry-spec.js @@ -0,0 +1,140 @@ +const {it, fit, ffit, fffit, beforeEach, afterEach, conditionPromise, timeoutPromise} = require('./async-spec-helpers') + +const path = require('path') +const TextBuffer = require('text-buffer') +const GrammarRegistry = require('../src/grammar-registry') + +describe('GrammarRegistry', () => { + let grammarRegistry + + beforeEach(() => { + grammarRegistry = new GrammarRegistry({config: atom.config}) + }) + + describe('.assignLanguageMode(buffer, languageName)', () => { + it('assigns to the buffer a language mode with the given language name', async () => { + grammarRegistry.loadGrammarSync(require.resolve('language-javascript/grammars/javascript.cson')) + grammarRegistry.loadGrammarSync(require.resolve('language-css/grammars/css.cson')) + + const buffer = new TextBuffer() + expect(grammarRegistry.assignLanguageMode(buffer, 'javascript')).toBe(true) + expect(buffer.getLanguageMode().getLanguageName()).toBe('JavaScript') + + // Returns true if we found the grammar, even if it didn't change + expect(grammarRegistry.assignLanguageMode(buffer, 'javascript')).toBe(true) + + // Language names are not case-sensitive + expect(grammarRegistry.assignLanguageMode(buffer, 'css')).toBe(true) + expect(buffer.getLanguageMode().getLanguageName()).toBe('CSS') + + // Returns false if no language is found + expect(grammarRegistry.assignLanguageMode(buffer, 'blub')).toBe(false) + expect(buffer.getLanguageMode().getLanguageName()).toBe('CSS') + }) + + describe('when no languageName is passed', () => { + it('assigns to the buffer a language mode based on the best available grammar', () => { + grammarRegistry.loadGrammarSync(require.resolve('language-javascript/grammars/javascript.cson')) + grammarRegistry.loadGrammarSync(require.resolve('language-css/grammars/css.cson')) + + const buffer = new TextBuffer() + buffer.setPath('foo.js') + expect(grammarRegistry.assignLanguageMode(buffer, 'css')).toBe(true) + expect(buffer.getLanguageMode().getLanguageName()).toBe('CSS') + + expect(grammarRegistry.assignLanguageMode(buffer, null)).toBe(true) + expect(buffer.getLanguageMode().getLanguageName()).toBe('JavaScript') + }) + }) + }) + + describe('.maintainLanguageMode', () => { + it('assigns a grammar to the buffer based on its path', async () => { + const buffer = new TextBuffer() + + grammarRegistry.loadGrammarSync(require.resolve('language-javascript/grammars/javascript.cson')) + grammarRegistry.loadGrammarSync(require.resolve('language-c/grammars/c.cson')) + + buffer.setPath('test.js') + grammarRegistry.maintainLanguageMode(buffer) + expect(buffer.getLanguageMode().getLanguageName()).toBe('JavaScript') + + buffer.setPath('test.c') + expect(buffer.getLanguageMode().getLanguageName()).toBe('C') + }) + + it('updates the buffer\'s grammar when a more appropriate grammar is added for its path', async () => { + const buffer = new TextBuffer() + expect(buffer.getLanguageMode().getLanguageName()).toBe('None') + + buffer.setPath('test.js') + grammarRegistry.maintainLanguageMode(buffer) + + grammarRegistry.loadGrammarSync(require.resolve('language-javascript/grammars/javascript.cson')) + expect(buffer.getLanguageMode().getLanguageName()).toBe('JavaScript') + }) + + it('can be overridden by calling .assignLanguageMode', () => { + const buffer = new TextBuffer() + expect(buffer.getLanguageMode().getLanguageName()).toBe('None') + + buffer.setPath('test.js') + grammarRegistry.maintainLanguageMode(buffer) + + grammarRegistry.loadGrammarSync(require.resolve('language-css/grammars/css.cson')) + expect(grammarRegistry.assignLanguageMode(buffer, 'css')).toBe(true) + expect(buffer.getLanguageMode().getLanguageName()).toBe('CSS') + + grammarRegistry.loadGrammarSync(require.resolve('language-javascript/grammars/javascript.cson')) + expect(buffer.getLanguageMode().getLanguageName()).toBe('CSS') + }) + + it('returns a disposable that can be used to stop the registry from updating the buffer', async () => { + const buffer = new TextBuffer() + grammarRegistry.loadGrammarSync(require.resolve('language-javascript/grammars/javascript.cson')) + + const previousSubscriptionCount = buffer.emitter.getTotalListenerCount() + const disposable = grammarRegistry.maintainLanguageMode(buffer) + expect(buffer.emitter.getTotalListenerCount()).toBeGreaterThan(previousSubscriptionCount) + expect(retainedBufferCount(grammarRegistry)).toBe(1) + + buffer.setPath('test.js') + expect(buffer.getLanguageMode().getLanguageName()).toBe('JavaScript') + + buffer.setPath('test.txt') + expect(buffer.getLanguageMode().getLanguageName()).toBe('Null Grammar') + + disposable.dispose() + expect(buffer.emitter.getTotalListenerCount()).toBe(previousSubscriptionCount) + expect(retainedBufferCount(grammarRegistry)).toBe(0) + + buffer.setPath('test.js') + expect(buffer.getLanguageMode().getLanguageName()).toBe('Null Grammar') + expect(retainedBufferCount(grammarRegistry)).toBe(0) + }) + + describe('when called twice with a given buffer', () => { + it('does nothing the second time', async () => { + const buffer = new TextBuffer() + grammarRegistry.loadGrammarSync(require.resolve('language-javascript/grammars/javascript.cson')) + const disposable1 = grammarRegistry.maintainLanguageMode(buffer) + const disposable2 = grammarRegistry.maintainLanguageMode(buffer) + + buffer.setPath('test.js') + expect(buffer.getLanguageMode().getLanguageName()).toBe('JavaScript') + + disposable2.dispose() + buffer.setPath('test.txt') + expect(buffer.getLanguageMode().getLanguageName()).toBe('Null Grammar') + + disposable1.dispose() + buffer.setPath('test.js') + expect(buffer.getLanguageMode().getLanguageName()).toBe('Null Grammar') + }) + }) + }) +}) + +function retainedBufferCount (grammarRegistry) { + return grammarRegistry.grammarScoresByBuffer.size +} diff --git a/spec/text-editor-registry-spec.js b/spec/text-editor-registry-spec.js index 017ef1f1b..d7ef829ea 100644 --- a/spec/text-editor-registry-spec.js +++ b/spec/text-editor-registry-spec.js @@ -76,72 +76,6 @@ describe('TextEditorRegistry', function () { }) }) - describe('.maintainGrammar', function () { - it('assigns a grammar to the editor based on its path', async function () { - await atom.packages.activatePackage('language-javascript') - await atom.packages.activatePackage('language-c') - - editor.getBuffer().setPath('test.js') - registry.maintainGrammar(editor) - - expect(editor.getGrammar().name).toBe('JavaScript') - - editor.getBuffer().setPath('test.c') - expect(editor.getGrammar().name).toBe('C') - }) - - it('updates the editor\'s grammar when a more appropriate grammar is added for its path', async function () { - expect(editor.getGrammar().name).toBe('Null Grammar') - - editor.getBuffer().setPath('test.js') - registry.maintainGrammar(editor) - await atom.packages.activatePackage('language-javascript') - expect(editor.getGrammar().name).toBe('JavaScript') - }) - - it('returns a disposable that can be used to stop the registry from updating the editor', async function () { - await atom.packages.activatePackage('language-javascript') - - const previousSubscriptionCount = getSubscriptionCount(editor) - const disposable = registry.maintainGrammar(editor) - expect(getSubscriptionCount(editor)).toBeGreaterThan(previousSubscriptionCount) - expect(registry.editorsWithMaintainedGrammar.size).toBe(1) - - editor.getBuffer().setPath('test.js') - expect(editor.getGrammar().name).toBe('JavaScript') - - editor.getBuffer().setPath('test.txt') - expect(editor.getGrammar().name).toBe('Null Grammar') - - disposable.dispose() - expect(getSubscriptionCount(editor)).toBe(previousSubscriptionCount) - expect(registry.editorsWithMaintainedGrammar.size).toBe(0) - - editor.getBuffer().setPath('test.js') - expect(editor.getGrammar().name).toBe('Null Grammar') - expect(retainedEditorCount(registry)).toBe(0) - }) - - describe('when called twice with a given editor', function () { - it('does nothing the second time', async function () { - await atom.packages.activatePackage('language-javascript') - const disposable1 = registry.maintainGrammar(editor) - const disposable2 = registry.maintainGrammar(editor) - - editor.getBuffer().setPath('test.js') - expect(editor.getGrammar().name).toBe('JavaScript') - - disposable2.dispose() - editor.getBuffer().setPath('test.txt') - expect(editor.getGrammar().name).toBe('Null Grammar') - - disposable1.dispose() - editor.getBuffer().setPath('test.js') - expect(editor.getGrammar().name).toBe('Null Grammar') - }) - }) - }) - describe('.setGrammarOverride', function () { it('sets the editor\'s grammar and does not update it based on other criteria', async function () { await atom.packages.activatePackage('language-c') diff --git a/src/grammar-registry.js b/src/grammar-registry.js index 7c33c4b73..c1fb33803 100644 --- a/src/grammar-registry.js +++ b/src/grammar-registry.js @@ -1,4 +1,5 @@ const _ = require('underscore-plus') +const Grim = require('grim') const FirstMate = require('first-mate') const {Disposable, CompositeDisposable} = require('event-kit') const TokenizedBuffer = require('./tokenized-buffer') @@ -7,7 +8,7 @@ const fs = require('fs-plus') const {Point, Range} = require('text-buffer') const GRAMMAR_SELECTION_RANGE = Range(Point.ZERO, Point(10, 0)).freeze() -const PathSplitRegex = new RegExp('[/.]') +const PATH_SPLIT_REGEX = new RegExp('[/.]') // Extended: Syntax class holding the grammars used for tokenizing. // @@ -20,7 +21,7 @@ class GrammarRegistry extends FirstMate.GrammarRegistry { constructor ({config} = {}) { super({maxTokensPerLine: 100, maxLineLength: 1000}) this.config = config - this.grammarOverridesByPath = {} + this.languageNameOverridesByBufferId = new Map() this.grammarScoresByBuffer = new Map() this.subscriptions = new CompositeDisposable() @@ -33,40 +34,60 @@ class GrammarRegistry extends FirstMate.GrammarRegistry { return new Token({value, scopes}) } - maintainGrammar (buffer) { - this.assignLanguageModeToBuffer(buffer) + maintainLanguageMode (buffer) { + const languageNameOverride = this.languageNameOverridesByBufferId.get(buffer.id) + if (languageNameOverride) { + this.assignLanguageMode(buffer, languageNameOverride) + } else { + this.assignLanguageMode(buffer, null) + } const pathChangeSubscription = buffer.onDidChangePath(() => { this.grammarScoresByBuffer.delete(buffer) - this.assignLanguageModeToBuffer(buffer) + if (!this.languageNameOverridesByBufferId.has(buffer.id)) { + this.assignLanguageMode(buffer, null) + } }) this.subscriptions.add(pathChangeSubscription) return new Disposable(() => { this.subscriptions.remove(pathChangeSubscription) + this.grammarScoresByBuffer.delete(buffer) pathChangeSubscription.dispose() }) } - assignLanguageModeToBuffer (buffer) { - let grammar = null - const overrideScopeName = this.grammarOverridesByPath[buffer.getPath()] - if (overrideScopeName) { - grammar = this.grammarForScopeName(overrideScopeName) - this.grammarScoresByBuffer.set(buffer, null) - } + assignLanguageMode (buffer, languageName) { + if (buffer.getBuffer) buffer = buffer.getBuffer() - if (!grammar) { + let grammar + if (languageName != null) { + const lowercaseLanguageName = languageName.toLowerCase() + grammar = this.grammarForLanguageName(lowercaseLanguageName) + this.languageNameOverridesByBufferId.set(buffer.id, lowercaseLanguageName) + this.grammarScoresByBuffer.set(buffer, null) + } else { const result = this.selectGrammarWithScore( buffer.getPath(), buffer.getTextInRange(GRAMMAR_SELECTION_RANGE) ) - grammar = result.grammar - this.grammarScoresByBuffer.set(buffer, result.score) + const currentScore = this.grammarScoresByBuffer.get(buffer) + if (currentScore == null || result.score > currentScore) { + grammar = result.grammar + this.languageNameOverridesByBufferId.delete(buffer.id) + this.grammarScoresByBuffer.set(buffer, result.score) + } } - buffer.setLanguageMode(this.languageModeForGrammarAndBuffer(grammar, buffer)) + if (grammar) { + if (grammar.name !== buffer.getLanguageMode().getLanguageName()) { + buffer.setLanguageMode(this.languageModeForGrammarAndBuffer(grammar, buffer)) + } + return true + } else { + return false + } } languageModeForGrammarAndBuffer (grammar, buffer) { @@ -120,7 +141,7 @@ class GrammarRegistry extends FirstMate.GrammarRegistry { if (!filePath) { return -1 } if (process.platform === 'win32') { filePath = filePath.replace(/\\/g, '/') } - const pathComponents = filePath.toLowerCase().split(PathSplitRegex) + const pathComponents = filePath.toLowerCase().split(PATH_SPLIT_REGEX) let pathScore = -1 let customFileTypes @@ -135,7 +156,7 @@ class GrammarRegistry extends FirstMate.GrammarRegistry { for (let i = 0; i < fileTypes.length; i++) { const fileType = fileTypes[i] - const fileTypeComponents = fileType.toLowerCase().split(PathSplitRegex) + const fileTypeComponents = fileType.toLowerCase().split(PATH_SPLIT_REGEX) const pathSuffix = pathComponents.slice(-fileTypeComponents.length) if (_.isEqual(pathSuffix, fileTypeComponents)) { pathScore = Math.max(pathScore, fileType.length) @@ -170,23 +191,30 @@ class GrammarRegistry extends FirstMate.GrammarRegistry { return grammar.firstLineRegex.testSync(lines.slice(0, numberOfNewlinesInRegex + 1).join('\n')) } - // Get the grammar override for the given file path. + // Deprecated: Get the grammar override for the given file path. // // * `filePath` A {String} file path. // // Returns a {String} such as `"source.js"`. grammarOverrideForPath (filePath) { - return this.grammarOverridesByPath[filePath] + Grim.deprecate('Use buffer.getLanguageMode().getLanguageName() instead') + const buffer = atom.project.findBufferForPath(filePath) + if (buffer) return this.languageNameOverridesByBufferId.get(buffer.id) } - // Set the grammar override for the given file path. + // Deprecated: Set the grammar override for the given file path. // // * `filePath` A non-empty {String} file path. // * `scopeName` A {String} such as `"source.js"`. // // Returns undefined. setGrammarOverrideForPath (filePath, scopeName) { - this.grammarOverridesByPath[filePath] = scopeName + Grim.deprecate('Use atom.grammars.assignLanguageMode(buffer, languageName) instead') + const buffer = atom.project.findBufferForPath(filePath) + if (buffer) { + const grammar = this.grammarForScopeName(scopeName) + if (grammar) this.languageNameOverridesByBufferId.set(buffer.id, grammar.name) + } } // Remove the grammar override for the given file path. @@ -195,7 +223,14 @@ class GrammarRegistry extends FirstMate.GrammarRegistry { // // Returns undefined. clearGrammarOverrideForPath (filePath) { - delete this.grammarOverridesByPath[filePath] + Grim.deprecate('Use atom.grammars.assignLanguageMode(buffer, null) instead') + const buffer = atom.project.findBufferForPath(filePath) + if (buffer) this.languageNameOverridesByBufferId.delete(buffer.id) + } + + grammarForLanguageName (languageName) { + const lowercaseLanguageName = languageName.toLowerCase() + return this.getGrammars().find(grammar => grammar.name.toLowerCase() === lowercaseLanguageName) } grammarAddedOrUpdated (grammar) { @@ -208,19 +243,16 @@ class GrammarRegistry extends FirstMate.GrammarRegistry { return } - const grammarOverride = this.grammarOverridesByPath[buffer.getPath()] - if (grammarOverride) { - if (grammar.scopeName === grammarOverride) { - buffer.setLanguageMode(this.languageModeForGrammarAndBuffer(grammar, buffer)) - } - } else { + if (grammar.name === buffer.getLanguageMode().getLanguageName()) { + buffer.setLanguageMode(this.languageModeForGrammarAndBuffer(grammar, buffer)) + } else if (!this.languageNameOverridesByBufferId.has(buffer.id)) { const score = this.getGrammarScore( grammar, buffer.getPath(), buffer.getTextInRange(GRAMMAR_SELECTION_RANGE) ) - let currentScore = this.grammarScoresByBuffer.get(buffer) + const currentScore = this.grammarScoresByBuffer.get(buffer) if (currentScore == null || score > currentScore) { buffer.setLanguageMode(this.languageModeForGrammarAndBuffer(grammar, buffer)) this.grammarScoresByBuffer.set(buffer, score) diff --git a/src/project.js b/src/project.js index fefd911b5..e5afd9eee 100644 --- a/src/project.js +++ b/src/project.js @@ -106,7 +106,7 @@ class Project extends Model { return Promise.all(bufferPromises).then(buffers => { this.buffers = buffers.filter(Boolean) for (let buffer of this.buffers) { - this.grammarRegistry.maintainGrammar(buffer) + this.grammarRegistry.maintainLanguageMode(buffer) this.subscribeToBuffer(buffer) } this.setPaths(state.paths || [], {mustExist: true, exact: true}) @@ -650,7 +650,7 @@ class Project extends Model { addBuffer (buffer, options = {}) { this.buffers.push(buffer) - this.grammarRegistry.maintainGrammar(buffer) + this.grammarRegistry.maintainLanguageMode(buffer) this.subscribeToBuffer(buffer) this.emitter.emit('did-add-buffer', buffer) return buffer diff --git a/src/text-editor.js b/src/text-editor.js index e96e6244b..16fa7dff4 100644 --- a/src/text-editor.js +++ b/src/text-editor.js @@ -3556,8 +3556,8 @@ class TextEditor { // // * `grammar` {Grammar} setGrammar (grammar) { - Grim.deprecate('Use atom.grammars.setGrammarOverrideForPath(filePath) instead') - atom.grammars.setGrammarOverrideForPath(this.getPath(), grammar.scopeName) + Grim.deprecate('Use atom.grammars.assignLanguageMode(buffer, languageName) instead') + atom.grammars.assignLanguageMode(this.getBuffer(), grammar.name) } // Experimental: Get a notification when async tokenization is completed. diff --git a/src/tokenized-buffer.js b/src/tokenized-buffer.js index afc109495..614b22970 100644 --- a/src/tokenized-buffer.js +++ b/src/tokenized-buffer.js @@ -55,6 +55,10 @@ class TokenizedBuffer { return this.grammar } + getLanguageName () { + return this.grammar.name + } + getNonWordCharacters (scope) { return this.config.get('editor.nonWordCharacters', {scope}) }