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

Unit tests for LESS/SCSS #8905

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
1929094
Add unit tests for live highlighting for LESS/SCSS.
RaymondLim Aug 22, 2014
6054ccb
Merge branch 'rlim/parse-less-and-scss' into rlim/less-and-scss-tests
RaymondLim Aug 22, 2014
468d75b
Merge branch 'rlim/parse-less-and-scss' into rlim/less-and-scss-tests
RaymondLim Aug 22, 2014
354eeee
- added new tests for CSS/LESS inline editing
ingorichter Aug 23, 2014
c46364e
- fixed broken test
ingorichter Aug 25, 2014
777977e
Merge branch 'rlim/parse-less-and-scss', remote-tracking branch 'orig…
RaymondLim Aug 25, 2014
3a41b54
- added more tests for CSS inline editing
ingorichter Aug 26, 2014
6669447
Merge branch 'rlim/less-and-scss-tests' of https://github.com/adobe/b…
ingorichter Aug 26, 2014
be1bcb9
Merge branch 'rlim/parse-less-and-scss' into rlim/less-and-scss-tests
RaymondLim Aug 26, 2014
b9a9fe1
Merge branch 'rlim/parse-less-and-scss' into rlim/less-and-scss-tests
RaymondLim Aug 26, 2014
899047f
Merge branch 'rlim/parse-less-and-scss' into rlim/less-and-scss-tests
RaymondLim Aug 26, 2014
c2b6dfb
- added some new test cases
ingorichter Aug 26, 2014
dc6f227
Merge branch 'rlim/less-and-scss-tests' of https://github.com/adobe/b…
RaymondLim Aug 26, 2014
5a1969e
Merge branch 'rlim/parse-less-and-scss', remote-tracking branch 'orig…
RaymondLim Aug 26, 2014
8695cf2
Fix group selector in root level issue and parent selector reference …
RaymondLim Aug 28, 2014
afb2874
- added integration test for https://github.com/adobe/brackets/issues…
ingorichter Aug 28, 2014
7e60ba5
- added integration test for https://github.com/adobe/brackets/issues…
ingorichter Aug 28, 2014
30ea286
- added integration test for https://github.com/adobe/brackets/issues…
ingorichter Aug 28, 2014
c2267e4
- added integration test for https://github.com/adobe/brackets/issues…
ingorichter Aug 28, 2014
b741dfc
Add unit tests for CSSUtils and CSS Parsing.
RaymondLim Aug 28, 2014
8a7da5e
Merge remote-tracking branch 'origin' into rlim/less-and-scss-tests
RaymondLim Aug 29, 2014
0fbe8bd
- changes based on review comments. Added some new test cases
ingorichter Sep 6, 2014
2e35a1d
- fixed failing test by re-arranging the execution order. This is wei…
ingorichter Sep 8, 2014
c1a62df
- added test case to ensure that files edited in inline editor will b…
ingorichter Sep 9, 2014
5bede5b
Fix the issues caused by commas in the property list and preceding @i…
RaymondLim Sep 9, 2014
217f225
Merge remote-tracking branch 'origin' into rlim/less-and-scss-tests
RaymondLim Sep 9, 2014
aff58c6
- fix formatting issues and use loc strings for comparison
ingorichter Sep 10, 2014
f056a53
Fix one more issue with missing semicolon in the last property.
RaymondLim Sep 10, 2014
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
181 changes: 103 additions & 78 deletions src/language/CSSUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,8 @@ define(function (require, exports, module) {
completeSelectors += info.selector;
}
return completeSelectors;
} else if (useGroup && info.selectorGroup) {
return info.selectorGroup;
}

