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

Smallish cleanups to docs & unit tests; improve a few tests #3429

Merged
merged 7 commits into from
Jul 30, 2013
Merged

Smallish cleanups to docs & unit tests; improve a few tests #3429

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5dc6489
Cleanups, mainly to unit tests:
peterflynn Apr 5, 2013
045fae6
Cleanups:
peterflynn Apr 12, 2013
f59be5a
Merge remote-tracking branch 'origin/master' into pflynn/unit-test-cl…
peterflynn Jul 23, 2013
7250b96
Fix JSLint/JSHint errors (some of which were on master).
peterflynn Jul 23, 2013
8d94d0d
Merge remote-tracking branch 'origin/master' into pflynn/unit-test-cl…
peterflynn Jul 30, 2013
98f23f2
Code review fixes - tweak docs, cleanup vars, convert for to forEach()
peterflynn Jul 30, 2013
9df7830
Code review: some more small JSDoc fixes
peterflynn Jul 30, 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
37 changes: 14 additions & 23 deletions src/document/DocumentManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,34 +812,25 @@ define(function (require, exports, module) {
* @param {boolean} isFolder True if path is a folder; False if it is a file.
*/
function notifyPathNameChanged(oldName, newName, isFolder) {
var i, path;
var i;

// Update open documents. This will update _currentDocument too, since
// the current document is always open.
var keysToDelete = [];
for (path in _openDocuments) {
if (_openDocuments.hasOwnProperty(path)) {
if (FileUtils.isAffectedWhenRenaming(path, oldName, newName, isFolder)) {
var doc = _openDocuments[path];

// Copy value to new key
var newKey = path.replace(oldName, newName);
_openDocuments[newKey] = doc;

keysToDelete.push(path);

// Update document file
FileUtils.updateFileEntryPath(doc.file, oldName, newName, isFolder);
doc._notifyFilePathChanged();

if (!isFolder) {
// If the path name is a file, there can only be one matched entry in the open document
// list, which we just updated. Break out of the for .. in loop.
break;
}
}
CollectionUtils.forEach(_openDocuments, function (doc, path) {
if (FileUtils.isAffectedWhenRenaming(path, oldName, newName, isFolder)) {
// Copy value to new key
var newKey = path.replace(oldName, newName);
_openDocuments[newKey] = doc;

keysToDelete.push(path);

// Update document file
FileUtils.updateFileEntryPath(doc.file, oldName, newName, isFolder);
doc._notifyFilePathChanged();
}
}
});

// Delete the old keys
for (i = 0; i < keysToDelete.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

This for and the next one should use Array.forEach

delete _openDocuments[keysToDelete[i]];
Expand Down
3 changes: 3 additions & 0 deletions src/editor/EditorManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,9 @@ define(function (require, exports, module) {
var editor = document._masterEditor;

if (!editor) {
if (!(document instanceof DocumentManager.Document)) {
throw new Error("_destroyEditorIfUnneeded() takes a Document arg, not an Editor");
}
return;
}

Expand Down
5 changes: 2 additions & 3 deletions src/extensions/default/HTMLCodeHints/unittests.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ define(function (require, exports, module) {
CodeHintManager = brackets.getModule("editor/CodeHintManager"),
HTMLCodeHints = require("main");

describe("HTML Attribute Hinting", function () {
describe("HTML Code Hinting", function () {

var defaultContent = "<!doctype html>\n" +
"<html>\n" +
Expand All @@ -49,7 +49,6 @@ define(function (require, exports, module) {
"</body>\n" +
"</html>\n";

var testWindow;
var testDocument, testEditor;

beforeEach(function () {
Expand Down Expand Up @@ -690,5 +689,5 @@ define(function (require, exports, module) {
});


}); // describe("HTML Attribute Hinting"
}); // describe("HTML Code Hinting"
});
23 changes: 12 additions & 11 deletions src/language/LanguageManager.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ define(function (require, exports, module) {
/**
* Resolves a language ID to a Language object.
* File names have a higher priority than file extensions.
* @param {!string} id Identifier for this language, use only letters a-z or digits 0-9 and _ inbetween (i.e. "cpp", "foo_bar", "c99")
* @param {!string} id Identifier for this language: lowercase letters and digits with _ separators (e.g. "cpp", "foo_bar", "c99")
* @return {Language} The language with the provided identifier or undefined
*/
function getLanguage(id) {
Expand Down Expand Up @@ -322,7 +322,7 @@ define(function (require, exports, module) {

/**
* Sets the identifier for this language or prints an error to the console.
* @param {!string} id Identifier for this language, use only letters a-z or digits 0-9, and _ inbetween (i.e. "cpp", "foo_bar", "c99")
* @param {!string} id Identifier for this language: lowercase letters and digits with _ separators (e.g. "cpp", "foo_bar", "c99")
* @return {boolean} Whether the ID was valid and set or not
*/
Language.prototype._setId = function (id) {
Expand Down Expand Up @@ -350,7 +350,7 @@ define(function (require, exports, module) {

/**
* Sets the human-readable name of this language or prints an error to the console.
* @param {!string} name Human-readable name of the language, as it's commonly referred to (i.e. "C++")
* @param {!string} name Human-readable name of the language, as it's commonly referred to (e.g. "C++")
* @return {boolean} Whether the name was valid and set or not
*/
Language.prototype._setName = function (name) {
Expand All @@ -373,7 +373,7 @@ define(function (require, exports, module) {
/**
* Loads a mode and sets it for this language.
*
* @param {string|Array.<string>} mode CodeMirror mode (i.e. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"]
* @param {string|Array.<string>} mode CodeMirror mode (e.g. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"]
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be {(string|Array.<string>)} since is an union type.
Same for lines 522, 646 and 647.

* Unless the mode is located in thirdparty/CodeMirror2/mode/<name>/<name>.js, you need to first load it yourself.
*
* @return {$.Promise} A promise object that will be resolved when the mode is loaded and set
Expand Down Expand Up @@ -519,7 +519,7 @@ define(function (require, exports, module) {
/**
* Sets the prefixes to use for line comments in this language or prints an error to the console.
* @param {!string|Array.<string>} prefix Prefix string or an array of prefix strings
* to use for line comments (i.e. "//" or ["//", "#"])
* to use for line comments (e.g. "//" or ["//", "#"])
* @return {boolean} Whether the syntax was valid and set or not
*/
Language.prototype.setLineCommentSyntax = function (prefix) {
Expand Down Expand Up @@ -637,13 +637,14 @@ define(function (require, exports, module) {
/**
* Defines a language.
*
* @param {!string} id Unique identifier for this language, use only letters a-z or digits 0-9, and _ inbetween (i.e. "cpp", "foo_bar", "c99")
* @param {!string} id Unique identifier for this language: lowercase letters and digits with _ separators (e.g. "cpp", "foo_bar", "c99")
* @param {!Object} definition An object describing the language
* @param {!string} definition.name Human-readable name of the language, as it's commonly referred to (i.e. "C++")
* @param {Array.<string>} definition.fileExtensions List of file extensions used by this language (i.e. ["php", "php3"])
* @param {Array.<string>} definition.blockComment Array with two entries defining the block comment prefix and suffix (i.e. ["<!--", "-->"])
* @param {string|Array.<string>} definition.lineComment Line comment prefixes (i.e. "//" or ["//", "#"])
* @param {string|Array.<string>} definition.mode CodeMirror mode (i.e. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"]
* @param {!string} definition.name Human-readable name of the language, as it's commonly referred to (e.g. "C++")
* @param {Array.<string>} definition.fileExtensions List of file extensions used by this language (e.g. ["php", "php3"]). May contain periods (e.g. ["coffee.md"])
* @param {Array.<string>} definition.fileNames List of file names (e.g. ["Makefile"] or ["package.json])
* @param {Array.<string>} definition.blockComment Array with two entries defining the block comment prefix and suffix (e.g. ["<!--", "-->"])
* @param {string|Array.<string>} definition.lineComment Line comment prefixes (e.g. "//" or ["//", "#"])
* @param {string|Array.<string>} definition.mode CodeMirror mode (e.g. "htmlmixed"), optionally with a MIME mode defined by that mode ["clike", "text/x-c++src"]
* Unless the mode is located in thirdparty/CodeMirror2/mode/<name>/<name>.js, you need to first load it yourself.
*
* @return {$.Promise} A promise object that will be resolved with a Language object
Expand Down
5 changes: 3 additions & 2 deletions src/widgets/Dialogs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -253,13 +253,14 @@ define(function (require, exports, module) {
* Creates a new general purpose modal dialog using the default template and the template variables given
* as parameters as described.
*
* @param {string} dlgClass A class name identifier for the dialog.
* @param {string} dlgClass A class name identifier for the dialog. Typically one of DefaultDialogs.*
* @param {string=} title The title of the dialog. Can contain HTML markup. If unspecified, the title
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you update this 2 comments too, since you updated the others? It was my mistake, but we can fix it here. In both param descriptions it should that it Defaults to an empty string, instead of the JSON stuff.

* in the JSON file is used unchanged.
* @param {string=} message The message to display in the dialog. Can contain HTML markup. If
* unspecified, the message in the JSON file is used.
* @param {Array.<{className: string, id: string, text: string>=} buttons An array of buttons where each button
Copy link
Contributor

Choose a reason for hiding this comment

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

I missed a } here... It should be {Array.<{className: string, id: string, text: string}>=}

* has a class, id and text property. The id is used in "data-button-id". It defaults to an Ok button
* has a class, id and text property. The id is used in "data-button-id". Defaults to a single Ok button.
* Typically className is one of DIALOG_BTN_CLASS_*, id is one of DIALOG_BTN_*
* @return {Dialog}
*/
function showModalDialog(dlgClass, title, message, buttons) {
Expand Down
6 changes: 1 addition & 5 deletions test/spec/CSSUtils-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1411,18 +1411,14 @@ define(function (require, exports, module) {
var testPath = SpecRunnerUtils.getTestPath("/spec/CSSUtils-test-files"),
CSSUtils,
DocumentManager,
FileViewController,
ProjectManager,
brackets;
FileViewController;

beforeEach(function () {
SpecRunnerUtils.createTestWindowAndRun(this, function (testWindow) {
// Load module instances from brackets.test
brackets = testWindow.brackets;
CSSUtils = testWindow.brackets.test.CSSUtils;
DocumentManager = testWindow.brackets.test.DocumentManager;
FileViewController = testWindow.brackets.test.FileViewController;
ProjectManager = testWindow.brackets.test.ProjectManager;

SpecRunnerUtils.loadProjectInTestWindow(testPath);
});
Expand Down
39 changes: 6 additions & 33 deletions test/spec/Editor-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ define(function (require, exports, module) {
}

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

function createTestEditor(content, languageId) {
Expand Down Expand Up @@ -110,42 +110,15 @@ define(function (require, exports, module) {
myEditor._codeMirror.setValue("new content");
expect(changeFired).toBe(true);
});

});

describe("File extension to mode mapping", function () {

it("should switch to the HTML mode for files ending in .html", function () {
// verify editor content
var mode = LanguageManager.getLanguageForPath("c:/only/testing/the/path.html").getMode();
expect(mode).toSpecifyModeNamed("text/x-brackets-html");
});

it("should switch modes for UNIX absolute path", function () {
// verify editor content
var mode = LanguageManager.getLanguageForPath("/only/testing/the/path.css").getMode();
expect(mode).toSpecifyModeNamed(langNames.css.mode);
});

it("should switch modes for relative path", function () {
// verify editor content
it("should set mode based on Document language", function () {
createTestEditor(defaultContent, "html");
var mode = LanguageManager.getLanguageForPath("only/testing/the/path.css").getMode();
expect(mode).toSpecifyModeNamed(langNames.css.mode);
});

it("should accept just a file name too", function () {
// verify editor content
var mode = LanguageManager.getLanguageForPath("path.js").getMode();
expect(mode).toSpecifyModeNamed(langNames.javascript.mode);
});

it("should default to plain text for unknown file extensions", function () {
// verify editor content
var mode = LanguageManager.getLanguageForPath("test.foo").getMode();

// "unknown" mode uses it's MIME type instead
expect(mode).toBe("text/plain");
var htmlLanguage = LanguageManager.getLanguage("html");
expect(myEditor.getModeForDocument()).toBe(htmlLanguage.getMode());
});

Copy link
Member Author

Choose a reason for hiding this comment

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

The tests above were just testing LanguageManager functionality. I simplified & moved those into LanguageManager-test, and in their place added a single test here to make sure the Document's language actually carries through to the Editor's mode (which the old tests never checked).

});

describe("Focus", function () {
Expand Down
39 changes: 37 additions & 2 deletions test/spec/FileIndexManager-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ define(function (require, exports, module) {

});

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

Expand All @@ -263,7 +263,7 @@ define(function (require, exports, module) {
.done(function (result) {
allFiles1 = result;
});
promise2 = FileIndexManager.getFileInfoList("all")
promise2 = FileIndexManager.getFileInfoList("all") // same request again
.done(function (result) {
allFiles2 = result;
});
Expand All @@ -282,6 +282,41 @@ define(function (require, exports, module) {
});
});

it("should handle differing simultaneous requests without doing extra work", function () { // #330
// Open a directory
SpecRunnerUtils.loadProjectInTestWindow(testPath);

var projectRoot;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should join this var statements, and maybe move them to the top of the function too.

var promise1, promise2;
var allFiles1, allFiles2;
runs(function () {
projectRoot = ProjectManager.getProjectRoot();
spyOn(projectRoot, "createReader").andCallThrough();

// Kick off two index requests in parallel
promise1 = FileIndexManager.getFileInfoList("all")
.done(function (result) {
allFiles1 = result;
});
promise2 = FileIndexManager.getFileInfoList("css") // different request in parallel
.done(function (result) {
allFiles2 = result;
});

waitsForDone(promise1, "First FileIndexManager.getFileInfoList()");
waitsForDone(promise2, "Second FileIndexManager.getFileInfoList()");
});

runs(function () {
// Correct response to both promises
expect(allFiles1.length).toEqual(8);
expect(allFiles2.length).toEqual(3);

// Didn't scan project tree twice
expect(projectRoot.createReader.callCount).toBe(1);
});
});

it("should match a specific filename and return the correct FileInfo", function () {
// Open a directory
SpecRunnerUtils.loadProjectInTestWindow(testPath);
Expand Down
Loading