From 61395289e9bdbc9444fc0367b9ab32cc79430d0c Mon Sep 17 00:00:00 2001 From: Dennis Kehrig Date: Wed, 27 Feb 2013 14:02:47 +0100 Subject: [PATCH 01/14] When defining a language, setting a mode is now required. In addition, defineLanguage now returns a promise that resolves once the mode has been loaded and set, instead of returning the language directly. To make things more consistent both within LanguageManager and with the rest of the code base, chaining support has been removed. --- src/extensions/default/LESSSupport/main.js | 2 +- src/language/LanguageManager.js | 81 +++++++++------------- src/language/languages.json | 3 +- 3 files changed, 35 insertions(+), 51 deletions(-) diff --git a/src/extensions/default/LESSSupport/main.js b/src/extensions/default/LESSSupport/main.js index 54a1a4bc179..ef233d4b3ee 100644 --- a/src/extensions/default/LESSSupport/main.js +++ b/src/extensions/default/LESSSupport/main.js @@ -30,7 +30,7 @@ define(function (require, exports, module) { var LanguageManager = brackets.getModule("language/LanguageManager"); - var language = LanguageManager.defineLanguage("less", { + LanguageManager.defineLanguage("less", { name: "LESS", mode: "less", fileExtensions: ["less"], diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index dfabc6de84e..fe9600b4fe9 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -34,7 +34,7 @@ * var language = LanguageManager.getLanguage(""); * * To define your own languages, call defineLanguage(): - * var language = LanguageManager.defineLanguage("haskell", { + * LanguageManager.defineLanguage("haskell", { * name: "Haskell", * mode: "haskell", * fileExtensions: ["hs"], @@ -42,7 +42,13 @@ * lineComment: "--" * }); * + * To use that language, wait for the returned promise to be resolved: + * LanguageManager.defineLanguage("haskell", definition).done(function (language) { + * console.log("Language " + language.name + " is now available!"); + * }); + * * You can also refine an existing language. Currently you can only set the comment styles: + * var language = LanguageManager.getLanguage("haskell"); * language.setLineComment("--"); * language.setBlockComment("{-", "-}"); * @@ -74,12 +80,6 @@ * * If a mode is not shipped with our CodeMirror distribution, you need to first load it yourself. * If the mode is part of our CodeMirror distribution, it gets loaded automatically. - * - * To wait until the mode is loaded and set, use the language.modeReady promise: - * language.modeReady.done(function () { - * // ... - * }); - * Note that this will never resolve for languages without a mode. */ define(function (require, exports, module) { "use strict"; @@ -226,10 +226,6 @@ define(function (require, exports, module) { this._fileExtensions = []; this._modeToLanguageMap = {}; - // Since setting the mode is asynchronous when the mode hasn't been loaded yet, offer a reliable way to wait until it is ready - this._modeReady = new $.Deferred(); - this.modeReady = this._modeReady.promise(); - _languages[id] = this; } @@ -239,25 +235,21 @@ define(function (require, exports, module) { /** @type {string} Human-readable name of the language */ Language.prototype.name = null; - /** @type {$.Promise} Promise that resolves when the mode has been loaded and set */ - Language.prototype.modeReady = null; - /** - * Sets the mode for this language. + * Loads a mode and sets it for this language. * - * @param {string|Array.} definition.mode CodeMirror mode (i.e. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] - * Unless the mode is located in thirdparty/CodeMirror2/mode//.js, you need to first load it yourself. - * @return {Language} This language + * @param {string|Array.} mode CodeMirror mode (i.e. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] + * Unless the mode is located in thirdparty/CodeMirror2/mode//.js, you need to first load it yourself. + * + * @return {$.Promise} A promise object that will be resolved when the mode is loaded and set */ - Language.prototype._setMode = function (mode) { - if (!mode) { - return; - } + Language.prototype._loadAndSetMode = function (mode) { + var result = new $.Deferred(); var language = this; // Mode can be an array specifying a mode plus a MIME mode defined by that mode ["clike", "text/x-c++src"] var mimeMode; - if ($.isArray(mode)) { + if (Array.isArray(mode)) { if (mode.length !== 2) { throw new Error("Mode must either be a string or an array containing two strings"); } @@ -265,12 +257,12 @@ define(function (require, exports, module) { mode = mode[0]; } - _validateNonEmptyString(mode, "mode"); + _validateString(mode, "mode"); var finish = function () { var i; - if (!CodeMirror.modes[mode]) { + if (mode !== "" && !CodeMirror.modes[mode]) { throw new Error("CodeMirror mode \"" + mode + "\" is not loaded"); } @@ -289,14 +281,16 @@ define(function (require, exports, module) { language.mode = mimeMode || mode; _setLanguageForMode(language.mode, language); - language._modeReady.resolve(language); + result.resolve(); }; - if (CodeMirror.modes[mode]) { + if (mode === "" || CodeMirror.modes[mode]) { finish(); } else { require(["thirdparty/CodeMirror2/mode/" + mode + "/" + mode], finish); } + + return result.promise(); }; /** @@ -313,7 +307,6 @@ define(function (require, exports, module) { * In case we ever open this up, we should think about whether we want to make this * configurable by the user. If so, the user has to be able to override these calls. * @param {!string} extension A file extension used by this language - * @return {Language} This language * @private */ Language.prototype._addFileExtension = function (extension) { @@ -328,15 +321,12 @@ define(function (require, exports, module) { _fileExtensionsToLanguageMap[extension] = this; } } - - return this; }; /** * Sets the prefix and suffix to use for blocks comments in this language. * @param {!string} prefix Prefix string to use for block comments (i.e. "") - * @return {Language} This language * @private */ Language.prototype.setBlockComment = function (prefix, suffix) { @@ -344,22 +334,17 @@ define(function (require, exports, module) { _validateNonEmptyString(suffix, "suffix"); this.blockComment = { prefix: prefix, suffix: suffix }; - - return this; }; /** * Sets the prefix to use for line comments in this language. * @param {!string} prefix Prefix string to use for block comments (i.e. "//") - * @return {Language} This language * @private */ Language.prototype.setLineComment = function (prefix) { _validateNonEmptyString(prefix, "prefix"); this.lineComment = { prefix: prefix }; - - return this; }; /** @@ -381,7 +366,6 @@ define(function (require, exports, module) { * Used to disambiguate modes used by multiple languages. * @param {!string} mode The mode to associate the language with * @param {!Language} language The language to associate with the mode - * @return {Language} This language * @private */ Language.prototype._setLanguageForMode = function (mode, language) { @@ -389,8 +373,6 @@ define(function (require, exports, module) { throw new Error("A language must always map its mode to itself"); } this._modeToLanguageMap[mode] = language; - - return this; }; @@ -405,12 +387,12 @@ define(function (require, exports, module) { * @param {string} definition.lineComment Line comment prefix (i.e. "//") * @param {string|Array.} definition.mode CodeMirror mode (i.e. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] * Unless the mode is located in thirdparty/CodeMirror2/mode//.js, you need to first load it yourself. - * {@link Language#modeReady} is a promise that resolves when a mode has been loaded and set. - * It will not resolve when no mode is specified. * - * @return {Language} The new language + * @return {$.Promise} A promise object that will be resolved with a Language object **/ function defineLanguage(id, definition) { + var result = new $.Deferred(); + var language = new Language(id, definition.name); var fileExtensions = definition.fileExtensions, @@ -431,14 +413,15 @@ define(function (require, exports, module) { language.setLineComment(lineComment); } - var mode = definition.mode; - if (mode) { - language._setMode(mode); - } else { - language._modeReady.reject(); - } + language._loadAndSetMode(definition.mode) + .done(function () { + result.resolve(language); + }) + .fail(function (error) { + result.reject(error); + }); - return language; + return result.promise(); } diff --git a/src/language/languages.json b/src/language/languages.json index 361e7292a99..2be7db13f98 100644 --- a/src/language/languages.json +++ b/src/language/languages.json @@ -1,6 +1,7 @@ { "unknown": { - "name": "Text" + "name": "Text", + "mode": "" }, "css": { From 3dceb961facbec0c2cd55a9c80da7c30a1927ee3 Mon Sep 17 00:00:00 2001 From: Dennis Kehrig Date: Wed, 27 Feb 2013 14:03:58 +0100 Subject: [PATCH 02/14] Fix: language IDs were documented as foo_bar, but validated as foo.bar --- src/language/LanguageManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index fe9600b4fe9..f5054e42477 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -211,7 +211,7 @@ define(function (require, exports, module) { _validateString(id, "Language ID"); // Make sure the ID is a string that can safely be used universally by the computer - as a file name, as an object key, as part of a URL, etc. // Hence we use _ instead of "." since this makes it easier to parse a file name containing a language ID - if (!id.match(/^[a-z]+(\.[a-z]+)*$/)) { + if (!id.match(/^[a-z]+(_[a-z]+)*$/)) { throw new Error("Invalid language ID \"" + id + "\": Only groups of letters a-z are allowed, separated by _ (i.e. \"cpp\" or \"foo_bar\")"); } if (_languages[id]) { From 38a015cf20566f9f02c727dc4e154ca58e85a5bc Mon Sep 17 00:00:00 2001 From: Dennis Kehrig Date: Wed, 27 Feb 2013 14:10:42 +0100 Subject: [PATCH 03/14] Normalize file extensions by also removing leading dots if necessary --- src/language/LanguageManager.js | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index f5054e42477..dcbba6bfd62 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -122,6 +122,23 @@ define(function (require, exports, module) { } } + /** + * Makes the file extension all lowercase and ensures it doesn't start with a dot + * @param {!string} extension The file extension + * @return {string} The normalized file extension + */ + function _normalizeFileExtension(extension) { + // Remove a leading dot if present + if (extension.charAt(0) === ".") { + extension = extension.substr(1); + } + + // Make checks below case-INsensitive + extension = extension.toLowerCase(); + + return extension; + } + /** * Monkey-patch CodeMirror to prevent modes from being overwritten by extensions. * We may rely on the tokens provided by some of these modes. @@ -141,7 +158,6 @@ define(function (require, exports, module) { * Adds a global mode-to-language association. * @param {!string} mode The mode to associate the language with * @param {!Language} language The language to associate with the mode - * @private */ function _setLanguageForMode(mode, language) { if (_modeToLanguageMap[mode]) { @@ -168,13 +184,7 @@ define(function (require, exports, module) { */ function getLanguageForFileExtension(path) { var extension = PathUtils.filenameExtension(path); - - if (extension.charAt(0) === ".") { - extension = extension.substr(1); - } - - // Make checks below case-INsensitive - extension = extension.toLowerCase(); + extension = _normalizeFileExtension(extension); var language = _fileExtensionsToLanguageMap[extension]; if (!language) { @@ -310,7 +320,8 @@ define(function (require, exports, module) { * @private */ Language.prototype._addFileExtension = function (extension) { - extension = extension.toLowerCase(); + extension = _normalizeFileExtension(extension); + if (this._fileExtensions.indexOf(extension) === -1) { this._fileExtensions.push(extension); @@ -327,7 +338,6 @@ define(function (require, exports, module) { * Sets the prefix and suffix to use for blocks comments in this language. * @param {!string} prefix Prefix string to use for block comments (i.e. "") - * @private */ Language.prototype.setBlockComment = function (prefix, suffix) { _validateNonEmptyString(prefix, "prefix"); @@ -339,7 +349,6 @@ define(function (require, exports, module) { /** * Sets the prefix to use for line comments in this language. * @param {!string} prefix Prefix string to use for block comments (i.e. "//") - * @private */ Language.prototype.setLineComment = function (prefix) { _validateNonEmptyString(prefix, "prefix"); From 37be3d98b8c350f0a917902f4ad6a5310d09c079 Mon Sep 17 00:00:00 2001 From: Dennis Kehrig Date: Wed, 27 Feb 2013 14:17:32 +0100 Subject: [PATCH 04/14] Use CollectionUtils instead of $.each --- src/language/LanguageManager.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index dcbba6bfd62..9da27b52f90 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -86,7 +86,8 @@ define(function (require, exports, module) { // Dependencies - var _defaultLanguagesJSON = require("text!language/languages.json"); + var CollectionUtils = require("utils/CollectionUtils"), + _defaultLanguagesJSON = require("text!language/languages.json"); // State @@ -447,7 +448,9 @@ define(function (require, exports, module) { }); // Load the default languages - $.each(JSON.parse(_defaultLanguagesJSON), defineLanguage); + CollectionUtils.forEach(JSON.parse(_defaultLanguagesJSON), function (definition, id) { + defineLanguage(id, definition); + }); // Get the object for HTML var html = getLanguage("html"); From 8793b2afbc161216b6475d661466e4b84b31776a Mon Sep 17 00:00:00 2001 From: Dennis Kehrig Date: Wed, 27 Feb 2013 18:46:22 +0100 Subject: [PATCH 05/14] Language now encapsulates its properties and makes them accessible exclusively through getters and setters Adjust the unit tests accordingly Added documentation to Language's properties Refined the comment about our special HTML MIME mode Renamed fileExtensionsToLanguageMap to fileExtensionToLanguageMap --- src/editor/Editor.js | 2 +- src/editor/EditorCommandHandlers.js | 13 +- .../JavaScriptCodeHints/ScopeManager.js | 2 +- src/language/LanguageManager.js | 156 ++++++++++++++---- test/spec/Editor-test.js | 29 ++-- test/spec/EditorCommandHandlers-test.js | 6 +- test/spec/LanguageManager-test.js | 85 ++++++---- 7 files changed, 206 insertions(+), 87 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index a8509aab2cf..96d5bf34a94 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -456,7 +456,7 @@ define(function (require, exports, module) { // We'd like undefined/null/"" to mean plain text mode. CodeMirror defaults to plaintext for any // unrecognized mode, but it complains on the console in that fallback case: so, convert // here so we're always explicit, avoiding console noise. - return this.document.getLanguage().mode || "text/plain"; + return this.document.getLanguage().getMode() || "text/plain"; }; diff --git a/src/editor/EditorCommandHandlers.js b/src/editor/EditorCommandHandlers.js index 99668d3b456..5907b61517a 100644 --- a/src/editor/EditorCommandHandlers.js +++ b/src/editor/EditorCommandHandlers.js @@ -468,8 +468,9 @@ define(function (require, exports, module) { var language = editor.getLanguageForSelection(); - if (language.blockComment) { - blockCommentPrefixSuffix(editor, language.blockComment.prefix, language.blockComment.suffix, language.lineComment ? language.lineComment.prefix : null); + if (language.hasBlockCommentSyntax()) { + // getLineCommentPrefix returns null if no line comment syntax is defined + blockCommentPrefixSuffix(editor, language.getBlockCommentPrefix(), language.getBlockCommentSuffix(), language.getLineCommentPrefix()); } } @@ -485,10 +486,10 @@ define(function (require, exports, module) { var language = editor.getLanguageForSelection(); - if (language.lineComment) { - lineCommentPrefix(editor, language.lineComment.prefix); - } else if (language.blockComment) { - lineCommentPrefixSuffix(editor, language.blockComment.prefix, language.blockComment.suffix); + if (language.hasLineCommentSyntax()) { + lineCommentPrefix(editor, language.getLineCommentPrefix()); + } else if (language.hasBlockCommentSyntax()) { + lineCommentPrefixSuffix(editor, language.getBlockCommentPrefix(), language.getBlockCommentSuffix()); } } diff --git a/src/extensions/default/JavaScriptCodeHints/ScopeManager.js b/src/extensions/default/JavaScriptCodeHints/ScopeManager.js index 180e09deb1e..55bf00450db 100644 --- a/src/extensions/default/JavaScriptCodeHints/ScopeManager.js +++ b/src/extensions/default/JavaScriptCodeHints/ScopeManager.js @@ -440,7 +440,7 @@ define(function (require, exports, module) { file = split.file; if (file.indexOf(".") > 1) { // ignore /.dotfiles - var mode = LanguageManager.getLanguageForFileExtension(entry.fullPath).mode; + var mode = LanguageManager.getLanguageForFileExtension(entry.fullPath).getMode(); if (mode === HintUtils.MODE_NAME) { DocumentManager.getDocumentForPath(path).done(function (document) { refreshOuterScope(dir, file, document.getText()); diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 9da27b52f90..1c807f49b43 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -62,7 +62,7 @@ * ... * }); * Definining the base mode is still necessary to know which file to load. - * Later however, language.mode will either refer to the MIME mode, + * Later however, language.getMode() will either refer to the MIME mode, * or the base mode if no MIME mode has been specified. * * If you need to configure a mode, you can just create a new MIME mode and use that: @@ -91,10 +91,10 @@ define(function (require, exports, module) { // State - var _fallbackLanguage = null, - _languages = {}, - _fileExtensionsToLanguageMap = {}, - _modeToLanguageMap = {}; + var _fallbackLanguage = null, + _languages = {}, + _fileExtensionToLanguageMap = {}, + _modeToLanguageMap = {}; // Helper functions @@ -187,7 +187,7 @@ define(function (require, exports, module) { var extension = PathUtils.filenameExtension(path); extension = _normalizeFileExtension(extension); - var language = _fileExtensionsToLanguageMap[extension]; + var language = _fileExtensionToLanguageMap[extension]; if (!language) { console.log("Called LanguageManager.getLanguageForFileExtension with an unhandled file extension: " + extension); } @@ -231,8 +231,8 @@ define(function (require, exports, module) { _validateNonEmptyString(name, "name"); - this.id = id; - this.name = name; + this._id = id; + this._name = name; this._fileExtensions = []; this._modeToLanguageMap = {}; @@ -241,10 +241,37 @@ define(function (require, exports, module) { } /** @type {string} Identifier for this language */ - Language.prototype.id = null; + Language.prototype._id = null; + + /** + * Returns the identifier for this language. + * @return {string} The identifier + */ + Language.prototype.getId = function () { + return this._id; + }; - /** @type {string} Human-readable name of the language */ - Language.prototype.name = null; + /** @type {string} Human-readable name of this language */ + Language.prototype._name = null; + + /** + * Returns the human-readable name of this language + * @return {string} The name + */ + Language.prototype.getName = function () { + return this._name; + }; + + /** @type {string} CodeMirror mode for this language */ + Language.prototype._mode = null; + + /** + * Returns the CodeMirror mode for this language + * @return {string} The mode + */ + Language.prototype.getMode = function () { + return this._mode; + }; /** * Loads a mode and sets it for this language. @@ -289,8 +316,8 @@ define(function (require, exports, module) { // This mode is now only about what to tell CodeMirror // The base mode was only necessary to load the proper mode file - language.mode = mimeMode || mode; - _setLanguageForMode(language.mode, language); + language._mode = mimeMode || mode; + _setLanguageForMode(language._mode, language); result.resolve(); }; @@ -304,6 +331,9 @@ define(function (require, exports, module) { return result.promise(); }; + /** @type {Array.} File extensions that use this language */ + Language.prototype._fileExtensions = null; + /** * Returns an array of file extensions for this language. * @return {Array.} File extensions used by this language @@ -311,7 +341,7 @@ define(function (require, exports, module) { Language.prototype.getFileExtensions = function () { return this._fileExtensions.concat(); }; - + /** * Adds a file extension to this language. * Private for now since dependent code would need to by kept in sync with such changes. @@ -326,37 +356,97 @@ define(function (require, exports, module) { if (this._fileExtensions.indexOf(extension) === -1) { this._fileExtensions.push(extension); - var language = _fileExtensionsToLanguageMap[extension]; + var language = _fileExtensionToLanguageMap[extension]; if (language) { console.warn("Cannot register file extension \"" + extension + "\" for " + this.name + ", it already belongs to " + language.name); } else { - _fileExtensionsToLanguageMap[extension] = this; + _fileExtensionToLanguageMap[extension] = this; } } }; + /** @type {{ prefix: string }} Line comment syntax */ + Language.prototype._lineCommentSyntax = null; + /** - * Sets the prefix and suffix to use for blocks comments in this language. - * @param {!string} prefix Prefix string to use for block comments (i.e. "") + * Returns whether the line comment syntax is defined for this language + * @return {boolean} Whether line comments are supported */ - Language.prototype.setBlockComment = function (prefix, suffix) { - _validateNonEmptyString(prefix, "prefix"); - _validateNonEmptyString(suffix, "suffix"); - - this.blockComment = { prefix: prefix, suffix: suffix }; + Language.prototype.hasLineCommentSyntax = function () { + return Boolean(this._lineCommentSyntax); + }; + + /** + * Returns the prefix to use for line comments + * @return {string} The prefix + */ + Language.prototype.getLineCommentPrefix = function () { + if (!this._lineCommentSyntax) { + return null; + } + return this._lineCommentSyntax.prefix; }; /** * Sets the prefix to use for line comments in this language. * @param {!string} prefix Prefix string to use for block comments (i.e. "//") */ - Language.prototype.setLineComment = function (prefix) { + Language.prototype.setLineCommentSyntax = function (prefix) { _validateNonEmptyString(prefix, "prefix"); - this.lineComment = { prefix: prefix }; + this._lineCommentSyntax = { prefix: prefix }; }; + + /** @type {{ prefix: string, suffix: string }} Block comment syntax */ + Language.prototype._blockCommentSyntax = null; + + /** + * Returns whether the block comment syntax is defined for this language + * @return {boolean} Whether block comments are supported + */ + Language.prototype.hasBlockCommentSyntax = function () { + return Boolean(this._blockCommentSyntax); + }; + + /** + * Returns the prefix to use for block comments + * @return {string} The prefix + */ + Language.prototype.getBlockCommentPrefix = function () { + if (!this._blockCommentSyntax) { + return null; + } + return this._blockCommentSyntax.prefix; + }; + + /** + * Returns the suffix to use for block comments + * @return {string} The suffix + */ + Language.prototype.getBlockCommentSuffix = function () { + if (!this._blockCommentSyntax) { + return null; + } + return this._blockCommentSyntax.suffix; + }; + + /** + * Sets the prefix and suffix to use for blocks comments in this language. + * @param {!string} prefix Prefix string to use for block comments (i.e. "") + */ + Language.prototype.setBlockCommentSyntax = function (prefix, suffix) { + _validateNonEmptyString(prefix, "prefix"); + _validateNonEmptyString(suffix, "suffix"); + + this._blockCommentSyntax = { prefix: prefix, suffix: suffix }; + }; + + + /** @type {Object.} Which language to use for what CodeMirror mode */ + Language.prototype._modeToLanguageMap = null; + /** * Returns either a language associated with the mode or the fallback language. * Used to disambiguate modes used by multiple languages. @@ -364,7 +454,7 @@ define(function (require, exports, module) { * @return {Language} This language if it uses the mode, or whatever {@link LanguageManager#_getLanguageForMode} returns */ Language.prototype.getLanguageForMode = function (mode) { - if (mode === this.mode) { + if (mode === this._mode) { return this; } @@ -379,7 +469,7 @@ define(function (require, exports, module) { * @private */ Language.prototype._setLanguageForMode = function (mode, language) { - if (mode === this.mode && language !== this) { + if (mode === this._mode && language !== this) { throw new Error("A language must always map its mode to itself"); } this._modeToLanguageMap[mode] = language; @@ -415,12 +505,12 @@ define(function (require, exports, module) { var blockComment = definition.blockComment; if (blockComment) { - language.setBlockComment(blockComment[0], blockComment[1]); + language.setBlockCommentSyntax(blockComment[0], blockComment[1]); } var lineComment = definition.lineComment; if (lineComment) { - language.setLineComment(lineComment); + language.setLineCommentSyntax(lineComment); } language._loadAndSetMode(definition.mode) @@ -438,9 +528,9 @@ define(function (require, exports, module) { // Prevent modes from being overwritten by extensions _patchCodeMirror(); - // Define a custom MIME mode here because JSON files must not contain regular expressions - // Also, all other modes so far were strings, so we spare us the trouble of allowing - // more complex mode values. + // Define a custom MIME mode here instead of putting it directly into languages.json + // because JSON files must not contain regular expressions. Also, all other modes so + // far were strings, so we spare us the trouble of allowing more complex mode values. CodeMirror.defineMIME("text/x-brackets-html", { "name": "htmlmixed", "scriptTypes": [{"matches": /\/x-handlebars-template|\/x-mustache/i, diff --git a/test/spec/Editor-test.js b/test/spec/Editor-test.js index ec58ff68165..cc10e9a1620 100644 --- a/test/spec/Editor-test.js +++ b/test/spec/Editor-test.js @@ -36,29 +36,30 @@ define(function (require, exports, module) { var langNames = { css: {mode: "css", lang: "CSS"}, javascript: {mode: "javascript", lang: "JavaScript"}, - html: {mode: "html", lang: "HTML"} + html: {mode: "html", lang: "HTML"}, + unknown: {mode: null, lang: "Text"} }; function compareMode(expected, actual) { - if (typeof actual === "string") { - return actual === expected; + if (actual && actual.name) { + return actual.name === expected; } - return actual.name === expected; + return actual === expected; } function expectModeAndLang(editor, lang) { expect(editor.getModeForSelection()).toSpecifyModeNamed(lang.mode); - expect(editor.getLanguageForSelection()).toBe(lang.lang); + expect(editor.getLanguageForSelection().getName()).toBe(lang.lang); } describe("Editor", function () { var defaultContent = 'Brackets is going to be awesome!\n'; var myDocument, myEditor; - function createTestEditor(content, mode) { + function createTestEditor(content, languageId) { // create dummy Document and Editor - var mocks = SpecRunnerUtils.createMockEditor(content, mode); + var mocks = SpecRunnerUtils.createMockEditor(content, languageId); myDocument = mocks.doc; myEditor = mocks.editor; } @@ -114,26 +115,26 @@ define(function (require, exports, module) { it("should switch to the HTML mode for files ending in .html", function () { // verify editor content - var mode = LanguageManager.getLanguageForFileExtension("file:///only/testing/the/path.html").mode; + var mode = LanguageManager.getLanguageForFileExtension("file:///only/testing/the/path.html").getMode(); expect(mode).toSpecifyModeNamed("text/x-brackets-html"); }); it("should switch modes even if the url has a query string", function () { // verify editor content - var mode = LanguageManager.getLanguageForFileExtension("http://only.org/testing/the/path.css?v=2").mode; + var mode = LanguageManager.getLanguageForFileExtension("http://only.org/testing/the/path.css?v=2").getMode(); expect(mode).toSpecifyModeNamed(langNames.css.mode); }); it("should accept just a file name too", function () { // verify editor content - var mode = LanguageManager.getLanguageForFileExtension("path.js").mode; + var mode = LanguageManager.getLanguageForFileExtension("path.js").getMode(); expect(mode).toSpecifyModeNamed(langNames.javascript.mode); }); it("should default to plaintext for unknown file extensions", function () { // verify editor content - var mode = LanguageManager.getLanguageForFileExtension("test.foo").mode; - expect(mode).toBe(undefined); + var mode = LanguageManager.getLanguageForFileExtension("test.foo").getMode(); + expect(mode).toBe(""); }); }); @@ -163,7 +164,7 @@ define(function (require, exports, module) { ""; it("should get mode in homogenous file", function () { - createTestEditor(jsContent, langNames.javascript.mode); + createTestEditor(jsContent, "javascript"); // Mode at point myEditor.setCursorPos(0, 0); // first char in text @@ -207,7 +208,7 @@ define(function (require, exports, module) { // Mode for range - mix of modes myEditor.setSelection({line: 2, ch: 4}, {line: 3, ch: 7}); - expectModeAndLang(myEditor, null); + expectModeAndLang(myEditor, langNames.unknown); // Mode for range - mix of modes where start & endpoints are same mode // Known limitation of getModeForSelection() that it does not spot where the mode diff --git a/test/spec/EditorCommandHandlers-test.js b/test/spec/EditorCommandHandlers-test.js index 640041314a2..74495c0fef5 100644 --- a/test/spec/EditorCommandHandlers-test.js +++ b/test/spec/EditorCommandHandlers-test.js @@ -46,12 +46,12 @@ define(function (require, exports, module) { var myDocument, myEditor; - function setupFullEditor(content, mode) { + function setupFullEditor(content, languageId) { content = content || defaultContent; - mode = mode || "javascript"; + languageId = languageId || "javascript"; // create dummy Document and Editor - var mocks = SpecRunnerUtils.createMockEditor(content, mode); + var mocks = SpecRunnerUtils.createMockEditor(content, languageId); myDocument = mocks.doc; myEditor = mocks.editor; diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index b27e2e1965c..81251379e1c 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -47,7 +47,18 @@ define(function (require, exports, module) { def.lineComment = def.lineComment.prefix; } - return LanguageManager.defineLanguage(definition.id, def); + var promise = LanguageManager.defineLanguage(definition.id, def), + language; + + if (definition.id) { + try { + language = LanguageManager.getLanguage(definition.id); + } catch (e) { + // Ignore errors here, this should be tested seperately + } + } + + return { promise: promise, language: language }; } function validateLanguage(expected, actual) { @@ -57,29 +68,24 @@ define(function (require, exports, module) { expect(LanguageManager.getLanguage(expected.id)).toBe(actual); } - expect(actual.id).toBe(expected.id); - expect(actual.name).toBe(expected.name); + expect(actual.getId()).toBe(expected.id); + expect(actual.getName()).toBe(expected.name); expect(actual.getFileExtensions()).toEqual(expected.fileExtensions || []); if (expected.blockComment) { - expect(expected.blockComment.prefix).toBe(actual.blockComment.prefix); - expect(expected.blockComment.suffix).toBe(actual.blockComment.suffix); + expect(actual.hasBlockCommentSyntax()).toBe(true); + expect(actual.getBlockCommentPrefix()).toBe(expected.blockComment.prefix); + expect(actual.getBlockCommentSuffix()).toBe(expected.blockComment.suffix); } else { - expect(actual.blockComment).toBe(undefined); + expect(actual.hasBlockCommentSyntax()).toBe(false); } if (expected.lineComment) { - expect(expected.lineComment.prefix).toBe(actual.lineComment.prefix); - } else { - expect(actual.lineComment).toBe(undefined); - } - - // using async waitsFor is ok if it's the last block in a spec - if (expected.mode) { - waitsForDone(actual.modeReady, '"' + expected.mode + '" mode loading', 10000); + expect(actual.hasLineCommentSyntax()).toBe(true); + expect(actual.getLineCommentPrefix()).toBe(expected.lineComment.prefix); } else { - waitsForFail(actual.modeReady, '"' + expected.mode + '" should not load', 10000); + expect(actual.hasLineCommentSyntax()).toBe(false); } } @@ -110,14 +116,14 @@ define(function (require, exports, module) { describe("LanguageManager API", function () { - it("should map modes to languages", function () { + it("should map identifiers to languages", function () { var html = LanguageManager.getLanguage("html"); expect(html).not.toBe(null); expect(LanguageManager.getLanguage("DoesNotExist")).toBe(undefined); }); - it("should map extensions to languages", function () { + it("should map file extensions to languages", function () { var html = LanguageManager.getLanguage("html"), unknown = LanguageManager.getLanguage("unknown"); @@ -131,10 +137,23 @@ define(function (require, exports, module) { describe("defineLanguage", function () { it("should create a basic language", function () { - var def = { id: "one", name: "One" }, - lang = defineLanguage(def); + var language, + promise, + def = { id: "one", name: "One", mode: "" }; + + runs(function () { + defineLanguage(def).promise.done(function (lang) { + language = lang; + }); + }); - validateLanguage(def, lang); + waitsFor(function () { + return Boolean(language); + }, "The language should be resolved", 50); + + runs(function () { + validateLanguage(def, language); + }); }); it("should throw errors for invalid language id values", function () { @@ -148,9 +167,13 @@ define(function (require, exports, module) { expect(function () { defineLanguage({ id: "three", name: "" }); }).toThrow(new Error("name must not be empty")); }); + it("should throw errors for missing mode value", function () { + expect(function () { defineLanguage({ id: "four", name: "Four" }); }).toThrow(new Error("mode must be a string")); + }); + it("should create a language with file extensions and a mode", function () { var def = { id: "pascal", name: "Pascal", fileExtensions: ["pas", "p"], mode: "pascal" }, - lang = defineLanguage(def); + lang = defineLanguage(def).language; expect(LanguageManager.getLanguageForFileExtension("file.p")).toBe(lang); @@ -160,7 +183,7 @@ define(function (require, exports, module) { it("should allow multiple languages to use the same mode", function () { var xmlBefore = LanguageManager.getLanguage("xml"), def = { id: "wix", name: "WiX", fileExtensions: ["wix"], mode: "xml" }, - lang = defineLanguage(def), + lang = defineLanguage(def).language, xmlAfter = LanguageManager.getLanguage("xml"); expect(xmlBefore).toBe(xmlAfter); @@ -180,11 +203,11 @@ define(function (require, exports, module) { it("should validate comment prefix/suffix", function () { var def = { id: "coldfusion", name: "ColdFusion", fileExtensions: ["cfml", "cfm"], mode: "xml" }, - lang = defineLanguage(def); + lang = defineLanguage(def).language; - expect(function () { lang.setLineComment(""); }).toThrow(new Error("prefix must not be empty")); - expect(function () { lang.setBlockComment(""); }).toThrow(new Error("prefix must not be empty")); + expect(function () { lang.setLineCommentSyntax(""); }).toThrow(new Error("prefix must not be empty")); + expect(function () { lang.setBlockCommentSyntax(""); }).toThrow(new Error("prefix must not be empty")); def.lineComment = { prefix: "//" @@ -194,8 +217,8 @@ define(function (require, exports, module) { suffix: "--->" }; - lang.setLineComment(def.lineComment.prefix); - lang.setBlockComment(def.blockComment.prefix, def.blockComment.suffix); + lang.setLineCommentSyntax(def.lineComment.prefix); + lang.setBlockCommentSyntax(def.blockComment.prefix, def.blockComment.suffix); validateLanguage(def, lang); }); @@ -208,11 +231,15 @@ define(function (require, exports, module) { expect(CodeMirror.modes[id]).toBe(undefined); var def = { id: id, name: "erlang", fileExtensions: ["erlang"], mode: "erlang" }, - lang = defineLanguage(def); + defRes = defineLanguage(def), + lang = defRes.language, + promise = defRes.promise; expect(LanguageManager.getLanguageForFileExtension("file.erlang")).toBe(lang); validateLanguage(def, lang); + + waitsForDone(promise, "should resolve language Erlang", 10000); }); runs(function () { From b5826558052c1660f4ad7125a0351950b736fd72 Mon Sep 17 00:00:00 2001 From: Dennis Kehrig Date: Wed, 27 Feb 2013 19:11:31 +0100 Subject: [PATCH 06/14] Documentation cleanup Call result.reject in a few places instead of throwing exceptions --- src/editor/Editor.js | 4 +-- src/language/LanguageManager.js | 56 +++++++++++++++++++-------------- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index 96d5bf34a94..0de045a3100 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -1208,7 +1208,7 @@ define(function (require, exports, module) { * * @return {?(Object|string)} Name of syntax-highlighting mode, or object containing a "name" property * naming the mode along with configuration options required by the mode. - * See {@link Languages#getLanguageForFileExtension()} and {@link Language#mode}. + * See {@link LanguageManager#getLanguageForFileExtension()} and {@link Language#getMode()}. */ Editor.prototype.getModeForSelection = function () { // Check for mixed mode info @@ -1241,7 +1241,7 @@ define(function (require, exports, module) { /** * Gets the syntax-highlighting mode for the document. * - * @return {Object|String} Object or Name of syntax-highlighting mode; see {@link Languages#getLanguageForFileExtension()} and {@link Language#mode}. + * @return {Object|String} Object or Name of syntax-highlighting mode; see {@link LanguageManager#getLanguageForFileExtension()} and {@link Language#getMode()}. */ Editor.prototype.getModeForDocument = function () { return this._codeMirror.getOption("mode"); diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 1c807f49b43..392898720bf 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -52,16 +52,19 @@ * language.setLineComment("--"); * language.setBlockComment("{-", "-}"); * + * Currently, languages are also accessible this way right after calling defineLanguage, + * even before the promise has resolved. Note, however, that their mode hasn't yet been set then. + * * Some CodeMirror modes define variations of themselves. They are called MIME modes. - * To find out existing MIME modes, search for "CodeMirror.defineMIME" in thirdparty/CodeMirror2/mode - * For instance, C++, C# and Java all use the clike mode. + * To find existing MIME modes, search for "CodeMirror.defineMIME" in thirdparty/CodeMirror2/mode + * For instance, C++, C# and Java all use the clike (C-like) mode. * You can refine the mode definition by specifying the MIME mode as well: * var language = LanguageManager.defineLanguage("csharp", { * name: "C#", * mode: ["clike", "text/x-csharp"], * ... * }); - * Definining the base mode is still necessary to know which file to load. + * Defining the base mode is still necessary to know which file to load. * Later however, language.getMode() will either refer to the MIME mode, * or the base mode if no MIME mode has been specified. * @@ -124,7 +127,7 @@ define(function (require, exports, module) { } /** - * Makes the file extension all lowercase and ensures it doesn't start with a dot + * Lowercases the file extension and ensures it doesn't start with a dot. * @param {!string} extension The file extension * @return {string} The normalized file extension */ @@ -179,24 +182,23 @@ define(function (require, exports, module) { } /** - * Resolves a file extension to a Language object + * Resolves a file extension to a Language object. * @param {!string} path Path to or extension of the file to find a language for * @return {Language} The language for the provided file type or the fallback language */ function getLanguageForFileExtension(path) { - var extension = PathUtils.filenameExtension(path); - extension = _normalizeFileExtension(extension); + var extension = _normalizeFileExtension(PathUtils.filenameExtension(path)), + language = _fileExtensionToLanguageMap[extension]; - var language = _fileExtensionToLanguageMap[extension]; if (!language) { - console.log("Called LanguageManager.getLanguageForFileExtension with an unhandled file extension: " + extension); + console.log("Called LanguageManager.getLanguageForFileExtension with an unhandled file extension:", extension); } return language || _fallbackLanguage; } /** - * Resolves a CodeMirror mode to a Language object + * Resolves a CodeMirror mode to a Language object. * @param {!string} mode CodeMirror mode * @return {Language} The language for the provided mode or the fallback language */ @@ -221,7 +223,7 @@ define(function (require, exports, module) { function Language(id, name) { _validateString(id, "Language ID"); // Make sure the ID is a string that can safely be used universally by the computer - as a file name, as an object key, as part of a URL, etc. - // Hence we use _ instead of "." since this makes it easier to parse a file name containing a language ID + // Hence we use "_" instead of "." since the latter often has special meaning if (!id.match(/^[a-z]+(_[a-z]+)*$/)) { throw new Error("Invalid language ID \"" + id + "\": Only groups of letters a-z are allowed, separated by _ (i.e. \"cpp\" or \"foo_bar\")"); } @@ -240,6 +242,7 @@ define(function (require, exports, module) { _languages[id] = this; } + /** @type {string} Identifier for this language */ Language.prototype._id = null; @@ -251,22 +254,24 @@ define(function (require, exports, module) { return this._id; }; + /** @type {string} Human-readable name of this language */ Language.prototype._name = null; /** - * Returns the human-readable name of this language + * Returns the human-readable name of this language. * @return {string} The name */ Language.prototype.getName = function () { return this._name; }; + /** @type {string} CodeMirror mode for this language */ Language.prototype._mode = null; /** - * Returns the CodeMirror mode for this language + * Returns the CodeMirror mode for this language. * @return {string} The mode */ Language.prototype.getMode = function () { @@ -301,16 +306,19 @@ define(function (require, exports, module) { var i; if (mode !== "" && !CodeMirror.modes[mode]) { - throw new Error("CodeMirror mode \"" + mode + "\" is not loaded"); + result.reject("CodeMirror mode \"" + mode + "\" is not loaded"); + return; } if (mimeMode) { var modeConfig = CodeMirror.mimeModes[mimeMode]; if (!modeConfig) { - throw new Error("CodeMirror MIME mode \"" + mimeMode + "\" not found"); + result.reject("CodeMirror MIME mode \"" + mimeMode + "\" not found"); + return; } if (modeConfig.name !== mode) { - throw new Error("CodeMirror MIME mode \"" + mimeMode + "\" does not belong to mode \"" + mode + "\""); + result.reject("CodeMirror MIME mode \"" + mimeMode + "\" does not belong to mode \"" + mode + "\""); + return; } } @@ -331,6 +339,7 @@ define(function (require, exports, module) { return result.promise(); }; + /** @type {Array.} File extensions that use this language */ Language.prototype._fileExtensions = null; @@ -339,14 +348,14 @@ define(function (require, exports, module) { * @return {Array.} File extensions used by this language */ Language.prototype.getFileExtensions = function () { + // Use concat to create a copy of this array, preventing external modification return this._fileExtensions.concat(); }; /** * Adds a file extension to this language. * Private for now since dependent code would need to by kept in sync with such changes. - * In case we ever open this up, we should think about whether we want to make this - * configurable by the user. If so, the user has to be able to override these calls. + * See https://github.com/adobe/brackets/issues/2966 for plans to make this public. * @param {!string} extension A file extension used by this language * @private */ @@ -364,12 +373,13 @@ define(function (require, exports, module) { } } }; + /** @type {{ prefix: string }} Line comment syntax */ Language.prototype._lineCommentSyntax = null; /** - * Returns whether the line comment syntax is defined for this language + * Returns whether the line comment syntax is defined for this language. * @return {boolean} Whether line comments are supported */ Language.prototype.hasLineCommentSyntax = function () { @@ -377,7 +387,7 @@ define(function (require, exports, module) { }; /** - * Returns the prefix to use for line comments + * Returns the prefix to use for line comments. * @return {string} The prefix */ Language.prototype.getLineCommentPrefix = function () { @@ -402,7 +412,7 @@ define(function (require, exports, module) { Language.prototype._blockCommentSyntax = null; /** - * Returns whether the block comment syntax is defined for this language + * Returns whether the block comment syntax is defined for this language. * @return {boolean} Whether block comments are supported */ Language.prototype.hasBlockCommentSyntax = function () { @@ -410,7 +420,7 @@ define(function (require, exports, module) { }; /** - * Returns the prefix to use for block comments + * Returns the prefix to use for block comments. * @return {string} The prefix */ Language.prototype.getBlockCommentPrefix = function () { @@ -421,7 +431,7 @@ define(function (require, exports, module) { }; /** - * Returns the suffix to use for block comments + * Returns the suffix to use for block comments. * @return {string} The suffix */ Language.prototype.getBlockCommentSuffix = function () { From fc829e9dbf7b9b50d0adc7c3fb000a19039c69f8 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Wed, 27 Feb 2013 16:28:16 -0800 Subject: [PATCH 07/14] code review comments pull #2980 --- src/language/LanguageManager.js | 152 ++++++++++++++--------------- test/spec/Editor-test.js | 4 +- test/spec/LanguageManager-test.js | 155 +++++++++++++++++++----------- 3 files changed, 172 insertions(+), 139 deletions(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 392898720bf..ace2490b0d4 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -89,7 +89,7 @@ define(function (require, exports, module) { // Dependencies - var CollectionUtils = require("utils/CollectionUtils"), + var Async = require("utils/Async"), _defaultLanguagesJSON = require("text!language/languages.json"); @@ -97,8 +97,8 @@ define(function (require, exports, module) { var _fallbackLanguage = null, _languages = {}, _fileExtensionToLanguageMap = {}, - _modeToLanguageMap = {}; - + _modeToLanguageMap = {}, + _ready; // Helper functions @@ -138,9 +138,7 @@ define(function (require, exports, module) { } // Make checks below case-INsensitive - extension = extension.toLowerCase(); - - return extension; + return extension.toLowerCase(); } /** @@ -224,7 +222,7 @@ define(function (require, exports, module) { _validateString(id, "Language ID"); // Make sure the ID is a string that can safely be used universally by the computer - as a file name, as an object key, as part of a URL, etc. // Hence we use "_" instead of "." since the latter often has special meaning - if (!id.match(/^[a-z]+(_[a-z]+)*$/)) { + if (!id.match(/^[a-zA-Z]+(_[a-zA-Z]+)*$/)) { throw new Error("Invalid language ID \"" + id + "\": Only groups of letters a-z are allowed, separated by _ (i.e. \"cpp\" or \"foo_bar\")"); } if (_languages[id]) { @@ -238,14 +236,30 @@ define(function (require, exports, module) { this._fileExtensions = []; this._modeToLanguageMap = {}; - - _languages[id] = this; } /** @type {string} Identifier for this language */ Language.prototype._id = null; + /** @type {string} Human-readable name of this language */ + Language.prototype._name = null; + + /** @type {string} CodeMirror mode for this language */ + Language.prototype._mode = null; + + /** @type {Array.} File extensions that use this language */ + Language.prototype._fileExtensions = null; + + /** @type {{ prefix: string }} Line comment syntax */ + Language.prototype._lineCommentSyntax = null; + + /** @type {Object.} Which language to use for what CodeMirror mode */ + Language.prototype._modeToLanguageMap = null; + + /** @type {{ prefix: string, suffix: string }} Block comment syntax */ + Language.prototype._blockCommentSyntax = null; + /** * Returns the identifier for this language. * @return {string} The identifier @@ -254,10 +268,6 @@ define(function (require, exports, module) { return this._id; }; - - /** @type {string} Human-readable name of this language */ - Language.prototype._name = null; - /** * Returns the human-readable name of this language. * @return {string} The name @@ -266,10 +276,6 @@ define(function (require, exports, module) { return this._name; }; - - /** @type {string} CodeMirror mode for this language */ - Language.prototype._mode = null; - /** * Returns the CodeMirror mode for this language. * @return {string} The mode @@ -281,17 +287,17 @@ define(function (require, exports, module) { /** * Loads a mode and sets it for this language. * - * @param {string|Array.} mode CodeMirror mode (i.e. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] - * Unless the mode is located in thirdparty/CodeMirror2/mode//.js, you need to first load it yourself. + * @param {string|Array.} mode CodeMirror mode (i.e. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] + * Unless the mode is located in thirdparty/CodeMirror2/mode//.js, you need to first load it yourself. + * @param {Array.} fileExtensions List of file extensions used by this language (i.e. ["php", "php3"]) * * @return {$.Promise} A promise object that will be resolved when the mode is loaded and set */ - Language.prototype._loadAndSetMode = function (mode) { - var result = new $.Deferred(); + Language.prototype._loadAndSetMode = function (mode, fileExtensions) { + var result = new $.Deferred(), + self = this, + mimeMode; // Mode can be an array specifying a mode plus a MIME mode defined by that mode ["clike", "text/x-c++src"] - var language = this; - // Mode can be an array specifying a mode plus a MIME mode defined by that mode ["clike", "text/x-c++src"] - var mimeMode; if (Array.isArray(mode)) { if (mode.length !== 2) { throw new Error("Mode must either be a string or an array containing two strings"); @@ -300,12 +306,16 @@ define(function (require, exports, module) { mode = mode[0]; } - _validateString(mode, "mode"); + // special handling for mode, require explicit declaration of "text/plain" if no mode is specified + if (!mode || mode === "") { + console.error("Mode must be specified as a built-in CodeMirror mode or 'text/plain'"); + mode = "text/plain"; + } var finish = function () { var i; - if (mode !== "" && !CodeMirror.modes[mode]) { + if (mode !== "text/plain" && !CodeMirror.modes[mode]) { result.reject("CodeMirror mode \"" + mode + "\" is not loaded"); return; } @@ -324,13 +334,23 @@ define(function (require, exports, module) { // This mode is now only about what to tell CodeMirror // The base mode was only necessary to load the proper mode file - language._mode = mimeMode || mode; - _setLanguageForMode(language._mode, language); + self._mode = mimeMode || mode; + _setLanguageForMode(self._mode, self); + + // register language file extensions after mode has loaded + if (fileExtensions) { + for (i = 0; i < fileExtensions.length; i++) { + self._addFileExtension(fileExtensions[i]); + } + } + + // finally, store lanuage to _language map + _languages[self.getId()] = self; - result.resolve(); + result.resolve(self); }; - if (mode === "" || CodeMirror.modes[mode]) { + if (mode === "text/plain" || CodeMirror.modes[mode]) { finish(); } else { require(["thirdparty/CodeMirror2/mode/" + mode + "/" + mode], finish); @@ -339,10 +359,6 @@ define(function (require, exports, module) { return result.promise(); }; - - /** @type {Array.} File extensions that use this language */ - Language.prototype._fileExtensions = null; - /** * Returns an array of file extensions for this language. * @return {Array.} File extensions used by this language @@ -373,10 +389,6 @@ define(function (require, exports, module) { } } }; - - - /** @type {{ prefix: string }} Line comment syntax */ - Language.prototype._lineCommentSyntax = null; /** * Returns whether the line comment syntax is defined for this language. @@ -407,10 +419,6 @@ define(function (require, exports, module) { this._lineCommentSyntax = { prefix: prefix }; }; - - /** @type {{ prefix: string, suffix: string }} Block comment syntax */ - Language.prototype._blockCommentSyntax = null; - /** * Returns whether the block comment syntax is defined for this language. * @return {boolean} Whether block comments are supported @@ -452,10 +460,6 @@ define(function (require, exports, module) { this._blockCommentSyntax = { prefix: prefix, suffix: suffix }; }; - - - /** @type {Object.} Which language to use for what CodeMirror mode */ - Language.prototype._modeToLanguageMap = null; /** * Returns either a language associated with the mode or the fallback language. @@ -501,17 +505,9 @@ define(function (require, exports, module) { * @return {$.Promise} A promise object that will be resolved with a Language object **/ function defineLanguage(id, definition) { - var result = new $.Deferred(); - - var language = new Language(id, definition.name); - - var fileExtensions = definition.fileExtensions, + var result = new $.Deferred(), + language = new Language(id, definition.name), i; - if (fileExtensions) { - for (i = 0; i < fileExtensions.length; i++) { - language._addFileExtension(fileExtensions[i]); - } - } var blockComment = definition.blockComment; if (blockComment) { @@ -523,15 +519,7 @@ define(function (require, exports, module) { language.setLineCommentSyntax(lineComment); } - language._loadAndSetMode(definition.mode) - .done(function () { - result.resolve(language); - }) - .fail(function (error) { - result.reject(error); - }); - - return result.promise(); + return language._loadAndSetMode(definition.mode, definition.fileExtensions); } @@ -548,25 +536,29 @@ define(function (require, exports, module) { }); // Load the default languages - CollectionUtils.forEach(JSON.parse(_defaultLanguagesJSON), function (definition, id) { - defineLanguage(id, definition); - }); + _defaultLanguagesJSON = JSON.parse(_defaultLanguagesJSON); + _ready = Async.doInParallel(Object.keys(_defaultLanguagesJSON), function (key) { + return defineLanguage(key, _defaultLanguagesJSON[key]); + }, false); // Get the object for HTML - var html = getLanguage("html"); - - // The htmlmixed mode uses the xml mode internally for the HTML parts, so we map it to HTML - html._setLanguageForMode("xml", html); - - // Currently we override the above mentioned "xml" in TokenUtils.getModeAt, instead returning "html". - // When the CSSInlineEditor and the hint providers are no longer based on modes, this can be changed. - // But for now, we need to associate this madeup "html" mode with our HTML language object. - _setLanguageForMode("html", html); - - // The fallback language for unknown modes and file extensions - _fallbackLanguage = getLanguage("unknown"); + _ready.always(function () { + var html = getLanguage("html"); + + // The htmlmixed mode uses the xml mode internally for the HTML parts, so we map it to HTML + html._setLanguageForMode("xml", html); + + // Currently we override the above mentioned "xml" in TokenUtils.getModeAt, instead returning "html". + // When the CSSInlineEditor and the hint providers are no longer based on modes, this can be changed. + // But for now, we need to associate this madeup "html" mode with our HTML language object. + _setLanguageForMode("html", html); + + // The fallback language for unknown modes and file extensions + _fallbackLanguage = getLanguage("unknown"); + }); // Public methods + exports.ready = _ready; exports.defineLanguage = defineLanguage; exports.getLanguage = getLanguage; exports.getLanguageForFileExtension = getLanguageForFileExtension; diff --git a/test/spec/Editor-test.js b/test/spec/Editor-test.js index 2ed4c2cc3f7..2f340bd3064 100644 --- a/test/spec/Editor-test.js +++ b/test/spec/Editor-test.js @@ -52,7 +52,7 @@ define(function (require, exports, module) { function expectModeAndLang(editor, lang) { expect(editor.getModeForSelection()).toSpecifyModeNamed(lang.mode); - expect(editor.getLanguageForSelection().name).toBe(lang.langName); + expect(editor.getLanguageForSelection().getName()).toBe(lang.langName); } describe("Editor", function () { @@ -166,7 +166,7 @@ define(function (require, exports, module) { ""; it("should get mode in homogenous file", function () { - createTestEditor(jsContent, "javascript"); + createTestEditor(jsContent, langNames.javascript.mode); // Mode at point myEditor.setCursorPos(0, 0); // first char in text diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index 81251379e1c..03cc93eddf2 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -23,7 +23,7 @@ /*jslint vars: true, plusplus: true, devel: true, browser: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define, $, describe, CodeMirror, jasmine, beforeEach, afterEach, it, runs, waitsFor, expect, waitsForDone, waitsForFail */ +/*global define, $, describe, CodeMirror, jasmine, beforeEach, afterEach, it, runs, waitsFor, expect, waitsForDone, waitsForFail, spyOn */ define(function (require, exports, module) { 'use strict'; @@ -36,6 +36,10 @@ define(function (require, exports, module) { describe("LanguageManager", function () { + beforeEach(function () { + waitsForDone(LanguageManager.ready, "LanguageManager ready", 10000); + }); + function defineLanguage(definition) { var def = $.extend({}, definition); @@ -47,18 +51,7 @@ define(function (require, exports, module) { def.lineComment = def.lineComment.prefix; } - var promise = LanguageManager.defineLanguage(definition.id, def), - language; - - if (definition.id) { - try { - language = LanguageManager.getLanguage(definition.id); - } catch (e) { - // Ignore errors here, this should be tested seperately - } - } - - return { promise: promise, language: language }; + return LanguageManager.defineLanguage(definition.id, def); } function validateLanguage(expected, actual) { @@ -70,7 +63,6 @@ define(function (require, exports, module) { expect(actual.getId()).toBe(expected.id); expect(actual.getName()).toBe(expected.name); - expect(actual.getFileExtensions()).toEqual(expected.fileExtensions || []); if (expected.blockComment) { @@ -140,18 +132,21 @@ define(function (require, exports, module) { var language, promise, def = { id: "one", name: "One", mode: "" }; + + spyOn(console, "error"); runs(function () { - defineLanguage(def).promise.done(function (lang) { + defineLanguage(def).done(function (lang) { language = lang; }); }); waitsFor(function () { return Boolean(language); - }, "The language should be resolved", 50); + }, "The language should be resolved", 0); runs(function () { + expect(console.error).toHaveBeenCalledWith("Mode must be specified as a built-in CodeMirror mode or 'text/plain'"); validateLanguage(def, language); }); }); @@ -167,30 +162,58 @@ define(function (require, exports, module) { expect(function () { defineLanguage({ id: "three", name: "" }); }).toThrow(new Error("name must not be empty")); }); - it("should throw errors for missing mode value", function () { - expect(function () { defineLanguage({ id: "four", name: "Four" }); }).toThrow(new Error("mode must be a string")); + it("should log errors for missing mode value", function () { + spyOn(console, "error"); + defineLanguage({ id: "four", name: "Four" }); + expect(console.error).toHaveBeenCalledWith("Mode must be specified as a built-in CodeMirror mode or 'text/plain'"); }); it("should create a language with file extensions and a mode", function () { - var def = { id: "pascal", name: "Pascal", fileExtensions: ["pas", "p"], mode: "pascal" }, - lang = defineLanguage(def).language; + var def = { id: "pascal", name: "Pascal", fileExtensions: ["pas", "p"], mode: "pascal" }, + language; - expect(LanguageManager.getLanguageForFileExtension("file.p")).toBe(lang); + runs(function () { + defineLanguage(def).done(function (lang) { + language = lang; + }); + }); + + waitsFor(function () { + return Boolean(language); + }, "The language should be resolved", 50); - validateLanguage(def, lang); + runs(function () { + expect(LanguageManager.getLanguageForFileExtension("file.p")).toBe(language); + validateLanguage(def, language); + }); }); it("should allow multiple languages to use the same mode", function () { - var xmlBefore = LanguageManager.getLanguage("xml"), + var xmlBefore, def = { id: "wix", name: "WiX", fileExtensions: ["wix"], mode: "xml" }, - lang = defineLanguage(def).language, - xmlAfter = LanguageManager.getLanguage("xml"); + lang, + xmlAfter; - expect(xmlBefore).toBe(xmlAfter); - expect(LanguageManager.getLanguageForFileExtension("file.wix")).toBe(lang); - expect(LanguageManager.getLanguageForFileExtension("file.xml")).toBe(xmlAfter); + runs(function () { + xmlBefore = LanguageManager.getLanguage("xml"); + + defineLanguage(def).done(function (language) { + lang = language; + xmlAfter = LanguageManager.getLanguage("xml"); + }); + }); - validateLanguage(def, lang); + waitsFor(function () { + return Boolean(lang); + }, "The language should be resolved", 50); + + runs(function () { + expect(xmlBefore).toBe(xmlAfter); + expect(LanguageManager.getLanguageForFileExtension("file.wix")).toBe(lang); + expect(LanguageManager.getLanguageForFileExtension("file.xml")).toBe(xmlAfter); + + validateLanguage(def, lang); + }); }); // FIXME: Add internal LanguageManager._reset() @@ -198,48 +221,66 @@ define(function (require, exports, module) { it("should return an error if a language is already defined", function () { var def = { id: "pascal", name: "Pascal", fileExtensions: ["pas", "p"], mode: "pascal" }; - expect(function () { defineLanguage(def); }).toThrow(new Error('Language "pascal" is already defined')); + runs(function () { + expect(function () { defineLanguage(def); }).toThrow(new Error('Language "pascal" is already defined')); + }); }); it("should validate comment prefix/suffix", function () { - var def = { id: "coldfusion", name: "ColdFusion", fileExtensions: ["cfml", "cfm"], mode: "xml" }, - lang = defineLanguage(def).language; - - expect(function () { lang.setLineCommentSyntax(""); }).toThrow(new Error("prefix must not be empty")); - expect(function () { lang.setBlockCommentSyntax(""); }).toThrow(new Error("prefix must not be empty")); + var def = { id: "coldfusion", name: "ColdFusion", fileExtensions: ["cfml", "cfm"], mode: "xml" }, + language; - def.lineComment = { - prefix: "//" - }; - def.blockComment = { - prefix: "" - }; + runs(function () { + defineLanguage(def).done(function (lang) { + language = lang; + }); + }); - lang.setLineCommentSyntax(def.lineComment.prefix); - lang.setBlockCommentSyntax(def.blockComment.prefix, def.blockComment.suffix); + waitsFor(function () { + return Boolean(language); + }, "The language should be resolved", 50); - validateLanguage(def, lang); + runs(function () { + expect(function () { language.setLineCommentSyntax(""); }).toThrow(new Error("prefix must not be empty")); + expect(function () { language.setBlockCommentSyntax(""); }).toThrow(new Error("prefix must not be empty")); + + def.lineComment = { + prefix: "//" + }; + def.blockComment = { + prefix: "" + }; + + language.setLineCommentSyntax(def.lineComment.prefix); + language.setBlockCommentSyntax(def.blockComment.prefix, def.blockComment.suffix); + + validateLanguage(def, language); + }); }); it("should load a built-in CodeMirror mode", function () { - var id = "erlang"; + var id = "erlang", + def = { id: id, name: "erlang", fileExtensions: ["erlang"], mode: "erlang" }, + language; runs(function () { // erlang is not defined in the default set of languages in languages.json expect(CodeMirror.modes[id]).toBe(undefined); - var def = { id: id, name: "erlang", fileExtensions: ["erlang"], mode: "erlang" }, - defRes = defineLanguage(def), - lang = defRes.language, - promise = defRes.promise; - - expect(LanguageManager.getLanguageForFileExtension("file.erlang")).toBe(lang); - - validateLanguage(def, lang); - - waitsForDone(promise, "should resolve language Erlang", 10000); + defineLanguage(def).done(function (lang) { + language = lang; + }); + }); + + waitsFor(function () { + return Boolean(language); + }, "The language should be resolved", 50); + + runs(function () { + expect(LanguageManager.getLanguageForFileExtension("file.erlang")).toBe(language); + validateLanguage(def, language); }); runs(function () { From 513e4e076b81935948af3114c95e12647026adea Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Thu, 28 Feb 2013 12:02:26 -0800 Subject: [PATCH 08/14] fixed mode handling for text/plain and validation of mode --- src/language/LanguageManager.js | 74 +++++++++++++++++++-------------- src/language/languages.json | 2 +- 2 files changed, 44 insertions(+), 32 deletions(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index ace2490b0d4..ae20bd6af94 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -289,11 +289,10 @@ define(function (require, exports, module) { * * @param {string|Array.} mode CodeMirror mode (i.e. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"] * Unless the mode is located in thirdparty/CodeMirror2/mode//.js, you need to first load it yourself. - * @param {Array.} fileExtensions List of file extensions used by this language (i.e. ["php", "php3"]) * * @return {$.Promise} A promise object that will be resolved when the mode is loaded and set */ - Language.prototype._loadAndSetMode = function (mode, fileExtensions) { + Language.prototype._loadAndSetMode = function (mode) { var result = new $.Deferred(), self = this, mimeMode; // Mode can be an array specifying a mode plus a MIME mode defined by that mode ["clike", "text/x-c++src"] @@ -307,52 +306,44 @@ define(function (require, exports, module) { } // special handling for mode, require explicit declaration of "text/plain" if no mode is specified - if (!mode || mode === "") { - console.error("Mode must be specified as a built-in CodeMirror mode or 'text/plain'"); - mode = "text/plain"; + var isPlainText = (mode === "") && (mimeMode === "text/plain"); + + if (!isPlainText && (!mode || mode === "")) { + result.reject("Mode must be specified as a built-in CodeMirror mode or [\'\', 'text/plain']"); } var finish = function () { var i; - if (mode !== "text/plain" && !CodeMirror.modes[mode]) { - result.reject("CodeMirror mode \"" + mode + "\" is not loaded"); - return; - } - - if (mimeMode) { - var modeConfig = CodeMirror.mimeModes[mimeMode]; - if (!modeConfig) { - result.reject("CodeMirror MIME mode \"" + mimeMode + "\" not found"); + if (!isPlainText) { + if (!CodeMirror.modes[mode]) { + result.reject("CodeMirror mode \"" + mode + "\" is not loaded"); return; } - if (modeConfig.name !== mode) { - result.reject("CodeMirror MIME mode \"" + mimeMode + "\" does not belong to mode \"" + mode + "\""); - return; + + if (mimeMode) { + var modeConfig = CodeMirror.mimeModes[mimeMode]; + if (!modeConfig) { + result.reject("CodeMirror MIME mode \"" + mimeMode + "\" not found"); + return; + } + if (modeConfig.name !== mode) { + result.reject("CodeMirror MIME mode \"" + mimeMode + "\" does not belong to mode \"" + mode + "\""); + return; + } } } // This mode is now only about what to tell CodeMirror // The base mode was only necessary to load the proper mode file self._mode = mimeMode || mode; - _setLanguageForMode(self._mode, self); - - // register language file extensions after mode has loaded - if (fileExtensions) { - for (i = 0; i < fileExtensions.length; i++) { - self._addFileExtension(fileExtensions[i]); - } - } - - // finally, store lanuage to _language map - _languages[self.getId()] = self; result.resolve(self); }; - if (mode === "text/plain" || CodeMirror.modes[mode]) { + if (isPlainText || CodeMirror.modes[mode]) { finish(); - } else { + } else if (mode) { require(["thirdparty/CodeMirror2/mode/" + mode + "/" + mode], finish); } @@ -507,6 +498,7 @@ define(function (require, exports, module) { function defineLanguage(id, definition) { var result = new $.Deferred(), language = new Language(id, definition.name), + fileExtensions = definition.fileExtensions, i; var blockComment = definition.blockComment; @@ -519,7 +511,27 @@ define(function (require, exports, module) { language.setLineCommentSyntax(lineComment); } - return language._loadAndSetMode(definition.mode, definition.fileExtensions); + language._loadAndSetMode(definition.mode).done(function () { + // register language file extensions after mode has loaded + if (fileExtensions) { + for (i = 0; i < fileExtensions.length; i++) { + language._addFileExtension(fileExtensions[i]); + } + } + + // globally associate mode to language + _setLanguageForMode(language.getMode(), language); + + // finally, store lanuage to _language map + _languages[id] = language; + + result.resolve(language); + }).fail(function (error) { + console.error(error); + result.reject(error); + }); + + return result.promise(); } diff --git a/src/language/languages.json b/src/language/languages.json index 8b20fe3d8d1..bb14120de3f 100644 --- a/src/language/languages.json +++ b/src/language/languages.json @@ -1,7 +1,7 @@ { "unknown": { "name": "Text", - "mode": "" + "mode": ["", "text/plain"] }, "css": { From ce7fd4b9606b206a649b2f728d8eaa6d71f5b883 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Thu, 28 Feb 2013 12:47:20 -0800 Subject: [PATCH 09/14] fix language unit tests --- test/spec/Editor-test.js | 6 ++++-- test/spec/LanguageManager-test.js | 17 ++++++++++------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/test/spec/Editor-test.js b/test/spec/Editor-test.js index 2f340bd3064..83b8e4ca77d 100644 --- a/test/spec/Editor-test.js +++ b/test/spec/Editor-test.js @@ -133,10 +133,12 @@ define(function (require, exports, module) { expect(mode).toSpecifyModeNamed(langNames.javascript.mode); }); - it("should default to plaintext for unknown file extensions", function () { + it("should default to plain text for unknown file extensions", function () { // verify editor content var mode = LanguageManager.getLanguageForFileExtension("test.foo").getMode(); - expect(mode).toBe(""); + + // "unknown" mode uses it's MIME type instead + expect(mode).toBe("text/plain"); }); }); diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index 03cc93eddf2..cf7ece4acd5 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -131,9 +131,7 @@ define(function (require, exports, module) { it("should create a basic language", function () { var language, promise, - def = { id: "one", name: "One", mode: "" }; - - spyOn(console, "error"); + def = { id: "one", name: "One", mode: ["", "text/plain"] }; runs(function () { defineLanguage(def).done(function (lang) { @@ -146,7 +144,6 @@ define(function (require, exports, module) { }, "The language should be resolved", 0); runs(function () { - expect(console.error).toHaveBeenCalledWith("Mode must be specified as a built-in CodeMirror mode or 'text/plain'"); validateLanguage(def, language); }); }); @@ -163,9 +160,15 @@ define(function (require, exports, module) { }); it("should log errors for missing mode value", function () { - spyOn(console, "error"); - defineLanguage({ id: "four", name: "Four" }); - expect(console.error).toHaveBeenCalledWith("Mode must be specified as a built-in CodeMirror mode or 'text/plain'"); + runs(function () { + spyOn(console, "error"); + var promise = defineLanguage({ id: "four", name: "Four" }); + waitsForFail(promise, "Promise should fail with missing mode"); + }); + + runs(function () { + expect(console.error).toHaveBeenCalledWith("Mode must be specified as a built-in CodeMirror mode or ['', 'text/plain']"); + }); }); it("should create a language with file extensions and a mode", function () { From 53e4686a83544e563f7e8b7fa5f5f55faab87960 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Thu, 28 Feb 2013 20:18:50 -0800 Subject: [PATCH 10/14] code review comments part 1 --- src/language/LanguageManager.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index ae20bd6af94..46c38a60b25 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -57,7 +57,7 @@ * * Some CodeMirror modes define variations of themselves. They are called MIME modes. * To find existing MIME modes, search for "CodeMirror.defineMIME" in thirdparty/CodeMirror2/mode - * For instance, C++, C# and Java all use the clike (C-like) mode. + * For instance, C++, C# and Java all use the clike (C-like) mode with different settings and a different MIME name. * You can refine the mode definition by specifying the MIME mode as well: * var language = LanguageManager.defineLanguage("csharp", { * name: "C#", @@ -65,8 +65,8 @@ * ... * }); * Defining the base mode is still necessary to know which file to load. - * Later however, language.getMode() will either refer to the MIME mode, - * or the base mode if no MIME mode has been specified. + * However, language.getMode() will return just the MIME mode if one was + * specified. * * If you need to configure a mode, you can just create a new MIME mode and use that: * CodeMirror.defineMIME("text/x-brackets-html", { From 90cb5c55028754ae1649ae251eae1794b417bfcc Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Fri, 1 Mar 2013 10:35:45 -0800 Subject: [PATCH 11/14] code review comments part 2. Track _pendingLanguages while loading. Use special null string for plain text mode. Update unit tests. --- src/language/LanguageManager.js | 84 +++++++++++++++---------------- src/language/languages.json | 2 +- test/spec/LanguageManager-test.js | 30 ++++------- 3 files changed, 50 insertions(+), 66 deletions(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 46c38a60b25..6c0516dd135 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -42,7 +42,7 @@ * lineComment: "--" * }); * - * To use that language, wait for the returned promise to be resolved: + * To use that language and it's related mode, wait for the returned promise to be resolved: * LanguageManager.defineLanguage("haskell", definition).done(function (language) { * console.log("Language " + language.name + " is now available!"); * }); @@ -52,9 +52,6 @@ * language.setLineComment("--"); * language.setBlockComment("{-", "-}"); * - * Currently, languages are also accessible this way right after calling defineLanguage, - * even before the promise has resolved. Note, however, that their mode hasn't yet been set then. - * * Some CodeMirror modes define variations of themselves. They are called MIME modes. * To find existing MIME modes, search for "CodeMirror.defineMIME" in thirdparty/CodeMirror2/mode * For instance, C++, C# and Java all use the clike (C-like) mode with different settings and a different MIME name. @@ -95,6 +92,7 @@ define(function (require, exports, module) { // State var _fallbackLanguage = null, + _pendingLanguages = {}, _languages = {}, _fileExtensionToLanguageMap = {}, _modeToLanguageMap = {}, @@ -222,7 +220,7 @@ define(function (require, exports, module) { _validateString(id, "Language ID"); // Make sure the ID is a string that can safely be used universally by the computer - as a file name, as an object key, as part of a URL, etc. // Hence we use "_" instead of "." since the latter often has special meaning - if (!id.match(/^[a-zA-Z]+(_[a-zA-Z]+)*$/)) { + if (!id.match(/^[a-z]+(_[a-z]+)*$/)) { throw new Error("Invalid language ID \"" + id + "\": Only groups of letters a-z are allowed, separated by _ (i.e. \"cpp\" or \"foo_bar\")"); } if (_languages[id]) { @@ -304,33 +302,28 @@ define(function (require, exports, module) { mimeMode = mode[1]; mode = mode[0]; } - - // special handling for mode, require explicit declaration of "text/plain" if no mode is specified - var isPlainText = (mode === "") && (mimeMode === "text/plain"); - - if (!isPlainText && (!mode || mode === "")) { - result.reject("Mode must be specified as a built-in CodeMirror mode or [\'\', 'text/plain']"); - } + + // mode must not be empty. Use "null" (the string "null") mode for plain text + _validateNonEmptyString(mode, "mode"); var finish = function () { - var i; + if (!CodeMirror.modes[mode]) { + result.reject("CodeMirror mode \"" + mode + "\" is not loaded"); + return; + } - if (!isPlainText) { - if (!CodeMirror.modes[mode]) { - result.reject("CodeMirror mode \"" + mode + "\" is not loaded"); + if (mimeMode) { + var modeConfig = CodeMirror.mimeModes[mimeMode]; + + if (!modeConfig) { + result.reject("CodeMirror MIME mode \"" + mimeMode + "\" not found"); return; } - if (mimeMode) { - var modeConfig = CodeMirror.mimeModes[mimeMode]; - if (!modeConfig) { - result.reject("CodeMirror MIME mode \"" + mimeMode + "\" not found"); - return; - } - if (modeConfig.name !== mode) { - result.reject("CodeMirror MIME mode \"" + mimeMode + "\" does not belong to mode \"" + mode + "\""); - return; - } + // modeConfig can be a string or mode object + if (modeConfig !== mode && modeConfig.name !== mode) { + result.reject("CodeMirror MIME mode \"" + mimeMode + "\" does not belong to mode \"" + mode + "\""); + return; } } @@ -341,9 +334,9 @@ define(function (require, exports, module) { result.resolve(self); }; - if (isPlainText || CodeMirror.modes[mode]) { + if (CodeMirror.modes[mode]) { finish(); - } else if (mode) { + } else { require(["thirdparty/CodeMirror2/mode/" + mode + "/" + mode], finish); } @@ -394,10 +387,7 @@ define(function (require, exports, module) { * @return {string} The prefix */ Language.prototype.getLineCommentPrefix = function () { - if (!this._lineCommentSyntax) { - return null; - } - return this._lineCommentSyntax.prefix; + return this._lineCommentSyntax && this._lineCommentSyntax.prefix; }; /** @@ -423,10 +413,7 @@ define(function (require, exports, module) { * @return {string} The prefix */ Language.prototype.getBlockCommentPrefix = function () { - if (!this._blockCommentSyntax) { - return null; - } - return this._blockCommentSyntax.prefix; + return this._blockCommentSyntax && this._blockCommentSyntax.prefix; }; /** @@ -434,16 +421,13 @@ define(function (require, exports, module) { * @return {string} The suffix */ Language.prototype.getBlockCommentSuffix = function () { - if (!this._blockCommentSyntax) { - return null; - } - return this._blockCommentSyntax.suffix; + return this._blockCommentSyntax && this._blockCommentSyntax.suffix; }; /** * Sets the prefix and suffix to use for blocks comments in this language. - * @param {!string} prefix Prefix string to use for block comments (i.e. "") + * @param {!string} prefix Prefix string to use for block comments (e.g. "") */ Language.prototype.setBlockCommentSyntax = function (prefix, suffix) { _validateNonEmptyString(prefix, "prefix"); @@ -496,8 +480,14 @@ define(function (require, exports, module) { * @return {$.Promise} A promise object that will be resolved with a Language object **/ function defineLanguage(id, definition) { - var result = new $.Deferred(), - language = new Language(id, definition.name), + var result = new $.Deferred(); + + if (_pendingLanguages[id]) { + result.reject("Language \"" + id + "\" is waiting to be resolved."); + return result.promise(); + } + + var language = new Language(id, definition.name), fileExtensions = definition.fileExtensions, i; @@ -511,6 +501,9 @@ define(function (require, exports, module) { language.setLineCommentSyntax(lineComment); } + // track languages that are currently loading + _pendingLanguages[id] = language; + language._loadAndSetMode(definition.mode).done(function () { // register language file extensions after mode has loaded if (fileExtensions) { @@ -529,6 +522,9 @@ define(function (require, exports, module) { }).fail(function (error) { console.error(error); result.reject(error); + }).always(function () { + // delete from pending languages after success and failure + delete _pendingLanguages[id]; }); return result.promise(); diff --git a/src/language/languages.json b/src/language/languages.json index bb14120de3f..ba71509fad7 100644 --- a/src/language/languages.json +++ b/src/language/languages.json @@ -1,7 +1,7 @@ { "unknown": { "name": "Text", - "mode": ["", "text/plain"] + "mode": ["null", "text/plain"] }, "css": { diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index cf7ece4acd5..71dd7baefab 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -131,21 +131,16 @@ define(function (require, exports, module) { it("should create a basic language", function () { var language, promise, - def = { id: "one", name: "One", mode: ["", "text/plain"] }; - - runs(function () { - defineLanguage(def).done(function (lang) { - language = lang; - }); + def = { id: "one", name: "One", mode: ["null", "text/plain"] }; + + // mode already exists, this test is completely synchronous + promise = defineLanguage(def).done(function (lang) { + language = lang; }); - waitsFor(function () { - return Boolean(language); - }, "The language should be resolved", 0); + expect(promise.isResolved()).toBeTruthy(); - runs(function () { - validateLanguage(def, language); - }); + validateLanguage(def, language); }); it("should throw errors for invalid language id values", function () { @@ -160,15 +155,8 @@ define(function (require, exports, module) { }); it("should log errors for missing mode value", function () { - runs(function () { - spyOn(console, "error"); - var promise = defineLanguage({ id: "four", name: "Four" }); - waitsForFail(promise, "Promise should fail with missing mode"); - }); - - runs(function () { - expect(console.error).toHaveBeenCalledWith("Mode must be specified as a built-in CodeMirror mode or ['', 'text/plain']"); - }); + expect(function () { defineLanguage({ id: "four", name: "Four" }); }).toThrow(new Error("mode must be a string")); + expect(function () { defineLanguage({ id: "five", name: "Five", mode: "" }); }).toThrow(new Error("mode must not be empty")); }); it("should create a language with file extensions and a mode", function () { From 3a4ca20afe18a0edf27bc8b1a8400a657e60e481 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Fri, 1 Mar 2013 12:14:57 -0800 Subject: [PATCH 12/14] update language id validation --- src/language/LanguageManager.js | 4 ++-- test/spec/LanguageManager-test.js | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 6c0516dd135..b750afc3362 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -220,8 +220,8 @@ define(function (require, exports, module) { _validateString(id, "Language ID"); // Make sure the ID is a string that can safely be used universally by the computer - as a file name, as an object key, as part of a URL, etc. // Hence we use "_" instead of "." since the latter often has special meaning - if (!id.match(/^[a-z]+(_[a-z]+)*$/)) { - throw new Error("Invalid language ID \"" + id + "\": Only groups of letters a-z are allowed, separated by _ (i.e. \"cpp\" or \"foo_bar\")"); + if (!id.match(/^[a-z0-9]+(_[a-z0-9]+)*$/)) { + throw new Error("Invalid language ID \"" + id + "\": Only groups of lower case letters and numbers are allowed, separated by underscores."); } if (_languages[id]) { throw new Error("Language \"" + id + "\" is already defined"); diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index 71dd7baefab..628a7e904df 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -144,9 +144,10 @@ define(function (require, exports, module) { }); it("should throw errors for invalid language id values", function () { - expect(function () { defineLanguage({ id: null }); }).toThrow(new Error("Language ID must be a string")); - expect(function () { defineLanguage({ id: "123" }); }).toThrow(new Error('Invalid language ID "123": Only groups of letters a-z are allowed, separated by _ (i.e. "cpp" or "foo_bar")')); - expect(function () { defineLanguage({ id: "html" }); }).toThrow(new Error('Language "html" is already defined')); + expect(function () { defineLanguage({ id: null }); }).toThrow(new Error("Language ID must be a string")); + expect(function () { defineLanguage({ id: "HTML5" }); }).toThrow(new Error("Invalid language ID \"HTML5\": Only groups of lower case letters and numbers are allowed, separated by underscores.")); + expect(function () { defineLanguage({ id: "_underscore" }); }).toThrow(new Error("Invalid language ID \"_underscore\": Only groups of lower case letters and numbers are allowed, separated by underscores.")); + expect(function () { defineLanguage({ id: "html" }); }).toThrow(new Error('Language "html" is already defined')); }); it("should throw errors for invalid language name values", function () { From b42ac954282e45350c2d9b957cf266559cfd66a0 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Fri, 1 Mar 2013 12:17:09 -0800 Subject: [PATCH 13/14] update defineLanguage jsdoc --- src/language/LanguageManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index b750afc3362..d149bde51a0 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -468,7 +468,7 @@ define(function (require, exports, module) { /** * Defines a language. * - * @param {!string} id Unique identifier for this language, use only letters a-z and _ inbetween (i.e. "cpp", "foo_bar") + * @param {!string} id Unique identifier for this language, use only letters a-z, numbers and _ inbetween (i.e. "cpp", "foo_bar") * @param {!Object} definition An object describing the language * @param {!string} definition.name Human-readable name of the language, as it's commonly referred to (i.e. "C++") * @param {Array.} definition.fileExtensions List of file extensions used by this language (i.e. ["php", "php3"]) From 6fce3077d4f2e2dfda73bf7426f3ac5205ad1ddf Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Fri, 1 Mar 2013 13:10:59 -0800 Subject: [PATCH 14/14] wait for LanguageManager to load before finish init --- src/brackets.js | 92 ++++++++++++++++++++++++++----------------------- 1 file changed, 48 insertions(+), 44 deletions(-) diff --git a/src/brackets.js b/src/brackets.js index e565ea0d874..31eba186c68 100644 --- a/src/brackets.js +++ b/src/brackets.js @@ -59,6 +59,7 @@ define(function (require, exports, module) { // Load dependent modules var Global = require("utils/Global"), AppInit = require("utils/AppInit"), + LanguageManager = require("language/LanguageManager"), ProjectManager = require("project/ProjectManager"), DocumentManager = require("document/DocumentManager"), EditorManager = require("editor/EditorManager"), @@ -179,55 +180,58 @@ define(function (require, exports, module) { $testDiv.remove(); } - - // Load all extensions. This promise will complete even if one or more - // extensions fail to load. - var extensionLoaderPromise = ExtensionLoader.init(params.get("extensions")); - - // Load the initial project after extensions have loaded - extensionLoaderPromise.always(function () { - // Finish UI initialization - var initialProjectPath = ProjectManager.getInitialProjectPath(); - ProjectManager.openProject(initialProjectPath).always(function () { - _initTest(); - - // If this is the first launch, and we have an index.html file in the project folder (which should be - // the samples folder on first launch), open it automatically. (We explicitly check for the - // samples folder in case this is the first time we're launching Brackets after upgrading from - // an old version that might not have set the "afterFirstLaunch" pref.) - var prefs = PreferencesManager.getPreferenceStorage(PREFERENCES_CLIENT_ID), - deferred = new $.Deferred(); - if (!params.get("skipSampleProjectLoad") && !prefs.getValue("afterFirstLaunch")) { - prefs.setValue("afterFirstLaunch", "true"); - if (ProjectManager.isWelcomeProjectPath(initialProjectPath)) { - var dirEntry = new NativeFileSystem.DirectoryEntry(initialProjectPath); - - dirEntry.getFile("index.html", {}, function (fileEntry) { - var promise = CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, { fullPath: fileEntry.fullPath }); - promise.pipe(deferred.resolve, deferred.reject); - }, deferred.reject); + + // Load default languages + LanguageManager.ready.always(function () { + // Load all extensions. This promise will complete even if one or more + // extensions fail to load. + var extensionLoaderPromise = ExtensionLoader.init(params.get("extensions")); + + // Load the initial project after extensions have loaded + extensionLoaderPromise.always(function () { + // Finish UI initialization + var initialProjectPath = ProjectManager.getInitialProjectPath(); + ProjectManager.openProject(initialProjectPath).always(function () { + _initTest(); + + // If this is the first launch, and we have an index.html file in the project folder (which should be + // the samples folder on first launch), open it automatically. (We explicitly check for the + // samples folder in case this is the first time we're launching Brackets after upgrading from + // an old version that might not have set the "afterFirstLaunch" pref.) + var prefs = PreferencesManager.getPreferenceStorage(PREFERENCES_CLIENT_ID), + deferred = new $.Deferred(); + if (!params.get("skipSampleProjectLoad") && !prefs.getValue("afterFirstLaunch")) { + prefs.setValue("afterFirstLaunch", "true"); + if (ProjectManager.isWelcomeProjectPath(initialProjectPath)) { + var dirEntry = new NativeFileSystem.DirectoryEntry(initialProjectPath); + + dirEntry.getFile("index.html", {}, function (fileEntry) { + var promise = CommandManager.execute(Commands.FILE_ADD_TO_WORKING_SET, { fullPath: fileEntry.fullPath }); + promise.pipe(deferred.resolve, deferred.reject); + }, deferred.reject); + } else { + deferred.resolve(); + } } else { deferred.resolve(); } - } else { - deferred.resolve(); - } - - deferred.always(function () { - // Signal that Brackets is loaded - AppInit._dispatchReady(AppInit.APP_READY); - PerfUtils.addMeasurement("Application Startup"); - }); - - // See if any startup files were passed to the application - if (brackets.app.getPendingFilesToOpen) { - brackets.app.getPendingFilesToOpen(function (err, files) { - files.forEach(function (filename) { - CommandManager.execute(Commands.FILE_OPEN, { fullPath: filename }); - }); + deferred.always(function () { + // Signal that Brackets is loaded + AppInit._dispatchReady(AppInit.APP_READY); + + PerfUtils.addMeasurement("Application Startup"); }); - } + + // See if any startup files were passed to the application + if (brackets.app.getPendingFilesToOpen) { + brackets.app.getPendingFilesToOpen(function (err, files) { + files.forEach(function (filename) { + CommandManager.execute(Commands.FILE_OPEN, { fullPath: filename }); + }); + }); + } + }); }); });