return info.selector;
Expand Down Expand Up @@ -843,17 +845,18 @@ define(function (require, exports, module) {
}

function _maybeProperty() {
return (state.state !== "top" && state.state !== "block" &&
return (/^-(moz|ms|o|webkit)-$/.test(token) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

Vendor prefixes can also appear as a prefix on values (such as -webkit-transform and -webkit-linear-gradient in brackets/src/styles folder) or at-rules (e.g. @-webkit-keyframes), so this may not be a good check that the token is a property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's ok. We want to skip anything that is in property name or property value as long as it is not a selector. So this check will take care of any -webkit-transform that is placed on a separate line. Regarding @-webkit-keyframes, it will be taking care of by _isStartAtRule() check which is done before we call _maybeProperty().

(state.state !== "top" && state.state !== "block" &&
// Has a semicolon as in "rgb(0,0,0);", but not one of those after a LESS
// mixin parameter variable as in ".size(@width; @height)"
stream.string.indexOf(";") !== -1 && !/\([^)]+;/.test(stream.string));
stream.string.indexOf(";") !== -1 && !/\([^)]+;/.test(stream.string)));
}

function _skipProperty() {
var prevToken = "";
while (token !== ";") {
// Skip tokens until the closing brace if we find an interpolated variable.
if (/#\{$/.test(token) || (token === "{" && /@$/.test(prevToken))) {
if (/#\{$/.test(token) || (token === "{" && /[#@]$/.test(prevToken))) {
_skipToClosingBracket("{");
if (token === "}") {
_nextToken(); // Skip the closing brace
Expand All @@ -865,13 +868,14 @@ define(function (require, exports, module) {
// If there is a '{' or '}' before the ';',
// then stop skipping.
if (token === "{" || token === "}") {
return;
return false; // can't tell if the entire property is skipped
}
prevToken = token;
if (!_nextTokenSkippingComments()) {
break;
}
}
return true; // skip the entire property
}

function _getParentSelectors() {
Expand All @@ -892,7 +896,8 @@ define(function (require, exports, module) {

// Everything until the next ',' or '{' is part of the current selector
while ((token !== "," && token !== "{") ||
(token === "{" && /@$/.test(currentSelector))) {
(token === "{" && /[#@]$/.test(currentSelector)) ||
(token === "," && !_hasNonWhitespace(currentSelector))) {
if (token === "{") {
// Append the interpolated variable to selector
currentSelector += _skipToClosingBracket("{");
Expand All @@ -913,7 +918,14 @@ define(function (require, exports, module) {
// Collect everything inside the parentheses as a whole chunk so that
// commas inside the parentheses won't be identified as selector separators
// by while loop.
currentSelector += _skipToClosingBracket("(");
if (_hasNonWhitespace(currentSelector)) {
currentSelector += _skipToClosingBracket("(");
} else {
// Nothing in currentSelector yet. Skip to the closing parenthesis
// without collecting the selector since a selector cannot start with
// an opening parenthesis.
_skipToClosingBracket("(");
}
} else if (_hasNonWhitespace(token) || _hasNonWhitespace(currentSelector)) {
currentSelector += token;
}
Expand Down Expand Up @@ -1016,7 +1028,7 @@ define(function (require, exports, module) {
if (sgLine === declListStartLine) {
endChar = declListStartChar;
}
selectorGroup += lines[sgLine].substring(startChar, endChar);
selectorGroup += lines[sgLine].substring(startChar, endChar).trim();
}
selectorGroup = selectorGroup.trim();
}
Expand Down Expand Up @@ -1115,13 +1127,13 @@ define(function (require, exports, module) {
// Skip everything until the opening '{'
while (token !== "{") {
if (!_nextTokenSkippingComments()) {
return false; // eof
return; // eof
}
}

// skip past '{', to next non-ws token
if (!_nextTokenSkippingWhitespace()) {
return false; // eof
return; // eof
}

if (currentLevel <= level) {
Expand All @@ -1137,30 +1149,25 @@ define(function (require, exports, module) {
currentLevel--;
}

} else if (/@(charset|import|namespace|include|extend|warn)/i.test(token) ||
!/\{/.test(stream.string)) {

} else {
// This code handles @rules in this format:
// @rule ... ;
// Or any less variable that starts with @var ... ;
// Skip everything until the next ';'
while (token !== ";") {
// This code handle @rules that use this format:
// @rule ... { ... }
// such as @page, @keyframes (also -webkit-keyframes, etc.), and @font-face.
// Skip everything including nested braces until the next matching '}'
if (token === "{") {
_skipToClosingBracket("{");
return;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was handling of @rule ... ; and @rule ... { ... } formats combined like this? I can't think of a case off the top-of-my-head that causes a problem, but CSS is still evolving, and it doesn't seem very much code is saved here, so this seems risky.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main reason of combining the two cases is not for saving some lines of code. It is for handling both cases the right way since we want to skip all in both cases. We use while (token !== ";") to take care of the case where we don't have a { } block, but using _skipToClosingBracket("{"); to take care of the { } block. And if there is any at-rule that we want to handle in the future, it should be done before else case here. For example, we may want to handle @at-root available in SCSS. Then, we need a special case above and not inside else block. Here we want to handle all those we don't know and want to skip.

if (!_nextTokenSkippingComments()) {
return false; // eof
return; // eof
}
}

} else {
// This code handle @rules that use this format:
// @rule ... { ... }
// such as @page, @keyframes (also -webkit-keyframes, etc.), and @font-face.
// Skip everything including nested braces until the next matching '}'
_skipToClosingBracket("{");
if (token === "}") {
return false;
}
}
return true;
}

// parse a style rule
Expand All @@ -1174,17 +1181,16 @@ define(function (require, exports, module) {
}

_parseRuleList = function (escapeToken, level) {
var skipNext = true;
while ((!escapeToken) || token !== escapeToken) {
if (_isVariableInterpolatedProperty()) {
_skipProperty();
if (!_skipProperty()) {
// We found a "{" or "}" while skipping a property. Return false to handle the
// opening or closing of a block properly.
return false;
}
} else if (_isStartAtRule()) {
// @rule
if (!_parseAtRule(level) && level > 0) {
skipNext = false;
} else {
skipNext = true;
}
_parseAtRule(level);
} else if (_isStartComment()) {
// comment - make this part of style rule
if (includeCommentInNextRule()) {
Expand All @@ -1194,15 +1200,12 @@ define(function (require, exports, module) {
_parseComment();
} else if (_maybeProperty()) {
// Skip the property.
_skipProperty();
// If we find a "{" or "}" while skipping a property, then don't skip it in
// this while loop. Otherwise, we will get into an infinite loop in parsing
// since we miss to handle the opening or closing of a block properly.
if (token === "{" || token === "}") {
if (!_skipProperty()) {
// We found a "{" or "}" while skipping a property. Return false to handle the
// opening or closing of a block properly.
return false;
}
} else {
skipNext = true; // reset skipNext
// Otherwise, it's style rule
if (!_parseRule(level === undefined ? 0 : level) && level > 0) {
return false;
Expand All @@ -1216,7 +1219,7 @@ define(function (require, exports, module) {
ruleStartLine = -1;
}

if (skipNext && !_nextTokenSkippingWhitespace()) {
if (!_nextTokenSkippingWhitespace()) {
break;
}
}
Expand Down Expand Up @@ -1251,6 +1254,57 @@ define(function (require, exports, module) {
}
*/

/**
* Helper function to remove whitespaces before and after a selector
* Returns trimmed selector if it is not an at-rule, or null if it starts with @.
*
* @param {string} selector
* @return {string}
*/
function _stripAtRules(selector) {
selector = selector.trim();
if (selector.indexOf("@") === 0) {
return "";
}
return selector;
}

/**
* Converts the given selector array into the actual CSS selectors similar to
* those generated by a CSS preprocessor.
*
* @param {Array.<string>} selectorArray
* @return {string}
*/
function _getSelectorInFinalCSSForm(selectorArray) {
var finalSelectorArray = [""],
parentSelectorArray = [],
group = [];
_.forEach(selectorArray, function (selector) {
selector = _stripAtRules(selector);
group = selector.split(",");
parentSelectorArray = [];
_.forEach(group, function (cs) {
var ampersandIndex = cs.indexOf("&");
_.forEach(finalSelectorArray, function (ps) {
if (ampersandIndex === -1) {
cs = _stripAtRules(cs);
if (ps.length && cs.length) {
ps += " ";
}
ps += cs;
} else {
// Replace all instances of & with regexp
ps = _stripAtRules(cs.replace(/&/g, ps));
}
parentSelectorArray.push(ps);
});
});
finalSelectorArray = parentSelectorArray;
});
return finalSelectorArray.join(", ");
}

/**
* Finds all instances of the specified selector in "text".
* Returns an Array of Objects with start and end properties.
Expand Down Expand Up @@ -1293,11 +1347,17 @@ define(function (require, exports, module) {

var re = new RegExp(selector + "(\\[[^\\]]*\\]|:{1,2}[\\w-()]+|\\.[\\w-]+|#[\\w-]+)*\\s*$", classOrIdSelector ? "" : "i");
allSelectors.forEach(function (entry) {
if (entry.selector.search(re) !== -1) {
var actualSelector = entry.selector;
if (entry.selector.indexOf("&") !== -1 && entry.parentSelectors) {
var selectorArray = entry.parentSelectors.split(" / ");
selectorArray.push(entry.selector);
actualSelector = _getSelectorInFinalCSSForm(selectorArray);
}
if (actualSelector.search(re) !== -1) {
result.push(entry);
} else if (!classOrIdSelector) {
// Special case for tag selectors - match "*" as the rightmost character
if (/\*\s*$/.test(entry.selector)) {
if (/\*\s*$/.test(actualSelector)) {
result.push(entry);
}
}
Expand Down Expand Up @@ -1443,41 +1503,6 @@ define(function (require, exports, module) {
var isPreprocessorDoc = FileUtils.isCSSPreprocessorFile(editor.document.file.fullPath);
var selectorArray = [];

function _stripAtRules(selector) {
selector = selector.trim();
if (selector.indexOf("@") === 0) {
return "";
}
return selector;
}

function _getSelectorInFinalCSSForm() {
var finalSelectorArray = [""],
parentSelectorArray = [],
group = [];
_.forEach(selectorArray, function (selector) {
selector = _stripAtRules(selector);
group = selector.split(", ");
parentSelectorArray = [];
_.forEach(group, function (cs) {
var ampersandIndex = cs.indexOf("&");
_.forEach(finalSelectorArray, function (ps) {
if (ampersandIndex === -1) {
if (ps.length) {
ps += " ";
}
ps += cs;
} else {
ps = cs.replace("&", ps);
}
parentSelectorArray.push(ps);
});
});
finalSelectorArray = parentSelectorArray;
});
return finalSelectorArray.join(", ");
}

function _skipToOpeningBracket(ctx, startChar) {
var unmatchedBraces = 0;
if (!startChar) {
Expand Down Expand Up @@ -1560,7 +1585,7 @@ define(function (require, exports, module) {
} else if (ctx.token.string === "{") {
selector = _parseSelector(ctx);
if (isPreprocessorDoc) {
if (!skipPrevSibling && !/^\s*@media/i.test(selector)) {
if (!skipPrevSibling && !/^\s*@/.test(selector)) {
selectorArray.unshift(selector);
}
if (skipPrevSibling) {
Expand Down Expand Up @@ -1608,7 +1633,7 @@ define(function (require, exports, module) {
if (ctx.token.type !== "comment") {
if (ctx.token.string === "{") {
selector = _parseSelector(ctx);
if (isPreprocessorDoc) {
if (isPreprocessorDoc && !/^\s*@/.test(selector)) {
selectorArray.push(selector);
}
break;
Expand All @@ -1623,7 +1648,7 @@ define(function (require, exports, module) {
}

if (isPreprocessorDoc) {
return _getSelectorInFinalCSSForm();
return _getSelectorInFinalCSSForm(selectorArray);
}

return _stripAtRules(selector);
Expand Down
29 changes: 15 additions & 14 deletions test/UnitTestSuite.js
Original file line number Diff line number Diff line change
@@ -1,37 +1,38 @@
/*
* Copyright (c) 2012 Adobe Systems Incorporated. All rights reserved.
*
*
* Permission is hereby granted, free of charge, to any person obtaining a
* copy of this software and associated documentation files (the "Software"),
* to deal in the Software without restriction, including without limitation
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
* and/or sell copies of the Software, and to permit persons to whom the
* copy of this software and associated documentation files (the "Software"),
* to deal in the Software without restriction, including without limitation
* the rights to use, copy, modify, merge, publish, distribute, sublicense,
* and/or sell copies of the Software, and to permit persons to whom the
* Software is furnished to do so, subject to the following conditions:
*
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
* DEALINGS IN THE SOFTWARE.
*
*
*/

/*jslint vars: true, plusplus: true, devel: true, browser: true, nomen: true, indent: 4, maxerr: 50 */
/*global define */
define(function (require, exports, module) {
"use strict";

require("spec/Async-test");
require("spec/CodeHint-test");
require("spec/CodeHintUtils-test");
require("spec/CodeInspection-test");
require("spec/CommandManager-test");
require("spec/CSSUtils-test");
require("spec/CSSInlineEdit-test");
require("spec/JSUtils-test");
require("spec/Document-test");
require("spec/DocumentCommandHandlers-test");
Expand Down Expand Up @@ -84,4 +85,4 @@ define(function (require, exports, module) {
require("spec/ViewUtils-test");
require("spec/WorkingSetView-test");
require("spec/WorkingSetSort-test");
});
});
Loading