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

Fixed: #1863 Selector width info is wrong when no space before '{' #3082

Merged
merged 5 commits into from
Mar 14, 2013
Merged

Fixed: #1863 Selector width info is wrong when no space before '{' #3082

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7835872
Enhanced the ability to find the selectorStart
WebsiteDeveloper Mar 9, 2013
ee203cf
Fixed a few other cases of incorrect selections
WebsiteDeveloper Mar 14, 2013
07c0dc7
Added unit tests
WebsiteDeveloper Mar 14, 2013
28d0ce7
Added two more unit tests
WebsiteDeveloper Mar 14, 2013
f3fc31c
A bit test cleanup
WebsiteDeveloper Mar 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
20 changes: 11 additions & 9 deletions src/language/CSSUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,10 +516,10 @@ define(function (require, exports, module) {
return true;
}

function _parseSelector() {
function _parseSelector(start) {

currentSelector = "";
selectorStartChar = stream.start;
selectorStartChar = start;
selectorStartLine = line;

// Everything until the next ',' or '{' is part of the current selector
Expand Down Expand Up @@ -548,15 +548,18 @@ define(function (require, exports, module) {
}

currentSelector = currentSelector.trim();
var startChar = (selectorGroupStartLine === -1) ? selectorStartChar : selectorStartChar + 1;
var selectorStart = (stream.string.indexOf(currentSelector, selectorStartChar) !== -1) ? stream.string.indexOf(currentSelector, selectorStartChar - currentSelector.length) : startChar;

if (currentSelector !== "") {
selectors.push({selector: currentSelector,
ruleStartLine: ruleStartLine,
ruleStartChar: ruleStartChar,
selectorStartLine: selectorStartLine,
selectorStartChar: selectorStartChar,
selectorStartChar: selectorStart,
declListEndLine: -1,
selectorEndLine: line,
selectorEndChar: stream.start - 1, // stream.start points to the first char of the non-selector token
selectorEndChar: selectorStart + currentSelector.length,
selectorGroupStartLine: selectorGroupStartLine,
selectorGroupStartChar: selectorGroupStartChar
});
Expand All @@ -566,16 +569,15 @@ define(function (require, exports, module) {
}

function _parseSelectorList() {

selectorGroupStartLine = line;
selectorGroupStartLine = (stream.string.indexOf(",") !== -1) ? line : -1;
selectorGroupStartChar = stream.start;

_parseSelector();
_parseSelector(stream.start);
while (token === ",") {
if (!_nextTokenSkippingComments()) {
break;
}
_parseSelector();
_parseSelector(stream.start);
}
}

Expand Down Expand Up @@ -1027,7 +1029,7 @@ define(function (require, exports, module) {

exports.getInfoAtPos = getInfoAtPos;

// The createInfo is reallyonly for the unit tests so they can make the same
// The createInfo is really only for the unit tests so they can make the same
// structure to compare results with.
exports.createInfo = createInfo;
});
23 changes: 23 additions & 0 deletions test/spec/CSSUtils-test-files/selector-positions.css
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
div{
color: "green";
}

div
{
color: "green";
}

div
{
color: "green";
}

h3, h2, h1
{
color: "yellow";
}

h3, h2, h1
{
color: "yellow";
}
75 changes: 62 additions & 13 deletions test/spec/CSSUtils-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,21 +27,22 @@
define(function (require, exports, module) {
'use strict';

var NativeFileSystem = require("file/NativeFileSystem").NativeFileSystem,
Async = require("utils/Async"),
FileUtils = require("file/FileUtils"),
CSSUtils = require("language/CSSUtils"),
SpecRunnerUtils = require("spec/SpecRunnerUtils");
var NativeFileSystem = require("file/NativeFileSystem").NativeFileSystem,
Async = require("utils/Async"),
FileUtils = require("file/FileUtils"),
CSSUtils = require("language/CSSUtils"),
SpecRunnerUtils = require("spec/SpecRunnerUtils");

var testPath = SpecRunnerUtils.getTestPath("/spec/CSSUtils-test-files"),
simpleCssFileEntry = new NativeFileSystem.FileEntry(testPath + "/simple.css"),
universalCssFileEntry = new NativeFileSystem.FileEntry(testPath + "/universal.css"),
groupsFileEntry = new NativeFileSystem.FileEntry(testPath + "/groups.css"),
offsetsCssFileEntry = new NativeFileSystem.FileEntry(testPath + "/offsets.css"),
bootstrapCssFileEntry = new NativeFileSystem.FileEntry(testPath + "/bootstrap.css"),
escapesCssFileEntry = new NativeFileSystem.FileEntry(testPath + "/escaped-identifiers.css");
var testPath = SpecRunnerUtils.getTestPath("/spec/CSSUtils-test-files"),
simpleCssFileEntry = new NativeFileSystem.FileEntry(testPath + "/simple.css"),
universalCssFileEntry = new NativeFileSystem.FileEntry(testPath + "/universal.css"),
groupsFileEntry = new NativeFileSystem.FileEntry(testPath + "/groups.css"),
offsetsCssFileEntry = new NativeFileSystem.FileEntry(testPath + "/offsets.css"),
bootstrapCssFileEntry = new NativeFileSystem.FileEntry(testPath + "/bootstrap.css"),
escapesCssFileEntry = new NativeFileSystem.FileEntry(testPath + "/escaped-identifiers.css"),
selectorPositionsFileEntry = new NativeFileSystem.FileEntry(testPath + "/selector-positions.css");

var contextTestCss = require("text!spec/CSSUtils-test-files/contexts.css");
var contextTestCss = require("text!spec/CSSUtils-test-files/contexts.css");

/**
* Verifies whether one of the results returned by CSSUtils._findAllMatchingSelectorsInText()
Expand Down Expand Up @@ -511,6 +512,54 @@ define(function (require, exports, module) {

});

describe("find correct positions of selectors", function () {
var selectors;

beforeEach(function () {
init(this, selectorPositionsFileEntry);
runs(function () {
selectors = CSSUtils.extractAllSelectors(this.fileCssContent);
Copy link
Contributor

Choose a reason for hiding this comment

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

Since you're using the same .css file for all tests in this section, and the file is not being edited, there's no need to run extractAllSelectors() (which is not cheap) before each test. The tests will run faster if this is done only once.

});
});

afterEach(function () {
selectors = null;
});

it("should find selector positions when no whitespace between selector and '{'", function () {
expect([selectors[0].selectorStartChar, selectors[0].selectorEndChar]).toEqual([0, 3]);
});

it("should find selector positions when '{' on the next line", function () {
expect([selectors[1].selectorStartChar, selectors[1].selectorEndChar]).toEqual([0, 3]);
});

it("should find selector positions when '{' on the next line and selector is indented", function () {
expect({ start: selectors[2].selectorStartChar, end: selectors[2].selectorEndChar}).toEqual({ start: 4, end: 7});
});

it("should find selector positions in a selector group when '{' on the next line", function () {
var expected = [0, 2, 4, 6, 8, 10],
result = [
selectors[3].selectorStartChar, selectors[3].selectorEndChar,
selectors[4].selectorStartChar, selectors[4].selectorEndChar,
selectors[5].selectorStartChar, selectors[5].selectorEndChar
];

expect(result).toEqual(expected);
});

it("should find selector positions in a selector group when '{' on the next line and selector group is indented", function () {
var expected = [4, 6, 8, 10, 12, 14],
result = [
selectors[6].selectorStartChar, selectors[6].selectorEndChar,
selectors[7].selectorStartChar, selectors[7].selectorEndChar,
selectors[8].selectorStartChar, selectors[8].selectorEndChar
];

expect(result).toEqual(expected);
});
});
}); // describe("CSSUtils")


Expand Down