From a42b1e34aa0aa8cc81c36efe22962951ae4cacdf Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Thu, 20 Mar 2014 10:35:13 -0700 Subject: [PATCH 1/3] - Fix & update documentation in a bunch of places - Add a fail-fast warning when missing DOM id will cause bottom panels to not get created correctly (per #6252) - Add extra FileSystem unit tests around the fs root - Fix bits of the ProjectManager exclusion regex that were missing trailing "$" (I verified these are all full names, not just prefixes), and add WebStorm project state as an excluded item - Slightly better messages when unit tests fail due to a Promise resolution going the wrong way - Fix some minor JSLint errors --- src/document/DocumentCommandHandlers.js | 3 -- src/editor/InlineWidget.js | 1 + src/filesystem/Directory.js | 4 ++- src/filesystem/File.js | 2 +- src/filesystem/FileSystem.js | 6 ++++ .../impls/appshell/AppshellFileSystem.js | 28 +++++++++++++++---- src/nls/fa-ir/urls.js | 2 +- src/project/ProjectManager.js | 2 +- src/project/SidebarView.js | 2 +- src/utils/Resizer.js | 7 ++++- src/view/PanelManager.js | 2 +- test/spec/FileSystem-test.js | 10 ++++++- test/spec/SpecRunnerUtils.js | 5 +++- 13 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index 553cf9f8b8f..842e0f9b5a5 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -1560,9 +1560,6 @@ define(function (require, exports, module) { // Register global commands CommandManager.register(Strings.CMD_FILE_OPEN, Commands.FILE_OPEN, handleFileOpen); CommandManager.register(Strings.CMD_ADD_TO_WORKING_SET, Commands.FILE_ADD_TO_WORKING_SET, handleFileAddToWorkingSet); - // TODO: (issue #274) For now, hook up File > New to the "new in project" handler. Eventually - // File > New should open a new blank tab, and handleFileNewInProject should - // be called from a "+" button in the project CommandManager.register(Strings.CMD_FILE_NEW_UNTITLED, Commands.FILE_NEW_UNTITLED, handleFileNew); CommandManager.register(Strings.CMD_FILE_NEW, Commands.FILE_NEW, handleFileNewInProject); CommandManager.register(Strings.CMD_FILE_NEW_FOLDER, Commands.FILE_NEW_FOLDER, handleNewFolderInProject); diff --git a/src/editor/InlineWidget.js b/src/editor/InlineWidget.js index ed122425845..c89f7cba21c 100644 --- a/src/editor/InlineWidget.js +++ b/src/editor/InlineWidget.js @@ -97,6 +97,7 @@ define(function (require, exports, module) { /** * Called once content is parented in the host editor's DOM. Useful for performing tasks like setting * focus or measuring content, which require htmlContent to be in the DOM tree. + * * IMPORTANT: onAdded() MUST be overridden to call hostEditor.setInlineWidgetHeight() at least once to * set the initial height (required to animate it open). The widget will never open otherwise. */ diff --git a/src/filesystem/Directory.js b/src/filesystem/Directory.js index fc657ed0795..e70ef332e99 100644 --- a/src/filesystem/Directory.js +++ b/src/filesystem/Directory.js @@ -127,7 +127,9 @@ define(function (require, exports, module) { } /** - * Read the contents of a Directory. + * Read the contents of a Directory. If this Directory is under a watch root, + * the listing will exclude any items filtered out by the watch root's filter + * function. * * @param {Directory} directory Directory whose contents you want to get * @param {function (?string, Array.=, Array.=, Object.=)} callback diff --git a/src/filesystem/File.js b/src/filesystem/File.js index 6c103b82510..8393e978685 100644 --- a/src/filesystem/File.js +++ b/src/filesystem/File.js @@ -149,7 +149,7 @@ define(function (require, exports, module) { // Request a consistency check if the write is not blind if (!options.blind) { - options.expectedHash = this._hash; + options.expectedHash = this._hash; options.expectedContents = this._contents; } diff --git a/src/filesystem/FileSystem.js b/src/filesystem/FileSystem.js index bd2d6480073..204008df84f 100644 --- a/src/filesystem/FileSystem.js +++ b/src/filesystem/FileSystem.js @@ -52,6 +52,8 @@ * to meet these requirements) * * FileSystem dispatches the following events: + * (NOTE: attach to these events via `FileSystem.on()` - not `$(FileSystem).on()`) + * * change - Sent whenever there is a change in the file system. The handler * is passed up to three arguments: the changed entry and, if that changed entry * is a Directory, a list of entries added to the directory and a list of entries @@ -262,6 +264,8 @@ define(function (require, exports, module) { commandName = shouldWatch ? "watchPath" : "unwatchPath"; if (recursiveWatch) { + // The impl can watch the entire subtree with one call on the root (we also fall into this case for + // unwatch, although that never requires us to do the recursion - see similar final case below) if (entry !== watchedRoot.entry) { // Watch and unwatch calls to children of the watched root are // no-ops if the impl supports recursiveWatch @@ -315,6 +319,8 @@ define(function (require, exports, module) { }); }, callback); } else { + // Unwatching never requires enumerating the subfolders (which is good, since after a + // delete/rename we may be unable to do so anyway) this._enqueueWatchRequest(function (requestCb) { impl.unwatchPath(entry.fullPath, requestCb); }, callback); diff --git a/src/filesystem/impls/appshell/AppshellFileSystem.js b/src/filesystem/impls/appshell/AppshellFileSystem.js index 9dc85aa6802..2bb48aa3845 100644 --- a/src/filesystem/impls/appshell/AppshellFileSystem.js +++ b/src/filesystem/impls/appshell/AppshellFileSystem.js @@ -35,10 +35,27 @@ define(function (require, exports, module) { var FILE_WATCHER_BATCH_TIMEOUT = 200; // 200ms - granularity of file watcher changes - var _changeCallback, // Callback to notify FileSystem of watcher changes - _offlineCallback, // Callback to notify FileSystem that watchers are offline - _changeTimeout, // Timeout used to batch up file watcher changes - _pendingChanges = {}; // Pending file watcher changes + /** + * Callback to notify FileSystem of watcher changes + * @type {?function(string, FileSystemStats=)} + */ + var _changeCallback; + + /** + * Callback to notify FileSystem if watchers stop working entirely + * @type {?function()} + */ + var _offlineCallback; + + /** Timeout used to batch up file watcher changes (setTimeout() return value) */ + var _changeTimeout; + + /** + * Pending file watcher changes - map from fullPath to flag indicating whether we need to pass stats + * to _changeCallback() for this path. + * @type {!Object.} + */ + var _pendingChanges = {}; var _bracketsPath = FileUtils.getNativeBracketsDirectoryPath(), _modulePath = FileUtils.getNativeModuleDirectoryPath(module), @@ -48,6 +65,7 @@ define(function (require, exports, module) { var _isRunningOnWindowsXP = navigator.userAgent.indexOf("Windows NT 5.") >= 0; + // If the connection closes, notify the FileSystem that watchers have gone offline. $(_nodeDomain.connection).on("close", function (event, promise) { if (_offlineCallback) { @@ -105,7 +123,7 @@ define(function (require, exports, module) { if (event === "change") { // Only register change events if filename is passed if (filename) { - // an existing file was created; stats are needed + // an existing file was modified; stats are needed change = path + filename; _enqueueChange(change, true); } diff --git a/src/nls/fa-ir/urls.js b/src/nls/fa-ir/urls.js index 5bd42f56dd3..e833da996b4 100644 --- a/src/nls/fa-ir/urls.js +++ b/src/nls/fa-ir/urls.js @@ -26,5 +26,5 @@ define({ // Relative to the samples folder - "WEB_PLATFORM_DOCS_LICENSE" : "http://creativecommons.org/licenses/by/3.0/deed.fa", + "WEB_PLATFORM_DOCS_LICENSE" : "http://creativecommons.org/licenses/by/3.0/deed.fa" }); \ No newline at end of file diff --git a/src/project/ProjectManager.js b/src/project/ProjectManager.js index 3640652e51c..6e932278e41 100644 --- a/src/project/ProjectManager.js +++ b/src/project/ProjectManager.js @@ -103,7 +103,7 @@ define(function (require, exports, module) { * https://github.com/adobe/brackets/issues/6781 * @type {RegExp} */ - var _exclusionListRegEx = /\.pyc$|^\.git$|^\.gitmodules$|^\.svn$|^\.DS_Store$|^Thumbs\.db$|^\.hg$|^CVS$|^\.hgtags$|^\.c9revisions|^\.SyncArchive|^\.SyncID|^\.SyncIgnore|\~$/; + var _exclusionListRegEx = /\.pyc$|^\.git$|^\.gitmodules$|^\.svn$|^\.DS_Store$|^Thumbs\.db$|^\.hg$|^CVS$|^\.hgtags$|^\.idea$|^\.c9revisions$|^\.SyncArchive$|^\.SyncID$|^\.SyncIgnore$|\~$/; /** * @private diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index 684d437aa6f..578c096656e 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -51,7 +51,7 @@ define(function (require, exports, module) { EditorManager = require("editor/EditorManager"), Global = require("utils/Global"), Resizer = require("utils/Resizer"), - _ = require("thirdparty/lodash"); + _ = require("thirdparty/lodash"); // These vars are initialized by the htmlReady handler // below since they refer to DOM elements diff --git a/src/utils/Resizer.js b/src/utils/Resizer.js index 5e92869fa32..4b68b179e1f 100644 --- a/src/utils/Resizer.js +++ b/src/utils/Resizer.js @@ -130,7 +130,7 @@ define(function (require, exports, module) { * - panelExpanded: When the panel gets expanded (or shown). Passed the initial size. * May occur without any resize events. * - * @param {!DOMNode} element DOM element which should be made resizable. + * @param {!DOMNode} element DOM element which should be made resizable. Must have an id attribute. * @param {!string} direction Direction of the resize action: one of the DIRECTION_* constants. * @param {!string} position Which side of the element can be dragged: one of the POSITION_* constants * (TOP/BOTTOM for vertical resizing or LEFT/RIGHT for horizontal). @@ -159,6 +159,11 @@ define(function (require, exports, module) { resizerCSSPosition = direction === DIRECTION_HORIZONTAL ? "left" : "top", contentSizeFunction = direction === DIRECTION_HORIZONTAL ? $resizableElement.width : $resizableElement.height; + if (!elementID) { + console.error("Resizable panels must have a DOM id to use as a preferences key:", element); + return; + } + if (minSize === undefined) { minSize = DEFAULT_MIN_SIZE; } diff --git a/src/view/PanelManager.js b/src/view/PanelManager.js index fcb27758d0f..3b7b0acef23 100644 --- a/src/view/PanelManager.js +++ b/src/view/PanelManager.js @@ -186,7 +186,7 @@ define(function (require, exports, module) { * Creates a new panel beneath the editor area and above the status bar footer. Panel is initially invisible. * * @param {!string} id Unique id for this panel. Use package-style naming, e.g. "myextension.feature.panelname" - * @param {!jQueryObject} $panel DOM content to use as the panel. Need not be in the document yet. + * @param {!jQueryObject} $panel DOM content to use as the panel. Need not be in the document yet. Must have an id. * @param {number=} minSize Minimum height of panel in px. * @return {!Panel} */ diff --git a/test/spec/FileSystem-test.js b/test/spec/FileSystem-test.js index c17c789cb45..d8511aa0180 100644 --- a/test/spec/FileSystem-test.js +++ b/test/spec/FileSystem-test.js @@ -293,10 +293,12 @@ define(function (require, exports, module) { }); it("should have a parentPath property if it is not a root directory", function () { var file = fileSystem.getFileForPath("/subdir/file3.txt"), - directory = fileSystem.getDirectoryForPath("/subdir/foo/"); + directory = fileSystem.getDirectoryForPath("/subdir/foo/"), + inRoot = fileSystem.getDirectoryForPath("/inRoot.txt"); expect(file.parentPath).toBe("/subdir/"); expect(directory.parentPath).toBe("/subdir/"); + expect(inRoot.parentPath).toBe("/"); }); it("should not have a parentPath property if it is a root directory", function () { var unixRootDir = fileSystem.getDirectoryForPath("/"), @@ -304,6 +306,8 @@ define(function (require, exports, module) { expect(unixRootDir.parentPath).toBeNull(); expect(winRootDir.parentPath).toBeNull(); + expect(unixRootDir.name).toBe(""); + expect(winRootDir.name).toBe("B:"); }); }); @@ -352,6 +356,7 @@ define(function (require, exports, module) { } if (expectedType) { expect(cb.entry instanceof expectedType).toBeTruthy(); + expect(cb.entry.fullPath).toBe(path); } }); } @@ -362,6 +367,9 @@ define(function (require, exports, module) { it("should resolve a Directory", function () { testResolve("/subdir/", null, Directory); }); + it("should resolve the root", function () { + testResolve("/", null, Directory); + }); it("should return an error if the File/Directory is not found", function () { testResolve("/doesnt-exist.txt", FileSystemError.NOT_FOUND); testResolve("/doesnt-exist/", FileSystemError.NOT_FOUND); diff --git a/test/spec/SpecRunnerUtils.js b/test/spec/SpecRunnerUtils.js index ecdc69d1ae8..ce05c2f3ee3 100644 --- a/test/spec/SpecRunnerUtils.js +++ b/test/spec/SpecRunnerUtils.js @@ -170,7 +170,7 @@ define(function (require, exports, module) { timeout = timeout || 1000; expect(promise).toBeTruthy(); promise.fail(function (err) { - expect("[" + operationName + "] promise rejected with: " + err).toBe(null); + expect("[" + operationName + "] promise rejected with: " + err).toBe("(expected resolved instead)"); }); waitsFor(function () { return promise.state() === "resolved"; @@ -187,6 +187,9 @@ define(function (require, exports, module) { window.waitsForFail = function (promise, operationName, timeout) { timeout = timeout || 1000; expect(promise).toBeTruthy(); + promise.done(function (result) { + expect("[" + operationName + "] promise resolved with: " + result).toBe("(expected rejected instead)"); + }); waitsFor(function () { return promise.state() === "rejected"; }, "failure " + operationName, timeout); From 67df79b2d7d7df76e1e573ce6617f69e630e8029 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Mon, 24 Mar 2014 17:52:09 -0700 Subject: [PATCH 2/3] Code review: better description of PanelManager/Resizer's use of DOM ids --- src/utils/Resizer.js | 6 ++++-- src/view/PanelManager.js | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/utils/Resizer.js b/src/utils/Resizer.js index 4b68b179e1f..569259b4941 100644 --- a/src/utils/Resizer.js +++ b/src/utils/Resizer.js @@ -111,7 +111,8 @@ define(function (require, exports, module) { } /** - * Adds resizing capabilities to a given html element. + * Adds resizing and (optionally) expand/collapse capabilities to a given html element. The element's size + * & visibility are automatically saved & restored as a view-state preference. * * Resizing can be configured in two directions: * - Vertical ("vert"): Resizes the height of the element @@ -130,7 +131,8 @@ define(function (require, exports, module) { * - panelExpanded: When the panel gets expanded (or shown). Passed the initial size. * May occur without any resize events. * - * @param {!DOMNode} element DOM element which should be made resizable. Must have an id attribute. + * @param {!DOMNode} element DOM element which should be made resizable. Must have an id attribute, for + * use as a preferences key. * @param {!string} direction Direction of the resize action: one of the DIRECTION_* constants. * @param {!string} position Which side of the element can be dragged: one of the POSITION_* constants * (TOP/BOTTOM for vertical resizing or LEFT/RIGHT for horizontal). diff --git a/src/view/PanelManager.js b/src/view/PanelManager.js index 3b7b0acef23..a40575796c6 100644 --- a/src/view/PanelManager.js +++ b/src/view/PanelManager.js @@ -183,10 +183,12 @@ define(function (require, exports, module) { /** - * Creates a new panel beneath the editor area and above the status bar footer. Panel is initially invisible. + * Creates a new resizabel panel beneath the editor area and above the status bar footer. Panel is initially invisible. + * The panel's size & visibility are automatically saved & restored as a view-state preference. * * @param {!string} id Unique id for this panel. Use package-style naming, e.g. "myextension.feature.panelname" - * @param {!jQueryObject} $panel DOM content to use as the panel. Need not be in the document yet. Must have an id. + * @param {!jQueryObject} $panel DOM content to use as the panel. Need not be in the document yet. Must have an id + * attribute, for use as a preferences key. * @param {number=} minSize Minimum height of panel in px. * @return {!Panel} */ From 7cf848690f0c44a98bccd49c1b94ce1d9dbe53bd Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Mon, 24 Mar 2014 21:53:03 -0700 Subject: [PATCH 3/3] Fix comment typo --- src/view/PanelManager.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/view/PanelManager.js b/src/view/PanelManager.js index a40575796c6..1eb4974af35 100644 --- a/src/view/PanelManager.js +++ b/src/view/PanelManager.js @@ -183,7 +183,7 @@ define(function (require, exports, module) { /** - * Creates a new resizabel panel beneath the editor area and above the status bar footer. Panel is initially invisible. + * Creates a new resizable panel beneath the editor area and above the status bar footer. Panel is initially invisible. * The panel's size & visibility are automatically saved & restored as a view-state preference. * * @param {!string} id Unique id for this panel. Use package-style naming, e.g. "myextension.feature.panelname"