From 36e60acfb45a872a307fada73c6cbeaa8580ed02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Wed, 13 Mar 2013 04:54:47 -0300 Subject: [PATCH 1/5] Adding support for multiple lineComment prefixes --- src/editor/EditorCommandHandlers.js | 81 ++++++++++++++++------ src/extensions/default/LESSSupport/main.js | 2 +- src/language/LanguageManager.js | 28 ++++---- src/language/languages.json | 13 ++-- 4 files changed, 84 insertions(+), 40 deletions(-) diff --git a/src/editor/EditorCommandHandlers.js b/src/editor/EditorCommandHandlers.js index 5f181c9c7c1..3dbfe55dcc6 100644 --- a/src/editor/EditorCommandHandlers.js +++ b/src/editor/EditorCommandHandlers.js @@ -46,23 +46,53 @@ define(function (require, exports, module) { var DIRECTION_DOWN = +1; + /** + * @private + * Creates regular expressions for multiple line comments prefixes + * @param {Array.} prefixes - Description + * @return {Array.} + */ + function _createLineExpressions(prefixes) { + var lineExps = []; + prefixes.forEach(function (prefix) { + lineExps.push(new RegExp("^\\s*" + StringUtils.regexEscape(prefix))); + }); + return lineExps; + } + + /** + * @private + * Returns true if any regular expression matches the given string + * @param {string} string + * @param {Array.} expressions + * @return {boolean} + */ + function _matchExpressions(string, expressions) { + var matchAny = false; + expressions.forEach(function (exp) { + matchAny = matchAny || string.match(exp); + }); + return matchAny; + } + /** * @private * Searchs for an uncommented line between startLine and endLine * @param {!Editor} editor * @param {!number} startLine - valid line inside the document * @param {!number} endLine - valid line inside the document + * @param {!Array.} prefixes - Array of line comment prefixes * @return {boolean} true if there is at least one uncommented line */ - function _containsUncommented(editor, startLine, endLine, prefix) { - var lineExp = new RegExp("^\\s*" + StringUtils.regexEscape(prefix)); + function _containsUncommented(editor, startLine, endLine, prefixes) { + var lineExp = _createLineExpressions(prefixes); var containsUncommented = false; var i; var line; for (i = startLine; i <= endLine; i++) { line = editor.document.getLine(i); // A line is commented out if it starts with 0-N whitespace chars, then "//" - if (!line.match(lineExp) && line.match(/\S/)) { + if (!_matchExpressions(line, lineExp) && line.match(/\S/)) { containsUncommented = true; break; } @@ -77,8 +107,11 @@ define(function (require, exports, module) { * If all non-whitespace lines are already commented out, then we uncomment; otherwise we comment * out. Commenting out adds the prefix at column 0 of every line. Uncommenting removes the first prefix * on each line (if any - empty lines might not have one). + * + * @param {!Editor} editor + * @param {!Array.} prefixes, e.g. ["//"] */ - function lineCommentPrefix(editor, prefix) { + function lineCommentPrefix(editor, prefixes) { var doc = editor.document; var sel = editor.getSelection(); var startLine = sel.start.line; @@ -95,32 +128,38 @@ define(function (require, exports, module) { // Decide if we're commenting vs. un-commenting // Are there any non-blank lines that aren't commented out? (We ignore blank lines because // some editors like Sublime don't comment them out) - var containsUncommented = _containsUncommented(editor, startLine, endLine, prefix); - var i; + var containsUncommented = _containsUncommented(editor, startLine, endLine, prefixes); + var i, j; var line; + var trimmedLine; + var commentI; var updateSelection = false; // Make the edit doc.batchOperation(function () { if (containsUncommented) { - // Comment out - prepend "//" to each line + // Comment out - prepend the first prefix to each line for (i = startLine; i <= endLine; i++) { - doc.replaceRange(prefix, {line: i, ch: 0}); + doc.replaceRange(prefixes[0], {line: i, ch: 0}); } - // Make sure selection includes "//" that was added at start of range + // Make sure selection includes the prefix that was added at start of range if (sel.start.ch === 0 && hasSelection) { updateSelection = true; } - + } else { - // Uncomment - remove first "//" on each line (if any) + // Uncomment - remove first every prefix on each line (if any) for (i = startLine; i <= endLine; i++) { line = doc.getLine(i); - var commentI = line.indexOf(prefix); - if (commentI !== -1) { - doc.replaceRange("", {line: i, ch: commentI}, {line: i, ch: commentI + prefix.length}); + trimmedLine = line.trim(); + for (j = 0; j < prefixes.length; j++) { + commentI = line.indexOf(prefixes[j]); + if (trimmedLine.indexOf(prefixes[j]) === 0) { + doc.replaceRange("", {line: i, ch: commentI}, {line: i, ch: commentI + prefixes[j].length}); + break; + } } } } @@ -206,9 +245,9 @@ define(function (require, exports, module) { * @param {!Editor} editor * @param {!string} prefix, e.g. "" - * @param {?string} linePrefix, e.g. "//" + * @param {?Array.} linePrefixes, e.g. ["//"] */ - function blockCommentPrefixSuffix(editor, prefix, suffix, linePrefix) { + function blockCommentPrefixSuffix(editor, prefix, suffix, linePrefixes) { var doc = editor.document, sel = editor.getSelection(), @@ -217,7 +256,7 @@ define(function (require, exports, module) { endCtx = TokenUtils.getInitialContext(editor._codeMirror, {line: sel.end.line, ch: sel.end.ch}), prefixExp = new RegExp("^" + StringUtils.regexEscape(prefix), "g"), suffixExp = new RegExp(StringUtils.regexEscape(suffix) + "$", "g"), - lineExp = linePrefix ? new RegExp("^" + StringUtils.regexEscape(linePrefix)) : null, + lineExp = linePrefixes ? _createLineExpressions(linePrefixes) : null, prefixPos = null, suffixPos = null, canComment = false, @@ -233,13 +272,13 @@ define(function (require, exports, module) { } // Check if we should just do a line uncomment (if all lines in the selection are commented). - if (lineExp && (ctx.token.string.match(lineExp) || endCtx.token.string.match(lineExp))) { + if (lineExp && (_matchExpressions(ctx.token.string, lineExp) || _matchExpressions(endCtx.token.string, lineExp))) { var startCtxIndex = editor.indexFromPos({line: ctx.pos.line, ch: ctx.token.start}); var endCtxIndex = editor.indexFromPos({line: endCtx.pos.line, ch: endCtx.token.start + endCtx.token.string.length}); // Find if we aren't actually inside a block-comment result = true; - while (result && ctx.token.string.match(lineExp)) { + while (result && _matchExpressions(ctx.token.string, lineExp)) { result = TokenUtils.moveSkippingWhitespace(TokenUtils.movePrevToken, ctx); } @@ -255,7 +294,7 @@ define(function (require, exports, module) { } // Find if all the lines are line-commented. - if (!_containsUncommented(editor, sel.start.line, endLine, linePrefix)) { + if (!_containsUncommented(editor, sel.start.line, endLine, linePrefixes)) { lineUncomment = true; // Block-comment in all the other cases @@ -327,7 +366,7 @@ define(function (require, exports, module) { return; } else if (lineUncomment) { - lineCommentPrefix(editor, linePrefix); + lineCommentPrefix(editor, linePrefixes); } else { doc.batchOperation(function () { diff --git a/src/extensions/default/LESSSupport/main.js b/src/extensions/default/LESSSupport/main.js index ef233d4b3ee..48295ea97b5 100644 --- a/src/extensions/default/LESSSupport/main.js +++ b/src/extensions/default/LESSSupport/main.js @@ -35,6 +35,6 @@ define(function (require, exports, module) { mode: "less", fileExtensions: ["less"], blockComment: ["/*", "*/"], - lineComment: "//" + lineComment: ["//"] }); }); diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 9d86f6493a9..7e2a8813954 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -39,7 +39,7 @@ * mode: "haskell", * fileExtensions: ["hs"], * blockComment: ["{-", "-}"], - * lineComment: "--" + * lineComment: ["--"] * }); * * To use that language and it's related mode, wait for the returned promise to be resolved: @@ -49,7 +49,7 @@ * * You can also refine an existing language. Currently you can only set the comment styles: * var language = LanguageManager.getLanguage("haskell"); - * language.setLineComment("--"); + * language.setLineComment(["--"]); * language.setBlockComment("{-", "-}"); * * Some CodeMirror modes define variations of themselves. They are called MIME modes. @@ -234,6 +234,7 @@ define(function (require, exports, module) { this._fileExtensions = []; this._modeToLanguageMap = {}; + this._lineCommentSyntax = []; } @@ -249,7 +250,7 @@ define(function (require, exports, module) { /** @type {Array.} File extensions that use this language */ Language.prototype._fileExtensions = null; - /** @type {{ prefix: string }} Line comment syntax */ + /** @type {Array.} Line comment syntax */ Language.prototype._lineCommentSyntax = null; /** @type {Object.} Which language to use for what CodeMirror mode */ @@ -384,25 +385,28 @@ define(function (require, exports, module) { * @return {boolean} Whether line comments are supported */ Language.prototype.hasLineCommentSyntax = function () { - return Boolean(this._lineCommentSyntax); + return Boolean(this._lineCommentSyntax.length); }; /** * Returns the prefix to use for line comments. - * @return {string} The prefix + * @return {Array.} The prefixes */ Language.prototype.getLineCommentPrefix = function () { - return this._lineCommentSyntax && this._lineCommentSyntax.prefix; + return this._lineCommentSyntax.length && this._lineCommentSyntax; }; /** * Sets the prefix to use for line comments in this language. - * @param {!string} prefix Prefix string to use for block comments (i.e. "//") + * @param {!Array.} prefixes Prefixes strings to use for line comments (i.e. ["//"]) */ - Language.prototype.setLineCommentSyntax = function (prefix) { - _validateNonEmptyString(prefix, "prefix"); - - this._lineCommentSyntax = { prefix: prefix }; + Language.prototype.setLineCommentSyntax = function (prefixes) { + var self = this; + prefixes.forEach(function (prefix) { + _validateNonEmptyString(prefix, "prefix"); + + self._lineCommentSyntax.push(prefix); + }); }; /** @@ -488,7 +492,7 @@ define(function (require, exports, module) { * @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"]) * @param {Array.} definition.blockComment Array with two entries defining the block comment prefix and suffix (i.e. [""]) - * @param {string} definition.lineComment Line comment prefix (i.e. "//") + * @param {Array.} definition.lineComment Array of multiple line comment prefixes (i.e. ["//"] or ["//", "#"]) * @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. * diff --git a/src/language/languages.json b/src/language/languages.json index bc796add524..819ae25c417 100644 --- a/src/language/languages.json +++ b/src/language/languages.json @@ -23,13 +23,14 @@ "mode": "javascript", "fileExtensions": ["js", "jsx"], "blockComment": ["/*", "*/"], - "lineComment": "//" + "lineComment": ["//"] }, "json": { "name": "JSON", "mode": ["javascript", "application/json"], - "fileExtensions": ["json"] + "fileExtensions": ["json"], + "lineComment": ["//"] }, "xml": { @@ -50,7 +51,7 @@ "mode": ["clike", "text/x-csrc"], "fileExtensions": ["c", "h", "i"], "blockComment": ["/*", "*/"], - "lineComment": "//" + "lineComment": ["//"] }, "cpp": { @@ -58,7 +59,7 @@ "mode": ["clike", "text/x-c++src"], "fileExtensions": ["cc", "cp", "cpp", "c++", "cxx", "hh", "hpp", "hxx", "h++", "ii"], "blockComment": ["/*", "*/"], - "lineComment": "//" + "lineComment": ["//"] }, "csharp": { @@ -66,14 +67,14 @@ "mode": ["clike", "text/x-csharp"], "fileExtensions": ["cs"], "blockComment": ["/*", "*/"], - "lineComment": "//" + "lineComment": ["//"] }, "clike": { "name": "clike", "mode": "clike", "blockComment": ["/*", "*/"], - "lineComment": "//" + "lineComment": ["//", "#"] }, "java": { From a07aff9f8d6586d45c5a4872653d15223c96448c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Wed, 13 Mar 2013 05:55:09 -0300 Subject: [PATCH 2/5] Fixing tests --- src/language/LanguageManager.js | 10 +++++----- test/spec/LanguageManager-test.js | 16 +++++----------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 7e2a8813954..f9db9b8b66d 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -401,12 +401,12 @@ define(function (require, exports, module) { * @param {!Array.} prefixes Prefixes strings to use for line comments (i.e. ["//"]) */ Language.prototype.setLineCommentSyntax = function (prefixes) { - var self = this; - prefixes.forEach(function (prefix) { - _validateNonEmptyString(prefix, "prefix"); + var self = this, i; + for (i = 0; i < prefixes.length; i++) { + _validateNonEmptyString(prefixes[i], "prefix"); - self._lineCommentSyntax.push(prefix); - }); + self._lineCommentSyntax.push(prefixes[i]); + } }; /** diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index 8a4f8a05188..b72bd237245 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -48,10 +48,6 @@ define(function (require, exports, module) { def.blockComment = [def.blockComment.prefix, def.blockComment.suffix]; } - if (def.lineComment) { - def.lineComment = def.lineComment.prefix; - } - return LanguageManager.defineLanguage(definition.id, def); } @@ -76,7 +72,7 @@ define(function (require, exports, module) { if (expected.lineComment) { expect(actual.hasLineCommentSyntax()).toBe(true); - expect(actual.getLineCommentPrefix()).toBe(expected.lineComment.prefix); + expect(actual.getLineCommentPrefix()).toEqual(expected.lineComment); } else { expect(actual.hasLineCommentSyntax()).toBe(false); } @@ -234,19 +230,17 @@ define(function (require, exports, module) { }, "The language should be resolved", 50); runs(function () { - expect(function () { language.setLineCommentSyntax(""); }).toThrow(new Error("prefix must not be empty")); + 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.lineComment = ["//"]; def.blockComment = { prefix: "" }; - language.setLineCommentSyntax(def.lineComment.prefix); + language.setLineCommentSyntax(def.lineComment); language.setBlockCommentSyntax(def.blockComment.prefix, def.blockComment.suffix); validateLanguage(def, language); @@ -381,7 +375,7 @@ define(function (require, exports, module) { name: "Shell", mode: "shell", fileExtensions: ["sh"], - lineComment: "#" + lineComment: ["#"] }).done(function (language) { shellLanguage = language; }); From b6a7769177d62fcfaf7cb3a2eea17db1cb6ff7e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Fri, 15 Mar 2013 02:31:37 -0300 Subject: [PATCH 3/5] Fixes after first review --- src/editor/EditorCommandHandlers.js | 75 ++++++++++++++++++----------- src/language/LanguageManager.js | 8 +-- test/spec/LanguageManager-test.js | 5 ++ 3 files changed, 56 insertions(+), 32 deletions(-) diff --git a/src/editor/EditorCommandHandlers.js b/src/editor/EditorCommandHandlers.js index 3dbfe55dcc6..3ccf37fdf62 100644 --- a/src/editor/EditorCommandHandlers.js +++ b/src/editor/EditorCommandHandlers.js @@ -48,8 +48,8 @@ define(function (require, exports, module) { /** * @private - * Creates regular expressions for multiple line comments prefixes - * @param {Array.} prefixes - Description + * Creates regular expressions for multiple line comment prefixes + * @param {Array.} prefixes - the line comment prefixes * @return {Array.} */ function _createLineExpressions(prefixes) { @@ -63,16 +63,32 @@ define(function (require, exports, module) { /** * @private * Returns true if any regular expression matches the given string - * @param {string} string - * @param {Array.} expressions + * @param {string} string - where to look + * @param {Array.} expressions - what to look * @return {boolean} */ function _matchExpressions(string, expressions) { - var matchAny = false; - expressions.forEach(function (exp) { - matchAny = matchAny || string.match(exp); + return expressions.some(function (exp) { + return string.match(exp); }); - return matchAny; + } + + /** + * @private + * Returns the line comment prefix that best matches + * @param {string} string - where to look + * @param {Array.} expressions - the line comment regular expressions + * @param {Array.} prefixes - the line comment prefixes + * @return {string} + */ + function _getLinePrefix(string, expressions, prefixes) { + var result = null; + expressions.forEach(function (exp, index) { + if (string.match(exp) && ((result && result.length < prefixes[index].length) || !result)) { + result = prefixes[index]; + } + }); + return result; } /** @@ -81,18 +97,19 @@ define(function (require, exports, module) { * @param {!Editor} editor * @param {!number} startLine - valid line inside the document * @param {!number} endLine - valid line inside the document - * @param {!Array.} prefixes - Array of line comment prefixes + * @param {!Array.} lineExp - an array of line comment prefixes regular expressions * @return {boolean} true if there is at least one uncommented line */ - function _containsUncommented(editor, startLine, endLine, prefixes) { - var lineExp = _createLineExpressions(prefixes); + function _containsUncommented(editor, startLine, endLine, lineExp) { var containsUncommented = false; var i; var line; + for (i = startLine; i <= endLine; i++) { line = editor.document.getLine(i); - // A line is commented out if it starts with 0-N whitespace chars, then "//" - if (!_matchExpressions(line, lineExp) && line.match(/\S/)) { + // A line is commented out if it starts with 0-N whitespace chars, then a line comment prefix + console.log(_matchExpressions(line, lineExp)); + if (line.match(/\S/) && !_matchExpressions(line, lineExp)) { containsUncommented = true; break; } @@ -112,10 +129,11 @@ define(function (require, exports, module) { * @param {!Array.} prefixes, e.g. ["//"] */ function lineCommentPrefix(editor, prefixes) { - var doc = editor.document; - var sel = editor.getSelection(); - var startLine = sel.start.line; - var endLine = sel.end.line; + var doc = editor.document, + sel = editor.getSelection(), + startLine = sel.start.line, + endLine = sel.end.line, + lineExp = _createLineExpressions(prefixes); // Is a range of text selected? (vs just an insertion pt) var hasSelection = (startLine !== endLine) || (sel.start.ch !== sel.end.ch); @@ -128,10 +146,10 @@ define(function (require, exports, module) { // Decide if we're commenting vs. un-commenting // Are there any non-blank lines that aren't commented out? (We ignore blank lines because // some editors like Sublime don't comment them out) - var containsUncommented = _containsUncommented(editor, startLine, endLine, prefixes); - var i, j; + var containsUncommented = _containsUncommented(editor, startLine, endLine, lineExp); + var i; var line; - var trimmedLine; + var prefix; var commentI; var updateSelection = false; @@ -152,14 +170,13 @@ define(function (require, exports, module) { } else { // Uncomment - remove first every prefix on each line (if any) for (i = startLine; i <= endLine; i++) { - line = doc.getLine(i); - trimmedLine = line.trim(); - for (j = 0; j < prefixes.length; j++) { - commentI = line.indexOf(prefixes[j]); - if (trimmedLine.indexOf(prefixes[j]) === 0) { - doc.replaceRange("", {line: i, ch: commentI}, {line: i, ch: commentI + prefixes[j].length}); - break; - } + line = doc.getLine(i); + prefix = _getLinePrefix(line, lineExp, prefixes); + + if (prefix) { + commentI = line.indexOf(prefix); + doc.replaceRange("", {line: i, ch: commentI}, {line: i, ch: commentI + prefix.length}); + break; } } } @@ -294,7 +311,7 @@ define(function (require, exports, module) { } // Find if all the lines are line-commented. - if (!_containsUncommented(editor, sel.start.line, endLine, linePrefixes)) { + if (!_containsUncommented(editor, sel.start.line, endLine, lineExp)) { lineUncomment = true; // Block-comment in all the other cases diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 7ab75aea58f..a1d2c5c506a 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -454,7 +454,7 @@ define(function (require, exports, module) { }; /** - * Returns the prefix to use for line comments. + * Returns an array of prefixes to use for line comments. * @return {Array.} The prefixes */ Language.prototype.getLineCommentPrefix = function () { @@ -462,12 +462,12 @@ define(function (require, exports, module) { }; /** - * Sets the prefix to use for line comments in this language. + * Sets the prefixes to use for line comments in this language. * @param {!string|Array.} prefix Prefix string or and array of prefix strings * to use for line comments (i.e. "//" or ["//", "#"]) */ Language.prototype.setLineCommentSyntax = function (prefix) { - var prefixes = !Array.isArray(prefix) ? [prefix] : prefix; + var prefixes = Array.isArray(prefix) ? prefix : [prefix]; var i; if (prefixes.length) { @@ -478,6 +478,8 @@ define(function (require, exports, module) { this._lineCommentSyntax.push(prefixes[i]); } this._wasModified(); + } else { + console.error("The prefix array should not be empty"); } }; diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index 5917995835a..9d8e3040648 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -39,6 +39,8 @@ define(function (require, exports, module) { beforeEach(function () { waitsForDone(LanguageManager.ready, "LanguageManager ready", 10000); + + spyOn(console, "error"); }); function defineLanguage(definition) { @@ -284,6 +286,9 @@ define(function (require, exports, module) { }, "The language should be resolved", 50); runs(function () { + language.setLineCommentSyntax([]); + expect(console.error).toHaveBeenCalledWith("The prefix array should not be empty"); + expect(function () { language.setLineCommentSyntax([""]); }).toThrow(new Error("prefix must not be empty")); expect(function () { language.setLineCommentSyntax(["#", ""]); }).toThrow(new Error("prefix must not be empty")); From 25287ad9e77ea8788706939563000b6c2139b411 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Sat, 16 Mar 2013 03:27:59 -0300 Subject: [PATCH 4/5] Fixes after second review and Editor Command Handlers tests --- src/editor/EditorCommandHandlers.js | 28 +++++----- src/language/LanguageManager.js | 2 +- src/language/languages.json | 3 +- test/spec/EditorCommandHandlers-test.js | 74 +++++++++++++++++++++++++ 4 files changed, 91 insertions(+), 16 deletions(-) diff --git a/src/editor/EditorCommandHandlers.js b/src/editor/EditorCommandHandlers.js index 3ccf37fdf62..fa410138e50 100644 --- a/src/editor/EditorCommandHandlers.js +++ b/src/editor/EditorCommandHandlers.js @@ -49,22 +49,22 @@ define(function (require, exports, module) { /** * @private * Creates regular expressions for multiple line comment prefixes - * @param {Array.} prefixes - the line comment prefixes + * @param {!Array.} prefixes - the line comment prefixes * @return {Array.} */ function _createLineExpressions(prefixes) { - var lineExps = []; + var lineExp = []; prefixes.forEach(function (prefix) { - lineExps.push(new RegExp("^\\s*" + StringUtils.regexEscape(prefix))); + lineExp.push(new RegExp("^\\s*" + StringUtils.regexEscape(prefix))); }); - return lineExps; + return lineExp; } /** * @private * Returns true if any regular expression matches the given string - * @param {string} string - where to look - * @param {Array.} expressions - what to look + * @param {!string} string - where to look + * @param {!Array.} expressions - what to look * @return {boolean} */ function _matchExpressions(string, expressions) { @@ -75,10 +75,12 @@ define(function (require, exports, module) { /** * @private - * Returns the line comment prefix that best matches - * @param {string} string - where to look - * @param {Array.} expressions - the line comment regular expressions - * @param {Array.} prefixes - the line comment prefixes + * Returns the line comment prefix that best matches the string. Since there might be line comment prefixes + * that are prefixes of other line comment prefixes, it searches throught all and returns the longest line + * comment prefix that matches the string. + * @param {!string} string - where to look + * @param {!Array.} expressions - the line comment regular expressions + * @param {!Array.} prefixes - the line comment prefixes * @return {string} */ function _getLinePrefix(string, expressions, prefixes) { @@ -108,7 +110,6 @@ define(function (require, exports, module) { for (i = startLine; i <= endLine; i++) { line = editor.document.getLine(i); // A line is commented out if it starts with 0-N whitespace chars, then a line comment prefix - console.log(_matchExpressions(line, lineExp)); if (line.match(/\S/) && !_matchExpressions(line, lineExp)) { containsUncommented = true; break; @@ -168,7 +169,7 @@ define(function (require, exports, module) { } } else { - // Uncomment - remove first every prefix on each line (if any) + // Uncomment - remove the prefix on each line (if any) for (i = startLine; i <= endLine; i++) { line = doc.getLine(i); prefix = _getLinePrefix(line, lineExp, prefixes); @@ -176,7 +177,6 @@ define(function (require, exports, module) { if (prefix) { commentI = line.indexOf(prefix); doc.replaceRange("", {line: i, ch: commentI}, {line: i, ch: commentI + prefix.length}); - break; } } } @@ -273,7 +273,7 @@ define(function (require, exports, module) { endCtx = TokenUtils.getInitialContext(editor._codeMirror, {line: sel.end.line, ch: sel.end.ch}), prefixExp = new RegExp("^" + StringUtils.regexEscape(prefix), "g"), suffixExp = new RegExp(StringUtils.regexEscape(suffix) + "$", "g"), - lineExp = linePrefixes ? _createLineExpressions(linePrefixes) : null, + lineExp = linePrefixes && linePrefixes.length ? _createLineExpressions(linePrefixes) : null, prefixPos = null, suffixPos = null, canComment = false, diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index a1d2c5c506a..6547386c2a0 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -458,7 +458,7 @@ define(function (require, exports, module) { * @return {Array.} The prefixes */ Language.prototype.getLineCommentPrefix = function () { - return this._lineCommentSyntax.length && this._lineCommentSyntax; + return this._lineCommentSyntax; }; /** diff --git a/src/language/languages.json b/src/language/languages.json index 6c86d8ad1a1..f97943dfdac 100644 --- a/src/language/languages.json +++ b/src/language/languages.json @@ -93,7 +93,8 @@ "clojure": { "name": "Clojure", "mode": "clojure", - "fileExtensions": ["clj"] + "fileExtensions": ["clj"], + "lineComment": [";", ";;"] }, "perl": { diff --git a/test/spec/EditorCommandHandlers-test.js b/test/spec/EditorCommandHandlers-test.js index 74495c0fef5..c0e5f4b5b80 100644 --- a/test/spec/EditorCommandHandlers-test.js +++ b/test/spec/EditorCommandHandlers-test.js @@ -31,6 +31,7 @@ define(function (require, exports, module) { EditorCommandHandlers = require("editor/EditorCommandHandlers"), Commands = require("command/Commands"), CommandManager = require("command/CommandManager"), + LanguageManager = require("language/LanguageManager"), SpecRunnerUtils = require("spec/SpecRunnerUtils"); describe("EditorCommandHandlers", function () { @@ -406,6 +407,79 @@ define(function (require, exports, module) { }); + describe("Line comment in languages with mutiple line comment prefixes", function () { + // Define a special version of JavaScript for testing purposes + LanguageManager.defineLanguage("javascript2", { + "name": "JavaScript2", + "mode": "javascript", + "fileExtensions": ["js2"], + "lineComment": ["//", "////", "#"] + }); + + beforeEach(function () { + setupFullEditor(null, "javascript2"); + }); + + it("should comment using the first prefix", function () { + // select first 2 lines + myEditor.setSelection({line: 0, ch: 4}, {line: 1, ch: 12}); + + CommandManager.execute(Commands.EDIT_LINE_COMMENT, myEditor); + + var lines = defaultContent.split("\n"); + lines[0] = "//function foo() {"; + lines[1] = "// function bar() {"; + var expectedText = lines.join("\n"); + + expect(myDocument.getText()).toEqual(expectedText); + expectSelection({start: {line: 0, ch: 6}, end: {line: 1, ch: 14}}); + }); + + it("should uncomment every prefix", function () { + // Start with lines 1-5 commented out, with multiple line comment variations + var lines = defaultContent.split("\n"); + lines[1] = "// function bar() {"; + lines[2] = " // "; + lines[3] = " //// a();"; + lines[4] = " "; + lines[5] = "# }"; + var startingContent = lines.join("\n"); + myDocument.setText(startingContent); + + // select lines 1-5 + myEditor.setSelection({line: 1, ch: 0}, {line: 6, ch: 0}); + + CommandManager.execute(Commands.EDIT_LINE_COMMENT, myEditor); + + expect(myDocument.getText()).toEqual(defaultContent); + expectSelection({start: {line: 1, ch: 0}, end: {line: 6, ch: 0}}); + }); + + it("should only uncomment the first prefix", function () { + // Start with lines 1-3 commented out, with multiple line comment variations + var lines = defaultContent.split("\n"); + lines[1] = "//# function bar() {"; + lines[2] = "// "; + lines[3] = "////// a();"; + var startingContent = lines.join("\n"); + myDocument.setText(startingContent); + + lines = defaultContent.split("\n"); + lines[1] = "# function bar() {"; + lines[2] = " "; + lines[3] = "// a();"; + var expectedContent = lines.join("\n"); + + // select lines 1-3 + myEditor.setSelection({line: 1, ch: 0}, {line: 4, ch: 0}); + + CommandManager.execute(Commands.EDIT_LINE_COMMENT, myEditor); + + expect(myDocument.getText()).toEqual(expectedContent); + expectSelection({start: {line: 1, ch: 0}, end: {line: 4, ch: 0}}); + }); + }); + /** * Invokes Toggle Block Comment, expects the given selection/cursor & document text, invokes From 379af7b155b66ceed0125149328cdd426c1af7b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Malbr=C3=A1n?= Date: Sat, 16 Mar 2013 18:15:10 -0300 Subject: [PATCH 5/5] Fixes after third review --- src/editor/EditorCommandHandlers.js | 16 ++++++++-------- src/language/LanguageManager.js | 10 +++++----- test/spec/LanguageManager-test.js | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/src/editor/EditorCommandHandlers.js b/src/editor/EditorCommandHandlers.js index fa410138e50..316e3f5ba3c 100644 --- a/src/editor/EditorCommandHandlers.js +++ b/src/editor/EditorCommandHandlers.js @@ -262,7 +262,7 @@ define(function (require, exports, module) { * @param {!Editor} editor * @param {!string} prefix, e.g. "" - * @param {?Array.} linePrefixes, e.g. ["//"] + * @param {!Array.} linePrefixes, e.g. ["//"] */ function blockCommentPrefixSuffix(editor, prefix, suffix, linePrefixes) { @@ -273,7 +273,7 @@ define(function (require, exports, module) { endCtx = TokenUtils.getInitialContext(editor._codeMirror, {line: sel.end.line, ch: sel.end.ch}), prefixExp = new RegExp("^" + StringUtils.regexEscape(prefix), "g"), suffixExp = new RegExp(StringUtils.regexEscape(suffix) + "$", "g"), - lineExp = linePrefixes && linePrefixes.length ? _createLineExpressions(linePrefixes) : null, + lineExp = _createLineExpressions(linePrefixes), prefixPos = null, suffixPos = null, canComment = false, @@ -289,7 +289,7 @@ define(function (require, exports, module) { } // Check if we should just do a line uncomment (if all lines in the selection are commented). - if (lineExp && (_matchExpressions(ctx.token.string, lineExp) || _matchExpressions(endCtx.token.string, lineExp))) { + if (lineExp.length && (_matchExpressions(ctx.token.string, lineExp) || _matchExpressions(endCtx.token.string, lineExp))) { var startCtxIndex = editor.indexFromPos({line: ctx.pos.line, ch: ctx.token.start}); var endCtxIndex = editor.indexFromPos({line: endCtx.pos.line, ch: endCtx.token.start + endCtx.token.string.length}); @@ -493,11 +493,11 @@ define(function (require, exports, module) { result = result && _findNextBlockComment(ctx, selEnd, prefixExp); if (className === "comment" || result || isLineSelection) { - blockCommentPrefixSuffix(editor, prefix, suffix); + blockCommentPrefixSuffix(editor, prefix, suffix, []); } else { // Set the new selection and comment it editor.setSelection(selStart, selEnd); - blockCommentPrefixSuffix(editor, prefix, suffix); + blockCommentPrefixSuffix(editor, prefix, suffix, []); // Restore the old selection taking into account the prefix change if (isMultipleLine) { @@ -525,8 +525,8 @@ define(function (require, exports, module) { var language = editor.getLanguageForSelection(); if (language.hasBlockCommentSyntax()) { - // getLineCommentPrefix returns null if no line comment syntax is defined - blockCommentPrefixSuffix(editor, language.getBlockCommentPrefix(), language.getBlockCommentSuffix(), language.getLineCommentPrefix()); + // getLineCommentPrefixes always return an array, and will be empty if no line comment syntax is defined + blockCommentPrefixSuffix(editor, language.getBlockCommentPrefix(), language.getBlockCommentSuffix(), language.getLineCommentPrefixes()); } } @@ -543,7 +543,7 @@ define(function (require, exports, module) { var language = editor.getLanguageForSelection(); if (language.hasLineCommentSyntax()) { - lineCommentPrefix(editor, language.getLineCommentPrefix()); + lineCommentPrefix(editor, language.getLineCommentPrefixes()); } else if (language.hasBlockCommentSyntax()) { lineCommentPrefixSuffix(editor, language.getBlockCommentPrefix(), language.getBlockCommentSuffix()); } diff --git a/src/language/LanguageManager.js b/src/language/LanguageManager.js index 6547386c2a0..04e8c045ac5 100644 --- a/src/language/LanguageManager.js +++ b/src/language/LanguageManager.js @@ -285,12 +285,12 @@ define(function (require, exports, module) { /** @type {Array.} File names for extensionless files that use this language */ Language.prototype._fileNames = null; - /** @type {Object.} Which language to use for what CodeMirror mode */ - Language.prototype._modeToLanguageMap = null; - /** @type {Array.} 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; @@ -450,14 +450,14 @@ define(function (require, exports, module) { * @return {boolean} Whether line comments are supported */ Language.prototype.hasLineCommentSyntax = function () { - return Boolean(this._lineCommentSyntax.length); + return this._lineCommentSyntax.length > 0; }; /** * Returns an array of prefixes to use for line comments. * @return {Array.} The prefixes */ - Language.prototype.getLineCommentPrefix = function () { + Language.prototype.getLineCommentPrefixes = function () { return this._lineCommentSyntax; }; diff --git a/test/spec/LanguageManager-test.js b/test/spec/LanguageManager-test.js index 9d8e3040648..e81004b73b5 100644 --- a/test/spec/LanguageManager-test.js +++ b/test/spec/LanguageManager-test.js @@ -76,7 +76,7 @@ define(function (require, exports, module) { if (expected.lineComment) { var lineComment = Array.isArray(expected.lineComment) ? expected.lineComment : [expected.lineComment]; expect(actual.hasLineCommentSyntax()).toBe(true); - expect(actual.getLineCommentPrefix().toString()).toBe(lineComment.toString()); + expect(actual.getLineCommentPrefixes().toString()).toBe(lineComment.toString()); } else { expect(actual.hasLineCommentSyntax()).toBe(false); }