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

Commit 045fae6

Browse files
peterflynnpeterflynn
committed
Cleanups:
* DocumentManager: use CollectionUtils.forEach() in notifyPathNameChanged() (removing early-exit optimization that's probably unneeded; can reintroduce when CollectionUtils.some() lands) * Move filename->language mapping tests from Editor-test to LanguageManager-test * Add another #330 testcase as suggested by Glenn in #3064 * Add StringMatch testcases as suggested by Kevin in #3417 * SpecRunnerUtils: use waitsForDone() more * CSSUtils-test: remove unused vars * Dialogs: tiny docs improvement
1 parent 5dc6489 commit 045fae6

File tree

8 files changed

+79
-68
lines changed

8 files changed

+79
-68
lines changed

‎src/document/DocumentManager.js

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1161,34 +1161,25 @@ define(function (require, exports, module) {
11611161
* @param {boolean} isFolder True if path is a folder; False if it is a file.
11621162
*/
11631163
function notifyPathNameChanged(oldName, newName, isFolder) {
1164-
var i, path;
1164+
var i;
11651165

11661166
// Update open documents. This will update _currentDocument too, since
11671167
// the current document is always open.
11681168
var keysToDelete = [];
1169-
for (path in _openDocuments) {
1170-
if (_openDocuments.hasOwnProperty(path)) {
1171-
if (FileUtils.isAffectedWhenRenaming(path, oldName, newName, isFolder)) {
1172-
var doc = _openDocuments[path];
1173-
1174-
// Copy value to new key
1175-
var newKey = path.replace(oldName, newName);
1176-
_openDocuments[newKey] = doc;
1177-
1178-
keysToDelete.push(path);
1179-
1180-
// Update document file
1181-
FileUtils.updateFileEntryPath(doc.file, oldName, newName, isFolder);
1182-
doc._notifyFilePathChanged();
1183-
1184-
if (!isFolder) {
1185-
// If the path name is a file, there can only be one matched entry in the open document
1186-
// list, which we just updated. Break out of the for .. in loop.
1187-
break;
1188-
}
1189-
}
1169+
CollectionUtils.forEach(_openDocuments, function (doc, path) {
1170+
if (FileUtils.isAffectedWhenRenaming(path, oldName, newName, isFolder)) {
1171+
// Copy value to new key
1172+
var newKey = path.replace(oldName, newName);
1173+
_openDocuments[newKey] = doc;
1174+
1175+
keysToDelete.push(path);
1176+
1177+
// Update document file
1178+
FileUtils.updateFileEntryPath(doc.file, oldName, newName, isFolder);
1179+
doc._notifyFilePathChanged();
11901180
}
1191-
}
1181+
});
1182+
11921183
// Delete the old keys
11931184
for (i = 0; i < keysToDelete.length; i++) {
11941185
delete _openDocuments[keysToDelete[i]];

‎src/widgets/Dialogs.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ define(function (require, exports, module) {
215215
* -- the HTML for the dialog contains elements with "title" and "message" classes, as well as a number
216216
* of elements with "dialog-button" class, each of which has a "data-button-id".
217217
*
218-
* @param {string} dlgClass The class of the dialog node in the HTML.
218+
* @param {string} dlgClass The class of the dialog node in the HTML. Typically one of DIALOG_ID_*.
219219
* @param {string=} title The title of the error dialog. Can contain HTML markup. If unspecified, title in
220220
* the HTML template is used unchanged.
221221
* @param {string=} message The message to display in the error dialog. Can contain HTML markup. If

‎test/spec/CSSUtils-test.js

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1404,18 +1404,14 @@ define(function (require, exports, module) {
14041404
var testPath = SpecRunnerUtils.getTestPath("/spec/CSSUtils-test-files"),
14051405
CSSUtils,
14061406
DocumentManager,
1407-
FileViewController,
1408-
ProjectManager,
1409-
brackets;
1407+
FileViewController;
14101408

14111409
beforeEach(function () {
14121410
SpecRunnerUtils.createTestWindowAndRun(this, function (testWindow) {
14131411
// Load module instances from brackets.test
1414-
brackets = testWindow.brackets;
14151412
CSSUtils = testWindow.brackets.test.CSSUtils;
14161413
DocumentManager = testWindow.brackets.test.DocumentManager;
14171414
FileViewController = testWindow.brackets.test.FileViewController;
1418-
ProjectManager = testWindow.brackets.test.ProjectManager;
14191415

14201416
SpecRunnerUtils.loadProjectInTestWindow(testPath);
14211417
});

‎test/spec/Editor-test.js

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ define(function (require, exports, module) {
5656
}
5757

5858
describe("Editor", function () {
59-
var defaultContent = 'Brackets is going to be awesome!\n';
59+
var defaultContent = "Brackets is going to be awesome!\n";
6060
var myDocument, myEditor;
6161

6262
function createTestEditor(content, languageId) {
@@ -110,36 +110,14 @@ define(function (require, exports, module) {
110110
myEditor._codeMirror.setValue("new content");
111111
expect(changeFired).toBe(true);
112112
});
113-
114-
});
115-
116-
describe("File extension to mode mapping", function () {
117-
118-
it("should switch to the HTML mode for files ending in .html", function () {
119-
// verify editor content
120-
var mode = LanguageManager.getLanguageForPath("file:///only/testing/the/path.html").getMode();
121-
expect(mode).toSpecifyModeNamed("text/x-brackets-html");
122-
});
123-
124-
it("should switch modes even if the url has a query string", function () {
125-
// verify editor content
126-
var mode = LanguageManager.getLanguageForPath("http://only.org/testing/the/path.css?v=2").getMode();
127-
expect(mode).toSpecifyModeNamed(langNames.css.mode);
128-
});
129113

130-
it("should accept just a file name too", function () {
131-
// verify editor content
132-
var mode = LanguageManager.getLanguageForPath("path.js").getMode();
133-
expect(mode).toSpecifyModeNamed(langNames.javascript.mode);
134-
});
135-
136-
it("should default to plain text for unknown file extensions", function () {
137-
// verify editor content
138-
var mode = LanguageManager.getLanguageForPath("test.foo").getMode();
114+
it("should set mode based on Document language", function () {
115+
createTestEditor(defaultContent, "html");
139116

140-
// "unknown" mode uses it's MIME type instead
141-
expect(mode).toBe("text/plain");
117+
var htmlLanguage = LanguageManager.getLanguage("html");
118+
expect(myEditor.getModeForDocument()).toBe(htmlLanguage.getMode());
142119
});
120+
143121
});
144122

145123
describe("Focus", function () {

‎test/spec/FileIndexManager-test.js

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ define(function (require, exports, module) {
8989

9090
});
9191

92-
it("should handle simultaneous requests without doing extra work", function () { // #330
92+
it("should handle identical simultaneous requests without doing extra work", function () { // #330
9393
// Open a directory
9494
SpecRunnerUtils.loadProjectInTestWindow(testPath);
9595

@@ -105,7 +105,7 @@ define(function (require, exports, module) {
105105
.done(function (result) {
106106
allFiles1 = result;
107107
});
108-
promise2 = FileIndexManager.getFileInfoList("all")
108+
promise2 = FileIndexManager.getFileInfoList("all") // same request again
109109
.done(function (result) {
110110
allFiles2 = result;
111111
});
@@ -124,6 +124,41 @@ define(function (require, exports, module) {
124124
});
125125
});
126126

127+
it("should handle differing simultaneous requests without doing extra work", function () { // #330
128+
// Open a directory
129+
SpecRunnerUtils.loadProjectInTestWindow(testPath);
130+
131+
var projectRoot;
132+
var promise1, promise2;
133+
var allFiles1, allFiles2;
134+
runs(function () {
135+
projectRoot = ProjectManager.getProjectRoot();
136+
spyOn(projectRoot, "createReader").andCallThrough();
137+
138+
// Kick off two index requests in parallel
139+
promise1 = FileIndexManager.getFileInfoList("all")
140+
.done(function (result) {
141+
allFiles1 = result;
142+
});
143+
promise2 = FileIndexManager.getFileInfoList("css") // different request in parallel
144+
.done(function (result) {
145+
allFiles2 = result;
146+
});
147+
148+
waitsForDone(promise1, "First FileIndexManager.getFileInfoList()");
149+
waitsForDone(promise2, "Second FileIndexManager.getFileInfoList()");
150+
});
151+
152+
runs(function () {
153+
// Correct response to both promises
154+
expect(allFiles1.length).toEqual(8);
155+
expect(allFiles2.length).toEqual(3);
156+
157+
// Didn't scan project tree twice
158+
expect(projectRoot.createReader.callCount).toBe(1);
159+
});
160+
});
161+
127162
it("should match a specific filename and return the correct FileInfo", function () {
128163
// Open a directory
129164
SpecRunnerUtils.loadProjectInTestWindow(testPath);

‎test/spec/LanguageManager-test.js

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,11 +129,21 @@ define(function (require, exports, module) {
129129

130130
it("should map file extensions to languages", function () {
131131
var html = LanguageManager.getLanguage("html"),
132+
css = LanguageManager.getLanguage("css"),
132133
unknown = LanguageManager.getLanguage("unknown");
133134

135+
// Bare file names
134136
expect(LanguageManager.getLanguageForPath("foo.html")).toBe(html);
135137
expect(LanguageManager.getLanguageForPath("INDEX.HTML")).toBe(html);
136138
expect(LanguageManager.getLanguageForPath("foo.doesNotExist")).toBe(unknown);
139+
140+
// URIs
141+
expect(LanguageManager.getLanguageForPath("file:///only/testing/the/path.html")).toBe(html);
142+
expect(LanguageManager.getLanguageForPath("http://only.org/testing/the/path.css?v=2")).toBe(css);
143+
144+
// Things that aren't extensions
145+
expect(LanguageManager.getLanguageForPath("/code/html")).toBe(unknown);
146+
expect(LanguageManager.getLanguageForPath("/code/foo.html.notreally")).toBe(unknown);
137147
});
138148

139149
it("should map complex file extensions to languages", function () {

‎test/spec/SpecRunnerUtils.js

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -338,23 +338,18 @@ define(function (require, exports, module) {
338338
dismissButton.click();
339339

340340
// Dialog should resolve/reject the promise
341-
waitsForDone(promise);
341+
waitsForDone(promise, "dismiss dialog");
342342
}
343343

344344

345345
function loadProjectInTestWindow(path) {
346-
var isReady = false;
347-
348346
runs(function () {
349347
// begin loading project path
350348
var result = _testWindow.brackets.test.ProjectManager.openProject(path);
351-
result.done(function () {
352-
isReady = true;
353-
});
349+
350+
// wait for file system to finish loading
351+
waitsForDone(result, "ProjectManager.openProject()");
354352
});
355-
356-
// wait for file system to finish loading
357-
waitsFor(function () { return isReady; }, "openProject() timeout", 1000);
358353
}
359354

360355
/**

‎test/spec/StringMatch-test.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -641,18 +641,24 @@ define(function (require, exports, module) {
641641
expect(toStringResult).toBeTruthy();
642642
toStringResult = matcher.match("toString", "x");
643643
expect(toStringResult).toBeFalsy();
644+
toStringResult = matcher.match("toString", "xx"); // 2nd no-match to test _noMatchCache
645+
expect(toStringResult).toBeFalsy();
644646

645647
// Array.prototype has length
646648
var lengthResult = matcher.match("length", "l");
647649
expect(lengthResult).toBeTruthy();
648650
lengthResult = matcher.match("length", "x");
649651
expect(lengthResult).toBeFalsy();
652+
lengthResult = matcher.match("length", "xx"); // 2nd no-match to test _noMatchCache
653+
expect(lengthResult).toBeFalsy();
650654

651655
// Object.prototype has hasOwnProperty
652656
var hasOwnPropertyResult = matcher.match("hasOwnProperty", "h");
653657
expect(hasOwnPropertyResult).toBeTruthy();
654658
hasOwnPropertyResult = matcher.match("hasOwnProperty", "x");
655659
expect(hasOwnPropertyResult).toBeFalsy();
660+
hasOwnPropertyResult = matcher.match("hasOwnProperty", "xx"); // 2nd no-match to test _noMatchCache
661+
expect(hasOwnPropertyResult).toBeFalsy();
656662
});
657663

658664
it("can reset the caches", function () {

0 commit comments

Comments
 (0)