Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Fix #3057: Toggle Block Comment doesn't work if the open/close delimiters are the same #3060

Closed
wants to merge 13 commits into from
Closed

Fix #3057: Toggle Block Comment doesn't work if the open/close delimiters are the same #3060

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
2d6039c
Fixing issues when open and close delimiters are the same
TomMalbran Mar 7, 2013
5fa487c
Merging to master
TomMalbran Mar 19, 2013
29d68d0
Improving the Block and Line comment fix
TomMalbran Mar 19, 2013
06e42b6
Fix Language test
TomMalbran Mar 19, 2013
e6c2abf
Adding new tests for languages like CoffeeScript
TomMalbran Mar 20, 2013
a283618
Fixing tests and docs
TomMalbran Mar 20, 2013
4b9cc7e
Fixes after second review
TomMalbran Mar 30, 2013
bedec43
Fix JSHint
TomMalbran Mar 30, 2013
1038904
Merge remote-tracking branch 'upstream/master' into tom/fix-issue-3057
TomMalbran Mar 30, 2013
cbf893a
Big code refactor
TomMalbran Apr 1, 2013
8c11ca7
Simplify the first step
TomMalbran Apr 1, 2013
3ec3cfa
Merge remote-tracking branch 'upstream/master' into tom/fix-issue-3057
TomMalbran Apr 13, 2013
851cfea
Updates and minor refactor after CodeMirror update
TomMalbran Apr 14, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
164 changes: 111 additions & 53 deletions src/editor/EditorCommandHandlers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,23 @@ define(function (require, exports, module) {
return lineExp;
}

/**
* @private
* Creates regular expressions for the block comment prefix and suffix
* @param {!string} prefixes - the block comment prefix
* @param {!string} prefixes - the block comment suffix
* @return {Array.<RegExp>}
*/
function _createBlockExpressions(prefix, suffix) {
if (prefix) {
return [
new RegExp("^" + StringUtils.regexEscape(prefix), "g"),
new RegExp(StringUtils.regexEscape(suffix) + "$", "g")
];
}
return [];
}

/**
* @private
* Returns true if any regular expression matches the given string
Expand Down Expand Up @@ -99,18 +116,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.<string>} lineExp - an array of line comment prefixes regular expressions
* @param {!Array.<RegExp>} lineExp - an array of line comment prefixes regular expressions
* @param {!Array.<RegExp>} blockExp - a prefix and a suffix block comment regular expressions
* @return {boolean} true if there is at least one uncommented line
*/
function _containsUncommented(editor, startLine, endLine, lineExp) {
function _containsUncommented(editor, startLine, endLine, lineExp, blockExp) {
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 a line comment prefix
if (line.match(/\S/) && !_matchExpressions(line, lineExp)) {
if ((line.match(/\S/) && !_matchExpressions(line, lineExp)) || _matchExpressions(line.trim(), blockExp)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the comment above this could use some updating. It's not immediately obvious why finding a block comment delimiter means there's an uncommented out line. Don't you need to know (a) whether it's an open vs. close delimiter, and (b) whether any non-blank lines exist on the "outside" side of the delimiter (and still within the startLine-endLine range)?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And as a corollary, it seems like with symmetrical delimiters (like "###"), you can only know "(a)" by looking at the tokens...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just here so when you want to toggle line comment a line starting with 0-N whitespaces and then a block comment prefix or suffix where at least one line comment prefix is the prefix of either one, then it shouldn't remove part of the block comment prefix, and instead just comment it. This doesn't check block comments and try to to be smart about them.

The case I am trying to avoid is when you use toggle block comment in the first line of the next code:

  ###
  block comment
  ###

that will turn into:

  ##
  block comment
  ###

Which breaks the block comment and doesn't do either a line comment or uncomment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think that makes sense. In that case, maybe the name should be changed to _containsNotLineComment() -- since block comments will cause it to return true too, and since that's actually what the code calling it wants to know.

Or if that name is too awkward you could invert the sense of it and use a name like _isAllLineCommented().

containsUncommented = true;
break;
}
Expand All @@ -128,13 +146,16 @@ define(function (require, exports, module) {
*
* @param {!Editor} editor
* @param {!Array.<string>} prefixes, e.g. ["//"]
* @param {?string} blockPrefix, e.g. "<!--"
* @param {?string} blockSuffix, e.g. "-->"
*/
function lineCommentPrefix(editor, prefixes) {
function lineCommentPrefix(editor, prefixes, blockPrefix, blockSuffix) {
var doc = editor.document,
sel = editor.getSelection(),
startLine = sel.start.line,
endLine = sel.end.line,
lineExp = _createLineExpressions(prefixes);
lineExp = _createLineExpressions(prefixes),
blockExp = _createBlockExpressions(blockPrefix, blockSuffix);

// Is a range of text selected? (vs just an insertion pt)
var hasSelection = (startLine !== endLine) || (sel.start.ch !== sel.end.ch);
Expand All @@ -147,7 +168,7 @@ 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, lineExp);
var containsUncommented = _containsUncommented(editor, startLine, endLine, lineExp, blockExp);
var i;
var line;
var prefix;
Expand Down Expand Up @@ -191,59 +212,75 @@ define(function (require, exports, module) {
}


/**
* @private
* Returns true if the token is a Block Comment
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be just a subset of a block comment -- might be worth clarifying that. (Unlike line comments, which are always a single token afaict).

* @param {!{editor:{CodeMirror}, pos:{ch:{number}, line:{number}}, token:{object}}} ctx - token context
* @param {!Array.<RegExp>} blockExp - a prefix and a suffix block comment regular expressions
* @param {!Array.<RegExp>} lineExp - an array of line comment prefixes regular expressions
* @return {boolean}
*/
function _isBlockComment(ctx, blockExp, lineExp) {
return ctx.token.className === "comment" &&
(_matchExpressions(ctx.token.string, blockExp) || !_matchExpressions(ctx.token.string, lineExp));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be good to explain why we need to check both (i.e. line comment delimiter may be a substring of block comment delimiter).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I didn't do it for that reason, I was thinking of the case where a line comment would come after the block comment, but I didn't thought about the cases where the line comments could be inside the block comment, and thinking it better, maybe this last check isn't needed? I need to think more about it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, seems like we need to sort out what _findCommentStart()/End() actually need before deciding what this function should be checking for exactly.

}

/**
* @private
* Moves the token context to the token that starts the block-comment. Ctx starts in a block-comment.
* Returns the position of the prefix or null if gets to the start of the document and didn't found it.
* @param {!{editor:{CodeMirror}, pos:{ch:{string}, line:{number}}, token:{object}}} ctx - token context
* @param {!RegExp} prefixExp - a valid regular expression
* @param {!{editor:{CodeMirror}, pos:{ch:{number}, line:{number}}, token:{object}}} ctx - token context
* @param {!Array.<RegExp>} blockExp - a prefix and a suffix block comment regular expressions
* @param {!Array.<RegExp>} lineExp - an array of line comment prefixes regular expressions
* @return {?{line: number, ch: number}}
*/
function _findCommentStart(ctx, prefixExp) {
var result = true;
function _findCommentStart(ctx, blockExp, lineExp) {
var result = _isBlockComment(ctx, blockExp, lineExp);

while (result && !ctx.token.string.match(prefixExp)) {
result = TokenUtils.moveSkippingWhitespace(TokenUtils.movePrevToken, ctx);
while (result && !ctx.token.string.match(blockExp[0])) {
result = TokenUtils.moveSkippingWhitespace(TokenUtils.movePrevToken, ctx) &&
_isBlockComment(ctx, blockExp, lineExp);
}
return result ? {line: ctx.pos.line, ch: ctx.token.start} : null;
}

/**
* @private
* Moves the token context to the token that ends the block-comment. Ctx starts in a block-comment.
* Returns the position of the sufix or null if gets to the end of the document and didn't found it.
* @param {!{editor:{CodeMirror}, pos:{ch:{string}, line:{number}}, token:{object}}} ctx - token context
* @param {!RegExp} suffixExp - a valid regular expression
* Returns the position of the suffix or null if it gets to the end of the document and didn't found it.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should clarify that this returns the start of the suffix delimiter, so the actual comment end is a few chars later. At first glance, I would have incorrectly assumed that the range of the comment is _findCommentStart() - _findCommentEnd().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, it actually tryes to find the Suffix, maybe the name is wrong too and should be _findCommentSuffix and _findCommentStart -> _findCommentPrefix?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That renaming sounds reasonable, but just clarifying the docs seems ok to me too.

* @param {!{editor:{CodeMirror}, pos:{ch:{number}, line:{number}}, token:{object}}} ctx - token context
* @param {!Array.<RegExp>} blockExp - a prefix and a suffix block comment regular expressions
* @param {!Array.<RegExp>} lineExp - an array of line comment prefixes regular expressions
* @param {!number} suffixLen - length of the suffix
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot to update this JSDocs. Will do it on the next commit

* @return {?{line: number, ch: number}}
*/
function _findCommentEnd(ctx, suffixExp, suffixLen) {
var result = true;
function _findCommentEnd(ctx, blockExp, lineExp, suffixLen) {
var result = _isBlockComment(ctx, blockExp, lineExp);

while (result && !ctx.token.string.match(suffixExp)) {
result = TokenUtils.moveSkippingWhitespace(TokenUtils.moveNextToken, ctx);
while (result && !ctx.token.string.match(blockExp[1])) {
result = TokenUtils.moveSkippingWhitespace(TokenUtils.moveNextToken, ctx) &&
_isBlockComment(ctx, blockExp, lineExp);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there's a line comment within a block comment, wouldn't this cause us to halt early and never find the end of the block comment?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And actually same about the reverse direction in _findCommentStart()... I wonder if it's actually enough just to have the one _isBlockComment() check at the top outside of the loop, and leave the loop code itself unchanged -- if the token we start from is inside a block comment, we shouldn't care about line comments we find in between there and the block comment delimiter.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Btw, we should add a couple unit tests for these nesting cases. E.g. try to block-uncomment a block comment containing a line comment -- right now that does nothing, I think because if the issues described above.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I didn't thought about those cases, was trying to cover the line comments right before or after the block comment. That is a good idea, to leave it just outside, but it might still check the line comment inside the block comment, and break early or if we remove the line comment prefix check and the token starts in a line comment it could still confuse a block suffix with a block prefix as before.

}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The repeated checks for result make the logic a tiny bit harder to follow -- maybe wrap this all in an if (result) { ... } else { return null; } to simplify? (Then you can remove the ternary operators, move the pos decl inside the if body, etc.)

return result ? {line: ctx.pos.line, ch: ctx.token.end - suffixLen} : null;
}

/**
* @private
* Moves the token context to the next block-comment if there is one before end.
* @param {!{editor:{CodeMirror}, pos:{ch:{string}, line:{number}}, token:{object}}} ctx - token context
* Moves the token context to the next block-comment if there is one before the end.
* @param {!{editor:{CodeMirror}, pos:{ch:{number}, line:{number}}, token:{object}}} ctx - token context
* @param {!{line: number, ch: number}} end - where to stop searching
* @param {!RegExp} prefixExp - a valid regular expression
* @return {boolean} - true if it found a block-comment
*/
function _findNextBlockComment(ctx, end, prefixExp) {
var index = ctx.editor.indexFromPos(end),
inside = ctx.editor.indexFromPos(ctx.pos) <= index,
result = true;
result = ctx.editor.indexFromPos(ctx.pos) <= index;

while (result && inside && !ctx.token.string.match(prefixExp)) {
result = TokenUtils.moveSkippingWhitespace(TokenUtils.moveNextToken, ctx);
inside = ctx.editor.indexFromPos(ctx.pos) <= index;
while (result && !ctx.token.string.match(prefixExp)) {
result = TokenUtils.moveSkippingWhitespace(TokenUtils.moveNextToken, ctx) &&
ctx.editor.indexFromPos(ctx.pos) <= index;
}
return result && inside && !!ctx.token.string.match(prefixExp);
return result && !!ctx.token.string.match(prefixExp);
}

/**
Expand All @@ -255,9 +292,8 @@ define(function (require, exports, module) {
* Commenting out adds the prefix before the selection and the suffix after.
* Uncommenting removes them.
*
* If slashComment is true and the start or end of the selection is inside a line-comment it
* will try to do a line uncomment if is not actually inside a bigger block comment and all
* the lines in the selection are line-commented.
* If list of line comment prefixes is provided and all the lines inside the selection are line commented,
* it will try to do a line uncomment if is not actually inside a bigger block comment.
*
* @param {!Editor} editor
* @param {!string} prefix, e.g. "<!--"
Expand All @@ -271,9 +307,8 @@ define(function (require, exports, module) {
ctx = TokenUtils.getInitialContext(editor._codeMirror, {line: sel.start.line, ch: sel.start.ch}),
startCtx = TokenUtils.getInitialContext(editor._codeMirror, {line: sel.start.line, ch: sel.start.ch}),
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 = _createLineExpressions(linePrefixes),
blockExp = _createBlockExpressions(prefix, suffix),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might make the following code easier to read if this was still broken out into prefixExp and suffixExp local vars

prefixPos = null,
suffixPos = null,
canComment = false,
Expand All @@ -283,13 +318,42 @@ define(function (require, exports, module) {

var result, text, line;

/**
* Intenal function that looks for the prefix and suffix position
* @param {!{line: number, ch: number}} newPrefixPos
*/
function findPrefixSuffix(newPrefixPos) {
prefixPos = newPrefixPos || _findCommentStart(ctx, blockExp, lineExp);
suffixPos = _findCommentEnd(ctx, blockExp, lineExp, suffix.length);

// There are some languages like CoffeeScript where the block comment prefix and suffix are the same, so
// when the prefix or suffix is alone in one line, both prefixPos and suffixPos might be the same.
// In those cases we need to find the real prefix or suffix
if (prefixPos && prefixPos.line === suffixPos.line && prefixPos.ch === suffixPos.ch) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suffixPos could be null here

// Lets create a new Token context to look for the prefix so that ctx stays at the suffix position
var newCtx = TokenUtils.getInitialContext(editor._codeMirror, {line: ctx.pos.line, ch: ctx.pos.ch });
result = TokenUtils.moveSkippingWhitespace(TokenUtils.movePrevToken, newCtx);

// The previows token is a block comment, so we found the suffix, we need to find the prefix
if (result && _isBlockComment(newCtx, blockExp, lineExp)) {
prefixPos = _findCommentStart(newCtx, blockExp, lineExp);
// We found the prefix, we move to the next token and try to find the suffix
} else {
result = TokenUtils.moveSkippingWhitespace(TokenUtils.moveNextToken, ctx);
suffixPos = result && _findCommentEnd(ctx, blockExp, lineExp, suffix.length);
}
}
}


// Move the context to the first non-empty token.
if (!ctx.token.className && ctx.token.string.trim().length === 0) {
result = TokenUtils.moveSkippingWhitespace(TokenUtils.moveNextToken, ctx);
}

// Check if we should just do a line uncomment (if all lines in the selection are commented).
if (lineExp.length && (_matchExpressions(ctx.token.string, lineExp) || _matchExpressions(endCtx.token.string, lineExp))) {
if (lineExp.length && (_matchExpressions(ctx.token.string, lineExp) || _matchExpressions(endCtx.token.string, lineExp)) &&
!ctx.token.string.match(blockExp[0])) {
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});

Expand All @@ -300,7 +364,7 @@ define(function (require, exports, module) {
}

// If we aren't in a block-comment.
if (!result || ctx.token.className !== "comment" || ctx.token.string.match(suffixExp)) {
if (!result || ctx.token.className !== "comment" || ctx.token.string.match(blockExp[1])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this logic (this diff plus the one just above it) wouldn't catch cases where a CoffeeScript-type block comment is embedded in between contiguous sets of line comments. The first line of the selection could be "#", and the while loop here could terminate on a "#", but some of the lines in between could be "###" -- the loop would continue past them because they do match lineExp too.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number of places where we have to catch this bug is high enough -- I've already lost count -- that I wonder whether we should be using a lineExp more like /#[^#]/ to guarantee that lineExp never matches a block comment. We could have some logic in _createBlockExpressions() to detect these tricky prefix cases and auto-generate such a regexp.

Edit: I guess the regexp might need to be a little trickier, e.g. /#($|[^#]|.[^#])/ to account for cases like #+eol and ## without a 3rd #... but it still seems possibly auto-generatable...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is trying to match the block prefix, not a line prefix.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, but the while loop just above here is matching on lineExp -- which without my suggested change to the regexp, would match block comments too. Hence the loop could fly past a block comment mixed in between line comments without realizing that it, and we'd erroneously decide the selection contained nothing but line comments.

E.g. a case like this:

# line 1
# line 2
###
block comment
###
# line 4
# line 5

// Is a range of text selected? (vs just an insertion pt)
var hasSelection = (sel.start.line !== sel.end.line) || (sel.start.ch !== sel.end.ch);

Expand All @@ -311,30 +375,27 @@ define(function (require, exports, module) {
}

// Find if all the lines are line-commented.
if (!_containsUncommented(editor, sel.start.line, endLine, lineExp)) {
if (!_containsUncommented(editor, sel.start.line, endLine, lineExp, blockExp)) {
lineUncomment = true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure where in here the bug lies, but I'm also seeing problems with cases like this:

###
math =
  root:   Math.sqrt
  cube:   (x) -> x * square x
###

# foo
# bar

Select the last two lines and invoke either of the Toggle Comment commands. The other block of commented-out code is uncommented instead.


// Block-comment in all the other cases
} else {
canComment = true;
}
} else {
prefixPos = _findCommentStart(startCtx, prefixExp);
suffixPos = _findCommentEnd(startCtx, suffixExp, suffix.length);
findPrefixSuffix();
}

// If we are in a selection starting and ending in invalid tokens and with no content (not considering spaces),
// find if we are inside a block-comment.
} else if (startCtx.token.className === null && endCtx.token.className === null &&
!editor.posWithinRange(ctx.pos, startCtx.pos, endCtx.pos)) {
result = TokenUtils.moveSkippingWhitespace(TokenUtils.moveNextToken, startCtx);

// We found a comment, find the start and end and check if the selection is inside the block-comment.
if (startCtx.token.className === "comment") {
prefixPos = _findCommentStart(startCtx, prefixExp);
suffixPos = _findCommentEnd(startCtx, suffixExp, suffix.length);
if (ctx.token.className === "comment") {
findPrefixSuffix();

if (prefixPos !== null && suffix !== null && !editor.posWithinRange(sel.start, prefixPos, suffixPos)) {
if (prefixPos !== null && !editor.posWithinRange(sel.start, prefixPos, suffixPos)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently it's possible to have prefixPos !== null yet suffixPos === null -- what should this code do in that case? (Right now it wouldn't even get this far since findPrefixSuffix() would crash... but assuming that's fixed...).

canComment = true;
}
} else {
Expand All @@ -343,23 +404,19 @@ define(function (require, exports, module) {

// If the start is inside a comment, find the prefix and suffix positions.
} else if (ctx.token.className === "comment") {
prefixPos = _findCommentStart(ctx, prefixExp);
suffixPos = _findCommentEnd(ctx, suffixExp, suffix.length);
findPrefixSuffix();

// If not try to find the first comment inside the selection.
} else {
result = _findNextBlockComment(ctx, sel.end, prefixExp);
result = _findNextBlockComment(ctx, sel.end, blockExp[0]);

// If nothing was found is ok to comment.
if (!result) {
canComment = true;
} else if (ctx.token.string.match(blockExp[0])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at _findNextBlockComment(), it seems like this is always true if result is true. Is the else block below this ever executed?

On the other hand, I'm not sure why the code in the else isn't good enough, i.e. why do we need to pass an explicit pos to findPrefixSuffix() -- isn't this explicit pos the same place where _findNextBlockComment() left off?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought the same, but for some reason test where failing when I removed this case before, but now I tested it again, with a few other changes in the code, and it works good without it.

findPrefixSuffix({line: ctx.pos.line, ch: ctx.token.start});
} else {
if (!ctx.token.string.match(prefixExp)) {
prefixPos = _findCommentStart(ctx, prefixExp);
} else {
prefixPos = {line: ctx.pos.line, ch: ctx.token.start};
}
suffixPos = _findCommentEnd(ctx, suffixExp, suffix.length);
findPrefixSuffix();
}
}

Expand All @@ -369,9 +426,10 @@ define(function (require, exports, module) {
if (editor.posWithinRange(start, sel.start, sel.end)) {
// Start searching at the next token, if there is one.
result = TokenUtils.moveSkippingWhitespace(TokenUtils.moveNextToken, ctx) &&
_findNextBlockComment(ctx, sel.end, prefixExp);
_findNextBlockComment(ctx, sel.end, blockExp[0]);

if (result) {
console.log("invalid", ctx);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove

invalidComment = true;
}
}
Expand Down Expand Up @@ -543,7 +601,7 @@ define(function (require, exports, module) {
var language = editor.getLanguageForSelection();

if (language.hasLineCommentSyntax()) {
lineCommentPrefix(editor, language.getLineCommentPrefixes());
lineCommentPrefix(editor, language.getLineCommentPrefixes(), language.getBlockCommentPrefix(), language.getBlockCommentSuffix());
} else if (language.hasBlockCommentSyntax()) {
lineCommentPrefixSuffix(editor, language.getBlockCommentPrefix(), language.getBlockCommentSuffix());
}
Expand Down
3 changes: 2 additions & 1 deletion src/language/languages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,8 @@
"name": "CoffeeScript",
"mode": "coffeescript",
"fileExtensions": ["coffee", "cf", "cson"],
"fileNames": ["Cakefile"]
"fileNames": ["Cakefile"],
"blockComment": ["###", "###"]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We want to add lineComment too, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do, since not having it can break stuff with out it, but since it was added in your pull request to add block and line comment for more languages, I did add it here.

},

"clojure": {
Expand Down
Loading