From c1e0aa1622f3534fcb0c45116d9370a9e18a80e1 Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Wed, 23 May 2012 10:53:30 -0700 Subject: [PATCH 01/45] Added requestAnimationFrame calls` --- src/project/SidebarView.js | 85 +++++++++++++++++++++----------------- 1 file changed, 48 insertions(+), 37 deletions(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index 10e35d433..f4796d6c7 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -23,7 +23,7 @@ /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define, $, document */ +/*global define, $, document, window */ define(function (require, exports, module) { 'use strict'; @@ -88,8 +88,8 @@ define(function (require, exports, module) { // event that we can just call from anywhere instead of hard-coding it. // waiting on a ProjectManager refactor to add that. $sidebar.find(".sidebar-selection").width(width); - $projectFilesContainer.triggerHandler("scroll"); - $openFilesContainer.triggerHandler("scroll"); + //$projectFilesContainer.triggerHandler("scroll"); + //$openFilesContainer.triggerHandler("scroll"); if (width > 10) { prefs.setValue("sidebarWidth", width); @@ -100,8 +100,6 @@ define(function (require, exports, module) { var text = (isSidebarClosed) ? "Show Sidebar" : "Hide Sidebar"; $sidebarMenuText.first().text(text); } - - EditorManager.resizeEditor(); } /** @@ -131,7 +129,9 @@ define(function (require, exports, module) { $body = $(document.body), prefs = PreferencesManager.getPreferenceStorage(PREFERENCES_CLIENT_ID, defaultPrefs), sidebarWidth = prefs.getValue("sidebarWidth"), - startingSidebarPosition = sidebarWidth; + startingSidebarPosition = sidebarWidth, + animationRequest = null, + isMouseDown = false; $sidebarResizer.css("left", sidebarWidth - 1); @@ -150,51 +150,62 @@ define(function (require, exports, module) { } }); $sidebarResizer.on("mousedown.sidebar", function (e) { - var startX = e.clientX; + var startX = e.clientX, + newWidth = Math.max(startX, 0), + doResize = true; + + isMouseDown = true; + $body.toggleClass("resizing"); // check to see if we're currently in hidden mode if (isSidebarClosed) { toggleSidebar(1); } - $mainView.on("mousemove.sidebar", function (e) { - var doResize = true, - newWidth = Math.max(e.clientX, 0); - - // if we've gone below 10 pixels on a mouse move, and the - // sidebar is shrinking, hide the sidebar automatically an - // unbind the mouse event. - if ((startX > 10) && (newWidth < 10)) { - toggleSidebar(startingSidebarPosition); - $mainView.off("mousemove.sidebar"); - $body.toggleClass("resizing"); - doResize = false; - } else if (startX < 10) { - // reset startX if we're going from a snapped closed position to open - startX = startingSidebarPosition; - } - - if (doResize) { - // if we've moving past 10 pixels, make the triangle visible again - // and register that the sidebar is no longer snapped closed. - var forceTriangle = null; - - if (newWidth > 10) { - forceTriangle = true; + animationRequest = window.webkitRequestAnimationFrame(function doRedraw() { + if (isMouseDown) { + // if we've gone below 10 pixels on a mouse move, and the + // sidebar is shrinking, hide the sidebar automatically an + // unbind the mouse event. + if ((startX > 10) && (newWidth < 10)) { + toggleSidebar(startingSidebarPosition); + $mainView.off("mousemove.sidebar"); + $body.toggleClass("resizing"); + doResize = false; + } else if (startX < 10) { + // reset startX if we're going from a snapped closed position to open + startX = startingSidebarPosition; } - - _setWidth(newWidth, false, forceTriangle); - } - if (newWidth === 0) { - $mainView.off("mousemove.sidebar"); - $("body").toggleClass("resizing"); + if (doResize) { + // if we've moving past 10 pixels, make the triangle visible again + // and register that the sidebar is no longer snapped closed. + var forceTriangle = null; + + if (newWidth > 10) { + forceTriangle = true; + } + + _setWidth(newWidth, false, true); + } + + if (newWidth === 0) { + $mainView.off("mousemove.sidebar"); + $("body").toggleClass("resizing"); + } + animationRequest = window.webkitRequestAnimationFrame(doRedraw); } + }); + + $mainView.on("mousemove.sidebar", function (e) { + newWidth = Math.max(e.clientX, 0); e.preventDefault(); }); $mainView.one("mouseup.sidebar", function (e) { + isMouseDown = false; + EditorManager.resizeEditor(); $mainView.off("mousemove.sidebar"); $body.toggleClass("resizing"); startingSidebarPosition = $sidebar.width(); From 6c45f1870ff89c571b03e2a6420b30a60e2c96d2 Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Fri, 25 May 2012 10:02:34 -0700 Subject: [PATCH 02/45] Optimized by removing shadows on click so the scroll doesn't need to get called each time --- src/project/SidebarView.js | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index f4796d6c7..d8f9d0935 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -88,8 +88,6 @@ define(function (require, exports, module) { // event that we can just call from anywhere instead of hard-coding it. // waiting on a ProjectManager refactor to add that. $sidebar.find(".sidebar-selection").width(width); - //$projectFilesContainer.triggerHandler("scroll"); - //$openFilesContainer.triggerHandler("scroll"); if (width > 10) { prefs.setValue("sidebarWidth", width); @@ -153,9 +151,12 @@ define(function (require, exports, module) { var startX = e.clientX, newWidth = Math.max(startX, 0), doResize = true; - + isMouseDown = true; + // take away the shadows + $sidebar.find(".scroller-shadow").css("display", "none"); + $body.toggleClass("resizing"); // check to see if we're currently in hidden mode if (isSidebarClosed) { @@ -186,7 +187,7 @@ define(function (require, exports, module) { forceTriangle = true; } - _setWidth(newWidth, false, true); + _setWidth(newWidth, false, false); } if (newWidth === 0) { @@ -205,7 +206,14 @@ define(function (require, exports, module) { $mainView.one("mouseup.sidebar", function (e) { isMouseDown = false; + + // replace shadows and triangle + $sidebar.find(".triangle-visible").css("display", "block"); + $sidebar.find(".scroller-shadow").css("display", "block"); + EditorManager.resizeEditor(); + $projectFilesContainer.triggerHandler("scroll"); + $openFilesContainer.triggerHandler("scroll"); $mainView.off("mousemove.sidebar"); $body.toggleClass("resizing"); startingSidebarPosition = $sidebar.width(); From efae6adf9410926453db985605969146825b668a Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Fri, 25 May 2012 10:20:55 -0700 Subject: [PATCH 03/45] Added some comments to explain changes. --- src/project/SidebarView.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index d8f9d0935..0cab732d2 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -154,7 +154,7 @@ define(function (require, exports, module) { isMouseDown = true; - // take away the shadows + // take away the shadows (for performance reasons during sidebarmovement) $sidebar.find(".scroller-shadow").css("display", "none"); $body.toggleClass("resizing"); @@ -164,6 +164,8 @@ define(function (require, exports, module) { } animationRequest = window.webkitRequestAnimationFrame(function doRedraw() { + // only run this if the mouse is down so we don't constantly loop even + // after we're done resizing. if (isMouseDown) { // if we've gone below 10 pixels on a mouse move, and the // sidebar is shrinking, hide the sidebar automatically an @@ -186,7 +188,9 @@ define(function (require, exports, module) { if (newWidth > 10) { forceTriangle = true; } - + // for right now, displayTriangle is always going to be false for _setWidth + // because we want to hide it when we move, and _setWidth only gets called + // on mousemove now. _setWidth(newWidth, false, false); } From aebd0e74a152ea7af1e7a8340799cf62933df452 Mon Sep 17 00:00:00 2001 From: Christian Cantrell Date: Thu, 31 May 2012 16:25:23 -0400 Subject: [PATCH 04/45] Added VIEW_RESTORE_FONT_SIZE --- src/command/Commands.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/command/Commands.js b/src/command/Commands.js index ddcc1e07e..6f35a338b 100644 --- a/src/command/Commands.js +++ b/src/command/Commands.js @@ -65,6 +65,7 @@ define(function (require, exports, module) { exports.VIEW_HIDE_SIDEBAR = "view.hideSidebar"; exports.VIEW_INCREASE_FONT_SIZE = "view.increaseFontSize"; exports.VIEW_DECREASE_FONT_SIZE = "view.decreaseFontSize"; + exports.VIEW_RESTORE_FONT_SIZE = "view.restoreFontSize"; // Navigate exports.NAVIGATE_NEXT_DOC = "navigate.nextDoc"; From 44fd6e6507603795d1b3ce598f56ca67482c2a82 Mon Sep 17 00:00:00 2001 From: Christian Cantrell Date: Thu, 31 May 2012 16:27:19 -0400 Subject: [PATCH 05/45] Added semi-colons to the end of symbol entities to make them compatible with integers, and added the restore font menu item. --- src/command/Menus.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index ebc167db9..95c206a36 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -137,9 +137,9 @@ define(function (require, exports, module) { var displayStr; if (brackets.platform === "mac") { displayStr = keyCmd.replace(/-/g, ""); // remove dashes - displayStr = displayStr.replace("Ctrl", "⌘"); // Ctrl > command symbol - displayStr = displayStr.replace("Shift", "⇧"); // Shift > shift symbol - displayStr = displayStr.replace("Alt", "⌥"); // Alt > option symbol + displayStr = displayStr.replace("Ctrl", "⌘"); // Ctrl > command symbol + displayStr = displayStr.replace("Shift", "⇧"); // Shift > shift symbol + displayStr = displayStr.replace("Alt", "⌥"); // Alt > option symbol } else { displayStr = keyCmd.replace(/-/g, "+"); } @@ -530,6 +530,7 @@ define(function (require, exports, module) { menu.addMenuDivider(); menu.addMenuItem("menu-view-increase-font", Commands.VIEW_INCREASE_FONT_SIZE, "Ctrl-="); menu.addMenuItem("menu-view-decrease-font", Commands.VIEW_DECREASE_FONT_SIZE, "Ctrl--"); + menu.addMenuItem("menu-view-restore-font", Commands.VIEW_RESTORE_FONT_SIZE, "Ctrl-0"); /* * Navigate menu From b704cffc5fd7db36c189cee91db850d83f6e9170 Mon Sep 17 00:00:00 2001 From: Christian Cantrell Date: Thu, 31 May 2012 16:27:58 -0400 Subject: [PATCH 06/45] Added 'Restore Font Size' --- src/strings.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/strings.js b/src/strings.js index c8bd7d2ff..fa7bc9488 100644 --- a/src/strings.js +++ b/src/strings.js @@ -152,6 +152,7 @@ define(function (require, exports, module) { exports.CMD_SHOW_SIDEBAR = "Show Sidebar"; exports.CMD_INCREASE_FONT_SIZE = "Increase Font Size"; exports.CMD_DECREASE_FONT_SIZE = "Decrease Font Size"; + exports.CMD_RESTORE_FONT_SIZE = "Restore Font Size"; // Navigate menu Commands exports.NAVIGATE_MENU = "Navigate"; From 6beb221c0fffaf39102f74756a414c85ae910827 Mon Sep 17 00:00:00 2001 From: Christian Cantrell Date: Thu, 31 May 2012 16:29:17 -0400 Subject: [PATCH 07/45] Added functionality to restore the font size to its original setting. --- src/view/ViewCommandHandlers.js | 27 +++++++++++++++++++++------ 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/src/view/ViewCommandHandlers.js b/src/view/ViewCommandHandlers.js index 158369a9d..ed5152743 100644 --- a/src/view/ViewCommandHandlers.js +++ b/src/view/ViewCommandHandlers.js @@ -32,15 +32,26 @@ define(function (require, exports, module) { Strings = require("strings"), ProjectManager = require("project/ProjectManager"), EditorManager = require("editor/EditorManager"); - + + /** + * @const + * @type {string} + */ + var DYNAMIC_FONT_STYLE_ID = "codemirror-dynamic-fonts"; + + function _removeDynamicFontSize(refresh) { + $("#" + DYNAMIC_FONT_STYLE_ID).remove(); + if (refresh) { + EditorManager.getCurrentFullEditor().refreshAll(); + } + } + /** * @private * Increases or decreases the editor's font size. * @param {number} -1 to make the font smaller; 1 to make it bigger. */ function _adjustFontSize(direction) { - var styleId = "codemirror-dynamic-fonts"; - var fs = $(".CodeMirror-scroll").css("font-size"); var lh = $(".CodeMirror-scroll").css("line-height"); @@ -76,8 +87,8 @@ define(function (require, exports, module) { } // It's necessary to inject a new rule to address all editors. - $("#" + styleId).remove(); - var style = $("").attr("id", styleId); + _removeDynamicFontSize(false); + var style = $("").attr("id", DYNAMIC_FONT_STYLE_ID); style.html(".CodeMirror-scroll {" + "font-size: " + fsStr + " !important;" + "line-height: " + lhStr + " !important;}"); @@ -85,7 +96,7 @@ define(function (require, exports, module) { EditorManager.getCurrentFullEditor().refreshAll(); } - + function _handleIncreaseFontSize() { _adjustFontSize(1); } @@ -94,7 +105,11 @@ define(function (require, exports, module) { _adjustFontSize(-1); } + function _handleRestoreFontSize() { + _removeDynamicFontSize(true); + } CommandManager.register(Strings.CMD_INCREASE_FONT_SIZE, Commands.VIEW_INCREASE_FONT_SIZE, _handleIncreaseFontSize); CommandManager.register(Strings.CMD_DECREASE_FONT_SIZE, Commands.VIEW_DECREASE_FONT_SIZE, _handleDecreaseFontSize); + CommandManager.register(Strings.CMD_RESTORE_FONT_SIZE, Commands.VIEW_RESTORE_FONT_SIZE, _handleRestoreFontSize); }); From 932be6f77e2a1b44eeea23bfd08782cc0dca5354 Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Sat, 2 Jun 2012 10:38:10 -0700 Subject: [PATCH 08/45] Checking in some optimization changes --- src/project/SidebarView.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index 0cab732d2..edab8fe35 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -98,6 +98,7 @@ define(function (require, exports, module) { var text = (isSidebarClosed) ? "Show Sidebar" : "Hide Sidebar"; $sidebarMenuText.first().text(text); } + EditorManager.resizeEditor(); } /** @@ -151,7 +152,7 @@ define(function (require, exports, module) { var startX = e.clientX, newWidth = Math.max(startX, 0), doResize = true; - + isMouseDown = true; // take away the shadows (for performance reasons during sidebarmovement) @@ -164,6 +165,7 @@ define(function (require, exports, module) { } animationRequest = window.webkitRequestAnimationFrame(function doRedraw() { + console.log('rAF'); // only run this if the mouse is down so we don't constantly loop even // after we're done resizing. if (isMouseDown) { @@ -171,10 +173,16 @@ define(function (require, exports, module) { // sidebar is shrinking, hide the sidebar automatically an // unbind the mouse event. if ((startX > 10) && (newWidth < 10)) { + console.log('beginning startX: ' + startX); toggleSidebar(startingSidebarPosition); $mainView.off("mousemove.sidebar"); $body.toggleClass("resizing"); doResize = false; + startX = 0; + console.log('ending startX: ' + startX); + window.webkitCancelRequestAnimationFrame(this); + return; + } else if (startX < 10) { // reset startX if we're going from a snapped closed position to open startX = startingSidebarPosition; @@ -215,7 +223,6 @@ define(function (require, exports, module) { $sidebar.find(".triangle-visible").css("display", "block"); $sidebar.find(".scroller-shadow").css("display", "block"); - EditorManager.resizeEditor(); $projectFilesContainer.triggerHandler("scroll"); $openFilesContainer.triggerHandler("scroll"); $mainView.off("mousemove.sidebar"); From c6cee9ca23f28225f95ea3d43a8552da08f35ba8 Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Sat, 2 Jun 2012 15:25:31 -0700 Subject: [PATCH 09/45] Fixed blinking bug and also bug where selector folder would leave an artifact while dragging --- src/project/SidebarView.js | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index 1f519857e..40e4490ff 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -75,7 +75,7 @@ define(function (require, exports, module) { if (typeof displayTriangle === "boolean") { var display = (displayTriangle) ? "block" : "none"; - $sidebar.find(".triangle-visible").css("display", display); + $sidebar.find(".sidebar-selection-triangle").css("display", display); } if (isSidebarClosed) { @@ -151,7 +151,7 @@ define(function (require, exports, module) { }); $sidebarResizer.on("mousedown.sidebar", function (e) { var startX = e.clientX, - newWidth = Math.max(startX, 0), + newWidth = Math.max(e.clientX, 0), doResize = true; isMouseDown = true; @@ -160,13 +160,14 @@ define(function (require, exports, module) { $sidebar.find(".scroller-shadow").css("display", "none"); $body.toggleClass("resizing"); + // check to see if we're currently in hidden mode if (isSidebarClosed) { toggleSidebar(1); - } + } + animationRequest = window.webkitRequestAnimationFrame(function doRedraw() { - console.log('rAF'); // only run this if the mouse is down so we don't constantly loop even // after we're done resizing. if (isMouseDown) { @@ -174,19 +175,20 @@ define(function (require, exports, module) { // sidebar is shrinking, hide the sidebar automatically an // unbind the mouse event. if ((startX > 10) && (newWidth < 10)) { - console.log('beginning startX: ' + startX); toggleSidebar(startingSidebarPosition); $mainView.off("mousemove.sidebar"); + + // turn off the mouseup event so that it doesn't fire twice and retoggle the + // resizing class + $mainView.off("mouseup.sidebar"); $body.toggleClass("resizing"); doResize = false; startX = 0; - console.log('ending startX: ' + startX); - window.webkitCancelRequestAnimationFrame(this); - return; - } else if (startX < 10) { - // reset startX if we're going from a snapped closed position to open - startX = startingSidebarPosition; + // force isMouseDown so that we don't keep calling requestAnimationFrame + // this keeps the sidebar from stuttering + isMouseDown = false; + } if (doResize) { @@ -204,8 +206,9 @@ define(function (require, exports, module) { } if (newWidth === 0) { - $mainView.off("mousemove.sidebar"); - $("body").toggleClass("resizing"); + console.log('newWidth === 0'); + //$mainView.off("mousemove.sidebar"); + //$("body").toggleClass("resizing"); } animationRequest = window.webkitRequestAnimationFrame(doRedraw); } @@ -213,7 +216,7 @@ define(function (require, exports, module) { $mainView.on("mousemove.sidebar", function (e) { newWidth = Math.max(e.clientX, 0); - + e.preventDefault(); }); @@ -221,7 +224,7 @@ define(function (require, exports, module) { isMouseDown = false; // replace shadows and triangle - $sidebar.find(".triangle-visible").css("display", "block"); + $sidebar.find(".sidebar-selection-triangle").css("display", "block"); $sidebar.find(".scroller-shadow").css("display", "block"); $projectFilesContainer.triggerHandler("scroll"); From dd925572e7a2b13fb47c12e51c361ef7b8b20cc5 Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Sat, 2 Jun 2012 15:48:25 -0700 Subject: [PATCH 10/45] Removed console.log statment --- src/project/SidebarView.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index 40e4490ff..b3e8249bd 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -206,9 +206,8 @@ define(function (require, exports, module) { } if (newWidth === 0) { - console.log('newWidth === 0'); - //$mainView.off("mousemove.sidebar"); - //$("body").toggleClass("resizing"); + $mainView.off("mousemove.sidebar"); + $("body").toggleClass("resizing"); } animationRequest = window.webkitRequestAnimationFrame(doRedraw); } From b400fee33972f933d1c14c5592869ed99c810149 Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Sat, 2 Jun 2012 15:50:50 -0700 Subject: [PATCH 11/45] Forgot to check JSLint - made small fix --- src/project/SidebarView.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index b3e8249bd..1a47abea2 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -164,7 +164,7 @@ define(function (require, exports, module) { // check to see if we're currently in hidden mode if (isSidebarClosed) { toggleSidebar(1); - } + } animationRequest = window.webkitRequestAnimationFrame(function doRedraw() { From 4b48a120fb79871e1ee49bc1e11ae3f63cd87130 Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Sat, 2 Jun 2012 15:48:25 -0700 Subject: [PATCH 12/45] Removed console.log statment Forgot to check JSLint - made small fix --- src/project/SidebarView.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index 40e4490ff..1a47abea2 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -164,7 +164,7 @@ define(function (require, exports, module) { // check to see if we're currently in hidden mode if (isSidebarClosed) { toggleSidebar(1); - } + } animationRequest = window.webkitRequestAnimationFrame(function doRedraw() { @@ -206,9 +206,8 @@ define(function (require, exports, module) { } if (newWidth === 0) { - console.log('newWidth === 0'); - //$mainView.off("mousemove.sidebar"); - //$("body").toggleClass("resizing"); + $mainView.off("mousemove.sidebar"); + $("body").toggleClass("resizing"); } animationRequest = window.webkitRequestAnimationFrame(doRedraw); } From c9574ffeffbb8d875643258d150f4439ae3159cb Mon Sep 17 00:00:00 2001 From: Joel Brandt Date: Wed, 6 Jun 2012 20:58:43 -0700 Subject: [PATCH 13/45] Modified ExtensionLoader to allow for retrieving the require context that loaded an extension --- src/utils/ExtensionLoader.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/utils/ExtensionLoader.js b/src/utils/ExtensionLoader.js index 64041f70c..87a75f96a 100644 --- a/src/utils/ExtensionLoader.js +++ b/src/utils/ExtensionLoader.js @@ -34,7 +34,19 @@ define(function (require, exports, module) { var NativeFileSystem = require("file/NativeFileSystem").NativeFileSystem, FileUtils = require("file/FileUtils"), - Async = require("utils/Async"); + Async = require("utils/Async"), + contexts = {}; + /** + * Returns the require.js require context used to load an extension + * + * @param {!string} name, used to identify the extension + * @return {!Object} A require.js require object used to load the extension, or undefined if + * there is no require object ith that name + */ + function getRequireContextForExtension(name) { + return contexts[name]; + } + /** * Loads the extension that lives at baseUrl into its own Require.js context @@ -50,6 +62,7 @@ define(function (require, exports, module) { context: name, baseUrl: baseUrl }); + contexts[name] = extensionRequire; console.log("[Extension] starting to load " + baseUrl); @@ -173,6 +186,7 @@ define(function (require, exports, module) { return _loadAll(directory, baseUrl, "unittests", testExtension); } + exports.getRequireContextForExtension = getRequireContextForExtension; exports.loadExtension = loadExtension; exports.testExtension = testExtension; exports.loadAllExtensionsInNativeDirectory = loadAllExtensionsInNativeDirectory; From c304c4bcf6c08ea16b6858e9ddb33247f351338f Mon Sep 17 00:00:00 2001 From: Joel Brandt Date: Wed, 6 Jun 2012 20:59:45 -0700 Subject: [PATCH 14/45] Rewrote unit tests for JavaScriptInlineEditor to get JSUtils from the require context that was used to load the extension. We only need to do this in unit tests where we rely on JSUtils to get the DocumentManager, etc. from the current window. Tests that JUST test JSUtils functionality (independent of Brackets) can load it with the standard require context. --- .../JavaScriptInlineEditor/unittests.js | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/extensions/disabled/JavaScriptInlineEditor/unittests.js b/src/extensions/disabled/JavaScriptInlineEditor/unittests.js index e48b9cc58..4951b77c1 100644 --- a/src/extensions/disabled/JavaScriptInlineEditor/unittests.js +++ b/src/extensions/disabled/JavaScriptInlineEditor/unittests.js @@ -44,7 +44,7 @@ define(function (require, exports, module) { var extensionPath = FileUtils.getNativeModuleDirectoryPath(module); - describe("JSQuickEdit", function () { + describe("JavaScriptInlineEditor", function () { var testPath = extensionPath + "/unittest-files", testWindow, @@ -495,7 +495,6 @@ define(function (require, exports, module) { SpecRunnerUtils.closeTestWindow(); }); -/*** it("should return the correct offsets if the file has changed", function () { var didOpen = false, gotError = false; @@ -531,8 +530,11 @@ define(function (require, exports, module) { FileIndexManager.getFileInfoList("all") .done(function (fileInfos) { + var extensionRequire = brackets.getModule('utils/ExtensionLoader').getRequireContextForExtension('JavaScriptInlineEditor'); + var JSUtilsInExtension = extensionRequire("JSUtils"); + // Look for "edit2" function - JSUtils.findMatchingFunctions("edit2", fileInfos) + JSUtilsInExtension.findMatchingFunctions("edit2", fileInfos) .done(function (result) { functions = result; }); }); }); @@ -545,8 +547,7 @@ define(function (require, exports, module) { expect(functions[0].lineEnd).toBe(13); }); }); -***/ -/*** + it("should return a newly created function in an unsaved file", function () { var didOpen = false, gotError = false; @@ -563,24 +564,31 @@ define(function (require, exports, module) { runs(function () { var doc = DocumentManager.getCurrentDocument(); - // Add a new function to the file doc.setText(doc.getText() + "\n\nfunction TESTFUNCTION() {\n return true;\n}\n"); // Look for the selector we just created - JSUtils.findMatchingFunctions("TESTFUNCTION", FileIndexManager.getFileInfoList("all")) - .done(function (result) { functions = result; }); + FileIndexManager.getFileInfoList("all") + .done(function (fileInfos) { + var extensionRequire = brackets.getModule('utils/ExtensionLoader').getRequireContextForExtension('JavaScriptInlineEditor'); + var JSUtilsInExtension = extensionRequire("JSUtils"); + + // Look for "TESTFUNCTION" function + JSUtilsInExtension.findMatchingFunctions("TESTFUNCTION", fileInfos) + .done(function (result) { + functions = result; + }); + }); }); waitsFor(function () { return functions !== null; }, "JSUtils.findMatchingFunctions() timeout", 1000); runs(function () { expect(functions.length).toBe(1); - expect(functions[0].lineStart).toBe(24); - expect(functions[0].lineEnd).toBe(26); + expect(functions[0].lineStart).toBe(33); + expect(functions[0].lineEnd).toBe(35); }); }); -***/ }); }); //describe("JS Parsing") }); From 664fea636fbab0207b8aa0759df69e4aa806ea4a Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Thu, 7 Jun 2012 09:29:35 -0700 Subject: [PATCH 15/45] always scroll selected doc into view in working set --- src/project/ProjectManager.js | 24 ++------- src/project/WorkingSetView.js | 94 ++++++++++++++++++++++------------- src/utils/ViewUtils.js | 50 +++++++++++++++++-- 3 files changed, 108 insertions(+), 60 deletions(-) diff --git a/src/project/ProjectManager.js b/src/project/ProjectManager.js index 73c45b740..a78afa0cb 100644 --- a/src/project/ProjectManager.js +++ b/src/project/ProjectManager.js @@ -835,11 +835,7 @@ define(function (require, exports, module) { _projectTree.jstree("create", node, position, {data: initialName}, null, skipRename); if (!skipRename) { - var $renameInput = _projectTree.find(".jstree-rename-input"), - projectTreeOffset = _projectTree.offset(), - projectTreeScroller = _projectTree.get(0), - renameInput = $renameInput.get(0), - renameInputOffset = $renameInput.offset(); + var $renameInput = _projectTree.find(".jstree-rename-input"); $renameInput.on("keydown", function (event) { // Listen for escape key on keydown, so we can remove the node in the create.jstree handler above @@ -847,22 +843,8 @@ define(function (require, exports, module) { escapeKeyPressed = true; } }); - - // make sure edit box is visible within the jstree, only scroll vertically when necessary - if (renameInputOffset.top + $renameInput.height() >= (projectTreeOffset.top + _projectTree.height())) { - // below viewport - renameInput.scrollIntoView(false); - } else if (renameInputOffset.top <= projectTreeOffset.top) { - // above viewport - renameInput.scrollIntoView(true); - } - - // left-align renameInput - if (renameInputOffset.left < 0) { - _projectTree.scrollLeft(_projectTree.scrollLeft() + renameInputOffset.left); - } else if (renameInputOffset.left + $renameInput.width() >= projectTreeOffset.left + _projectTree.width()) { - _projectTree.scrollLeft(renameInputOffset.left - projectTreeOffset.left); - } + + ViewUtils.scrollElementIntoView(_projectTree, $renameInput, true); } return result.promise(); diff --git a/src/project/WorkingSetView.js b/src/project/WorkingSetView.js index b10ddc9cf..29a1af7dd 100644 --- a/src/project/WorkingSetView.js +++ b/src/project/WorkingSetView.js @@ -180,42 +180,8 @@ define(function (require, exports, module) { _redraw(); } - - /** - * @private - */ - function _updateListSelection() { - var doc; - if (FileViewController.getFileSelectionFocus() === FileViewController.WORKING_SET_VIEW) { - doc = DocumentManager.getCurrentDocument(); - } else { - doc = null; - } - - // Iterate through working set list and update the selection on each - var items = $openFilesContainer.find("ul").children().each(function () { - _updateListItemSelection(this, doc); - }); - - _fireSelectionChanged(); - } - /** - * @private - */ - function _handleFileAdded(file) { - _createNewListItem(file); - _redraw(); - } - - /** - * @private - */ - function _handleDocumentSelectionChange() { - _updateListSelection(); - } - - /** + /** * Finds the listItem item assocated with the file. Returns null if not found. * @private * @param {!FileEntry} file @@ -239,6 +205,64 @@ define(function (require, exports, module) { return result; } + /** + * @private + */ + function _scrollSelectedDocIntoView() { + if (FileViewController.getFileSelectionFocus() !== FileViewController.WORKING_SET_VIEW) { + return; + } + + var doc = DocumentManager.getCurrentDocument(); + if (!doc) { + return; + } + + var $selectedDoc = _findListItemFromFile(doc.file); + if (!$selectedDoc) { + return; + } + + ViewUtils.scrollElementIntoView($openFilesContainer, $selectedDoc, false); + } + + /** + * @private + */ + function _updateListSelection() { + var doc; + if (FileViewController.getFileSelectionFocus() === FileViewController.WORKING_SET_VIEW) { + doc = DocumentManager.getCurrentDocument(); + } else { + doc = null; + } + + // Iterate through working set list and update the selection on each + var items = $openFilesContainer.find("ul").children().each(function () { + _updateListItemSelection(this, doc); + }); + + // Make sure selection is in view + _scrollSelectedDocIntoView(); + + _fireSelectionChanged(); + } + + /** + * @private + */ + function _handleFileAdded(file) { + _createNewListItem(file); + _redraw(); + } + + /** + * @private + */ + function _handleDocumentSelectionChange() { + _updateListSelection(); + } + /** * @private * @param {FileEntry} file diff --git a/src/utils/ViewUtils.js b/src/utils/ViewUtils.js index 8b95861b0..bb8c1611c 100644 --- a/src/utils/ViewUtils.js +++ b/src/utils/ViewUtils.js @@ -277,13 +277,55 @@ define(function (require, exports, module) { f.apply(); }); } + + /** + * Within a scrolling DOMElement, if necessary, scroll element into viewport. + * + * To Perform the minimum amount of scrolling necessary, cases should be handled as follows: + * - element already completely in view : no scrolling + * - element above viewport : scroll view so element is at top + * - element left of viewport : scroll view so element is at left + * - element below viewport : scroll view so element is at bottom + * - element right of viewport : scroll view so element is at right + * + * Assumptions: + * - $view is a scrolling container + * + * @param {!DOMElement} $view - A jQuery scrolling container + * @param {!DOMElement} $element - A jQuery element + * @param {?boolean} scrollHorizontal - whether to also scroll horizonally + */ + function scrollElementIntoView($view, $element, scrollHorizontal) { + var viewOffset = $view.offset(), + viewScroller = $view.get(0), + element = $element.get(0), + elementOffset = $element.offset(); + + // scroll minimum amount + if (elementOffset.top + $element.height() >= (viewOffset.top + $view.height())) { + // below viewport + element.scrollIntoView(false); + } else if (elementOffset.top <= viewOffset.top) { + // above viewport + element.scrollIntoView(true); + } + + if (scrollHorizontal) { + if (elementOffset.left < 0) { + $view.scrollLeft($view.scrollLeft() + elementOffset.left); + } else if (elementOffset.left + $element.width() >= viewOffset.left + $view.width()) { + $view.scrollLeft(elementOffset.left - viewOffset.left); + } + } + } // handle all resize handlers in a single listener $(window).resize(_handleResize); // Define public API - exports.SCROLL_SHADOW_HEIGHT = SCROLL_SHADOW_HEIGHT; - exports.addScrollerShadow = addScrollerShadow; - exports.removeScrollerShadow = removeScrollerShadow; - exports.sidebarList = sidebarList; + exports.SCROLL_SHADOW_HEIGHT = SCROLL_SHADOW_HEIGHT; + exports.addScrollerShadow = addScrollerShadow; + exports.removeScrollerShadow = removeScrollerShadow; + exports.sidebarList = sidebarList; + exports.scrollElementIntoView = scrollElementIntoView; }); From afa8c660a02d71052519b0ec558ca038978d43c1 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Thu, 7 Jun 2012 10:41:01 -0700 Subject: [PATCH 16/45] Fix extension loader baseUrl --- src/utils/ExtensionLoader.js | 41 ++++++++++++++++++++++---------- test/SpecRunner.js | 8 +++++-- test/perf/Performance-test.js | 4 ++-- test/perf/PerformanceReporter.js | 2 +- 4 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/utils/ExtensionLoader.js b/src/utils/ExtensionLoader.js index 64041f70c..e24990579 100644 --- a/src/utils/ExtensionLoader.js +++ b/src/utils/ExtensionLoader.js @@ -44,17 +44,17 @@ define(function (require, exports, module) { * @param {!string} entryPoint, name of the main js file to load * @return {!$.Promise} A promise object that is resolved when the extension is loaded. */ - function loadExtension(name, baseUrl, entryPoint) { + function loadExtension(name, config, entryPoint) { var result = new $.Deferred(), extensionRequire = brackets.libRequire.config({ context: name, - baseUrl: baseUrl + baseUrl: config.baseUrl }); - console.log("[Extension] starting to load " + baseUrl); + console.log("[Extension] starting to load " + config.baseUrl); extensionRequire([entryPoint], function () { - console.log("[Extension] finished loading " + baseUrl); + console.log("[Extension] finished loading " + config.baseUrl); result.resolve(); }); @@ -69,13 +69,13 @@ define(function (require, exports, module) { * @param {!string} entryPoint, name of the main js file to load * @return {!$.Promise} A promise object that is resolved when all extensions complete loading. */ - function testExtension(name, baseUrl, entryPoint) { + function testExtension(name, config, entryPoint) { var result = new $.Deferred(), extensionPath = FileUtils.getNativeBracketsDirectoryPath(); // Assumes the caller's window.location context is /test/SpecRunner.html extensionPath = extensionPath.replace("brackets/test", "brackets/src"); // convert from "test" to "src" - extensionPath += "/" + baseUrl + "/" + entryPoint + ".js"; + extensionPath += "/" + config.baseUrl + "/" + entryPoint + ".js"; var fileExists = false, statComplete = false; brackets.fs.stat(extensionPath, function (err, stat) { @@ -84,12 +84,13 @@ define(function (require, exports, module) { // unit test file exists var extensionRequire = brackets.libRequire.config({ context: name, - baseUrl: "../src/" + baseUrl + baseUrl: "../src/" + config.baseUrl, + paths: config.paths }); - console.log("[Extension] loading unit test " + baseUrl); + console.log("[Extension] loading unit test " + config.baseUrl); extensionRequire([entryPoint], function () { - console.log("[Extension] loaded unit tests " + baseUrl); + console.log("[Extension] loaded unit tests " + config.baseUrl); result.resolve(); }); } else { @@ -111,7 +112,7 @@ define(function (require, exports, module) { * @param {function} processExtension * @return {!$.Promise} A promise object that is resolved when all extensions complete loading. */ - function _loadAll(directory, baseUrl, entryPoint, processExtension) { + function _loadAll(directory, config, entryPoint, processExtension) { var result = new $.Deferred(); NativeFileSystem.requestNativeFileSystem(directory, @@ -130,7 +131,11 @@ define(function (require, exports, module) { } Async.doInParallel(extensions, function (item) { - return processExtension(item, baseUrl + "/" + item, entryPoint); + var extConfig = { + baseUrl: config.baseUrl + "/" + item, + paths: config.paths + }; + return processExtension(item, extConfig, entryPoint); }).done(function () { result.resolve(); }).fail(function () { @@ -158,7 +163,7 @@ define(function (require, exports, module) { * @return {!$.Promise} A promise object that is resolved when all extensions complete loading. */ function loadAllExtensionsInNativeDirectory(directory, baseUrl) { - return _loadAll(directory, baseUrl, "main", loadExtension); + return _loadAll(directory, {baseUrl: baseUrl}, "main", loadExtension); } /** @@ -170,7 +175,17 @@ define(function (require, exports, module) { * @return {!$.Promise} A promise object that is resolved when all extensions complete loading. */ function testAllExtensionsInNativeDirectory(directory, baseUrl) { - return _loadAll(directory, baseUrl, "unittests", testExtension); + var bracketsPath = FileUtils.getNativeBracketsDirectoryPath(), + config = { + baseUrl: baseUrl + }; + + config.paths = { + "perf": bracketsPath + "/perf", + "spec": bracketsPath + "/spec" + }; + + return _loadAll(directory, config, "unittests", testExtension); } exports.loadExtension = loadExtension; diff --git a/test/SpecRunner.js b/test/SpecRunner.js index 0c0ffcedb..2c750ae52 100644 --- a/test/SpecRunner.js +++ b/test/SpecRunner.js @@ -26,7 +26,11 @@ // Set the baseUrl to brackets/src require.config({ - baseUrl: "../src"/*, + baseUrl: "../src", + paths: { + "perf": "../test/perf", + "spec": "../test/spec" + }/*, urlArgs: "bust=" + (new Date()).getTime() // cache busting */ }); @@ -38,7 +42,7 @@ define(function (require, exports, module) { ExtensionLoader = require("utils/ExtensionLoader"), FileUtils = require("file/FileUtils"), Menus = require("command/Menus"), - PerformanceReporter = require("perf/PerformanceReporter.js").PerformanceReporter; + PerformanceReporter = require("perf/PerformanceReporter").PerformanceReporter; var suite; diff --git a/test/perf/Performance-test.js b/test/perf/Performance-test.js index 204f62b13..ddf696528 100644 --- a/test/perf/Performance-test.js +++ b/test/perf/Performance-test.js @@ -36,8 +36,8 @@ define(function (require, exports, module) { PerfUtils, // loaded from brackets.test JSLintUtils, // loaded from brackets.test DocumentManager, // loaded from brackets.test - SpecRunnerUtils = require("../spec/SpecRunnerUtils.js"), - PerformanceReporter = require("../perf/PerformanceReporter.js"); + SpecRunnerUtils = require("spec/SpecRunnerUtils"), + PerformanceReporter = require("perf/PerformanceReporter"); var jsLintPrevSetting; diff --git a/test/perf/PerformanceReporter.js b/test/perf/PerformanceReporter.js index eda681c6d..d18610959 100644 --- a/test/perf/PerformanceReporter.js +++ b/test/perf/PerformanceReporter.js @@ -27,7 +27,7 @@ define(function (require, exports, module) { 'use strict'; - var SpecRunnerUtils = require("spec/SpecRunnerUtils.js"); + var SpecRunnerUtils = require("spec/SpecRunnerUtils"); var records = {}; From 3c7b82340846c0a0c60861e34ce26412ce3922b5 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Fri, 8 Jun 2012 09:03:16 -0700 Subject: [PATCH 17/45] loadStyleSheet proposal for extensions --- src/utils/ExtensionLoader.js | 37 ++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/src/utils/ExtensionLoader.js b/src/utils/ExtensionLoader.js index 64041f70c..0f9ac9918 100644 --- a/src/utils/ExtensionLoader.js +++ b/src/utils/ExtensionLoader.js @@ -173,8 +173,45 @@ define(function (require, exports, module) { return _loadAll(directory, baseUrl, "unittests", testExtension); } + /** + * Loads a style sheet relative to the extension module. + * + * @param {!module} module Module provided by RequireJS + * @param {!string} path Relative path from the extension folder to a CSS file + * @return {!$.Promise} A promise object that is resolved when the CSS file and it's dependencies are loaded + */ + function loadStyleSheet(module, path) { + var url = module.uri.replace("main.js", "") + path, + $link = $(""), + onload, + onerror, + result = new $.Deferred(); + + if ($link[0].onload && $link[0].onerror) { + onload = function () { result.resolve(); }; + onerror = function () { result.reject(); }; + } + + $link.attr({ + type: "text/css", + rel: "stylesheet", + href: url, + onload: onload, + onerror: onerror + }); + + $("head").append($link); + + if (!onload) { + result.resolve(); + } + + return result; + } + exports.loadExtension = loadExtension; exports.testExtension = testExtension; exports.loadAllExtensionsInNativeDirectory = loadAllExtensionsInNativeDirectory; exports.testAllExtensionsInNativeDirectory = testAllExtensionsInNativeDirectory; + exports.loadStyleSheet = loadStyleSheet; }); From 32dd9b6279ea36f49bfabbe50bc184d0a5c693d1 Mon Sep 17 00:00:00 2001 From: Ty Voliter Date: Fri, 8 Jun 2012 11:35:19 -0700 Subject: [PATCH 18/45] * moved format() and offsetToLineNum() to StringUtils --- src/LiveDevelopment/LiveDevelopment.js | 2 +- src/document/DocumentCommandHandlers.js | 4 +- src/file/FileUtils.js | 4 +- src/project/FileSyncManager.js | 6 +-- src/project/ProjectManager.js | 12 ++--- src/strings.js | 22 -------- src/utils/StringUtils.js | 71 ++++++++++++++++++++++++- 7 files changed, 83 insertions(+), 38 deletions(-) diff --git a/src/LiveDevelopment/LiveDevelopment.js b/src/LiveDevelopment/LiveDevelopment.js index a26b37d91..508e03b70 100644 --- a/src/LiveDevelopment/LiveDevelopment.js +++ b/src/LiveDevelopment/LiveDevelopment.js @@ -354,7 +354,7 @@ define(function LiveDevelopment(require, exports, module) { if (err === FileError.NOT_FOUND_ERR) { message = Strings.ERROR_CANT_FIND_CHROME; } else { - message = Strings.format(Strings.ERROR_LAUNCHING_BROWSER, err); + message = StringUtils.format(Strings.ERROR_LAUNCHING_BROWSER, err); } Dialogs.showModalDialog( diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index ba2e9731c..b9a5dc752 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -324,7 +324,7 @@ define(function (require, exports, module) { return Dialogs.showModalDialog( Dialogs.DIALOG_ID_ERROR, Strings.ERROR_SAVING_FILE_TITLE, - Strings.format( + StringUtils.format( Strings.ERROR_SAVING_FILE, StringUtils.htmlEscape(path), FileUtils.getFileErrorString(code) @@ -506,7 +506,7 @@ define(function (require, exports, module) { Dialogs.showModalDialog( Dialogs.DIALOG_ID_SAVE_CLOSE, Strings.SAVE_CLOSE_TITLE, - Strings.format(Strings.SAVE_CLOSE_MESSAGE, StringUtils.htmlEscape(filename)) + StringUtils.format(Strings.SAVE_CLOSE_MESSAGE, StringUtils.htmlEscape(filename)) ).done(function (id) { if (id === Dialogs.DIALOG_BTN_CANCEL) { result.reject(); diff --git a/src/file/FileUtils.js b/src/file/FileUtils.js index ba28c0df9..38fc8a6d7 100644 --- a/src/file/FileUtils.js +++ b/src/file/FileUtils.js @@ -166,7 +166,7 @@ define(function (require, exports, module) { } else if (code === FileError.NO_MODIFICATION_ALLOWED_ERR) { result = Strings.NO_MODIFICATION_ALLOWED_ERR_FILE; } else { - result = Strings.format(Strings.GENERIC_ERROR, code); + result = StringUtils.format(Strings.GENERIC_ERROR, code); } return result; @@ -176,7 +176,7 @@ define(function (require, exports, module) { return Dialogs.showModalDialog( Dialogs.DIALOG_ID_ERROR, Strings.ERROR_OPENING_FILE_TITLE, - Strings.format( + StringUtils.format( Strings.ERROR_OPENING_FILE, StringUtils.htmlEscape(path), getFileErrorString(code) diff --git a/src/project/FileSyncManager.js b/src/project/FileSyncManager.js index 0fce603f8..4ad00ca28 100644 --- a/src/project/FileSyncManager.js +++ b/src/project/FileSyncManager.js @@ -212,7 +212,7 @@ define(function (require, exports, module) { return Dialogs.showModalDialog( Dialogs.DIALOG_ID_ERROR, Strings.ERROR_RELOADING_FILE_TITLE, - Strings.format( + StringUtils.format( Strings.ERROR_RELOADING_FILE, StringUtils.htmlEscape(doc.file.fullPath), FileUtils.getFileErrorString(error.code) @@ -261,7 +261,7 @@ define(function (require, exports, module) { if (i < editConflicts.length) { toClose = false; dialogId = Dialogs.DIALOG_ID_EXT_CHANGED; - message = Strings.format( + message = StringUtils.format( Strings.EXT_MODIFIED_MESSAGE, StringUtils.htmlEscape(ProjectManager.makeProjectRelativeIfPossible(doc.file.fullPath)) ); @@ -269,7 +269,7 @@ define(function (require, exports, module) { } else { toClose = true; dialogId = Dialogs.DIALOG_ID_EXT_DELETED; - message = Strings.format( + message = StringUtils.format( Strings.EXT_DELETED_MESSAGE, StringUtils.htmlEscape(ProjectManager.makeProjectRelativeIfPossible(doc.file.fullPath)) ); diff --git a/src/project/ProjectManager.js b/src/project/ProjectManager.js index 725565c68..dd39ff0a3 100644 --- a/src/project/ProjectManager.js +++ b/src/project/ProjectManager.js @@ -540,7 +540,7 @@ define(function (require, exports, module) { Dialogs.showModalDialog( Dialogs.DIALOG_ID_ERROR, Strings.ERROR_LOADING_PROJECT, - Strings.format(Strings.READ_DIRECTORY_ENTRIES_ERROR, + StringUtils.format(Strings.READ_DIRECTORY_ENTRIES_ERROR, StringUtils.htmlEscape(dirEntry.fullPath), error.code) ); @@ -637,7 +637,7 @@ define(function (require, exports, module) { Dialogs.showModalDialog( Dialogs.DIALOG_ID_ERROR, Strings.ERROR_LOADING_PROJECT, - Strings.format( + StringUtils.format( Strings.REQUEST_NATIVE_FILE_SYSTEM_ERROR, StringUtils.htmlEscape(rootPath), error.code, @@ -685,7 +685,7 @@ define(function (require, exports, module) { Dialogs.showModalDialog( Dialogs.DIALOG_ID_ERROR, Strings.ERROR_LOADING_PROJECT, - Strings.format(Strings.OPEN_DIALOG_ERROR, error.code) + StringUtils.format(Strings.OPEN_DIALOG_ERROR, error.code) ); } ); @@ -812,15 +812,15 @@ define(function (require, exports, module) { Dialogs.showModalDialog( Dialogs.DIALOG_ID_ERROR, Strings.INVALID_FILENAME_TITLE, - Strings.format(Strings.FILE_ALREADY_EXISTS, + StringUtils.format(Strings.FILE_ALREADY_EXISTS, StringUtils.htmlEscape(data.rslt.name)) ); } else { var errString = error.code === FileError.NO_MODIFICATION_ALLOWED_ERR ? Strings.NO_MODIFICATION_ALLOWED_ERR : - Strings.format(String.GENERIC_ERROR, error.code); + StringUtils.format(String.GENERIC_ERROR, error.code); - var errMsg = Strings.format(Strings.ERROR_CREATING_FILE, + var errMsg = StringUtils.format(Strings.ERROR_CREATING_FILE, StringUtils.htmlEscape(data.rslt.name), errString); diff --git a/src/strings.js b/src/strings.js index 12190f8d7..91f047782 100644 --- a/src/strings.js +++ b/src/strings.js @@ -27,28 +27,6 @@ define(function (require, exports, module) { 'use strict'; - - /** - * Format a string by replacing placeholder symbols with passed in arguments. - * - * Example: var formatted = Strings.format("Hello {0}", "World"); - * - * @param {string} str The base string - * @param {...} Arguments to be substituted into the string - * - * @return {string} Formatted string - */ - function format(str) { - // arguments[0] is the base string, so we need to adjust index values here - var args = [].slice.call(arguments, 1); - return str.replace(/\{(\d+)\}/g, function (match, num) { - return typeof args[num] !== 'undefined' ? args[num] : match; - }); - } - - - // Define public API - exports.format = format; // General file io error strings exports.GENERIC_ERROR = "(error {0})"; diff --git a/src/utils/StringUtils.js b/src/utils/StringUtils.js index 09d2d4e94..900778fa1 100644 --- a/src/utils/StringUtils.js +++ b/src/utils/StringUtils.js @@ -31,6 +31,24 @@ define(function (require, exports, module) { 'use strict'; + /** + * Format a string by replacing placeholder symbols with passed in arguments. + * + * Example: var formatted = StringUtils.format("Hello {0}", "World"); + * + * @param {string} str The base string + * @param {...} Arguments to be substituted into the string + * + * @return {string} Formatted string + */ + function format(str) { + // arguments[0] is the base string, so we need to adjust index values here + var args = [].slice.call(arguments, 1); + return str.replace(/\{(\d+)\}/g, function (match, num) { + return typeof args[num] !== 'undefined' ? args[num] : match; + }); + } + function htmlEscape(str) { return String(str) .replace(/&/g, '&') @@ -44,6 +62,55 @@ define(function (require, exports, module) { return str.replace(/([.?*+\^$\[\]\\(){}|\-])/g, "\\$1"); } - exports.htmlEscape = htmlEscape; - exports.regexEscape = regexEscape; + /** + * Splits the text by new line characters and returns an array of lines + * @param {string} text + * @return {Array.} lines + */ + function getLines(text) { + return text.split("\n"); + } + + /** + * Returns a line number corresponding to an offset in some text. This version + * is optimized for repeatidly calling the function on the same text in a loop. + * Use getLines() to divide the text into lines onces, then repeatidly call + * this function to compute a line number from the offset. + * + * @param {Array.} lines - an array of strings corresponding to the + * lines of text in a string. + * @param {number} offset + * @return {number} line number + */ + function offsetToLineNumForLoops(lines, offset) { + var line, total = 0; + for (line = 0; line < lines.length; line++) { + if (total < offset) { + total += lines[line].length + 1; + } else { + return line - 1; + } + } + + return undefined; + } + + /** + * Returns the line number corresponding to an offset in text + * + * Note: When repeatidly computing line numbers for offsets in the same text + * use offsetToLineNumForLoops() instead which is performance optimized for loops + * + * @param {string} text + * @param {number} offset + * @return {number} line number + */ + function _offsetToLineNum(text, offset) { + return text.substr(0, offset).split("\n").length - 1; + } + + // Define public API + exports.format = format; + exports.htmlEscape = htmlEscape; + exports.regexEscape = regexEscape; }); \ No newline at end of file From 00015ec950869532b6f96a6d7a897b3e29319ce4 Mon Sep 17 00:00:00 2001 From: Ty Voliter Date: Fri, 8 Jun 2012 13:20:29 -0700 Subject: [PATCH 19/45] offset fix --- src/utils/StringUtils.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/utils/StringUtils.js b/src/utils/StringUtils.js index 900778fa1..c2c688abe 100644 --- a/src/utils/StringUtils.js +++ b/src/utils/StringUtils.js @@ -87,8 +87,10 @@ define(function (require, exports, module) { for (line = 0; line < lines.length; line++) { if (total < offset) { total += lines[line].length + 1; + console.log( lines[line] + "\b" + total ); } else { - return line - 1; + return line -1; + console.log( "returned " + line ); } } @@ -105,7 +107,7 @@ define(function (require, exports, module) { * @param {number} offset * @return {number} line number */ - function _offsetToLineNum(text, offset) { + function offsetToLineNum(text, offset) { return text.substr(0, offset).split("\n").length - 1; } @@ -113,4 +115,6 @@ define(function (require, exports, module) { exports.format = format; exports.htmlEscape = htmlEscape; exports.regexEscape = regexEscape; + exports.offsetToLineNum = offsetToLineNum; + exports.offsetToLineNumForLoops = offsetToLineNumForLoops; }); \ No newline at end of file From a3aa502da602789381171af68356277aa1bf916c Mon Sep 17 00:00:00 2001 From: Ty Voliter Date: Fri, 8 Jun 2012 13:49:29 -0700 Subject: [PATCH 20/45] Fixing offset to line converstion. Convert FindInFiles to use StringUtils --- src/search/FindInFiles.js | 15 ++++----------- src/utils/StringUtils.js | 9 +++++---- 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/src/search/FindInFiles.js b/src/search/FindInFiles.js index 544a0df67..eeac09721 100644 --- a/src/search/FindInFiles.js +++ b/src/search/FindInFiles.js @@ -36,7 +36,6 @@ * - Search files in working set that are *not* in the project * - Handle matches that span mulitple lines * - Refactor UI from functionality to enable unit testing - * - Cache result of getLine() */ @@ -47,6 +46,7 @@ define(function (require, exports, module) { CommandManager = require("command/CommandManager"), Commands = require("command/Commands"), Strings = require("strings"), + StringUtils = require("utils/StringUtils"), DocumentManager = require("document/DocumentManager"), EditorManager = require("editor/EditorManager"), FileIndexManager = require("project/FileIndexManager"); @@ -138,19 +138,12 @@ define(function (require, exports, module) { var matchStart; var matches = []; - function getLineNum(offset) { - return contents.substr(0, offset).split("\n").length - 1; // 0 based linenum - } - - function getLine(lineNum) { - // Future: cache result - return contents.split("\n")[lineNum]; - } var match; + var lines = StringUtils.getLines(contents); while ((match = queryExpr.exec(contents)) !== null) { - var lineNum = getLineNum(match.index); - var line = getLine(lineNum); + var lineNum = StringUtils.offsetToLineNumForLoops(lines,match.index); + var line = lines[lineNum]; var ch = match.index - contents.lastIndexOf("\n", match.index) - 1; // 0-based index var matchLength = match[0].length; diff --git a/src/utils/StringUtils.js b/src/utils/StringUtils.js index c2c688abe..71a725316 100644 --- a/src/utils/StringUtils.js +++ b/src/utils/StringUtils.js @@ -85,12 +85,12 @@ define(function (require, exports, module) { function offsetToLineNumForLoops(lines, offset) { var line, total = 0; for (line = 0; line < lines.length; line++) { - if (total < offset) { - total += lines[line].length + 1; - console.log( lines[line] + "\b" + total ); + if (total <= offset) { + // add 1 per line since /n were removed by splitting, but they needed to + // contribute to the total offset count + total += lines[line].length + 1; } else { return line -1; - console.log( "returned " + line ); } } @@ -115,6 +115,7 @@ define(function (require, exports, module) { exports.format = format; exports.htmlEscape = htmlEscape; exports.regexEscape = regexEscape; + exports.getLines = getLines; exports.offsetToLineNum = offsetToLineNum; exports.offsetToLineNumForLoops = offsetToLineNumForLoops; }); \ No newline at end of file From ac9dea4eb5da8b9a9896efb92baf5dfd2acb5233 Mon Sep 17 00:00:00 2001 From: Ty Voliter Date: Fri, 8 Jun 2012 13:59:38 -0700 Subject: [PATCH 21/45] Make JSUtils use optimized way to convert offset to lines --- .../JavaScriptInlineEditor/JSUtils.js | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/extensions/disabled/JavaScriptInlineEditor/JSUtils.js b/src/extensions/disabled/JavaScriptInlineEditor/JSUtils.js index 78a3ddbe0..2995c3032 100644 --- a/src/extensions/disabled/JavaScriptInlineEditor/JSUtils.js +++ b/src/extensions/disabled/JavaScriptInlineEditor/JSUtils.js @@ -34,6 +34,7 @@ define(function (require, exports, module) { // Load brackets modules var Async = brackets.getModule("utils/Async"), DocumentManager = brackets.getModule("document/DocumentManager"), + StringUtils = brackets.getModule("utils/StringUtils"), NativeFileSystem = brackets.getModule("file/NativeFileSystem").NativeFileSystem; // Return an Array with names and offsets for all functions in the specified text @@ -94,9 +95,6 @@ define(function (require, exports, module) { return length; } - function _offsetToLineNum(text, offset) { - return text.substr(0, offset).split("\n").length - 1; - } // Search function list for a specific function. If found, extract info about the function // (line start, line end, etc.) and return. @@ -111,6 +109,7 @@ define(function (require, exports, module) { DocumentManager.getDocumentForPath(fileInfo.fullPath) .done(function (doc) { var text = doc.getText(); + var lines = StringUtils.getLines(text); functions.forEach(function (funcEntry) { if (funcEntry.functionName === functionName) { @@ -118,8 +117,8 @@ define(function (require, exports, module) { matchingFunctions.push({ document: doc, name: funcEntry.functionName, - lineStart: _offsetToLineNum(text, funcEntry.offset), - lineEnd: _offsetToLineNum(text, endOffset) + lineStart: StringUtils.offsetToLineNumForLoops(lines, funcEntry.offset), + lineEnd: StringUtils.offsetToLineNumForLoops(lines, endOffset) }); } }); @@ -272,21 +271,22 @@ define(function (require, exports, module) { * @return {Array.<{offset:number, functionName:string}>} * Array of objects containing the start offset for each matched function name. */ - function _findAllMatchingFunctionsInText(text, functionName) { + function _findAllMatchingFunctionsInText(text, functionName) { var allFunctions = _findAllFunctionsInText(text); var result = []; - + var lines = text.split("\n"); + allFunctions.forEach(function (funcEntry) { - if (funcEntry.functionName === functionName) { + if (funcEntry.functionName === functionName || functionName === "*") { var endOffset = _getFunctionEndOffset(text, funcEntry.offset); result.push({ name: funcEntry.functionName, - lineStart: _offsetToLineNum(text, funcEntry.offset), - lineEnd: _offsetToLineNum(text, endOffset) + lineStart: StringUtils.offsetToLineNumForLoops(lines, funcEntry.offset), + lineEnd: StringUtils.offsetToLineNumForLoops(lines, endOffset) }); } }); - + return result; } From fd4fbeac71c29d2762dc453e201b412311a2f437 Mon Sep 17 00:00:00 2001 From: Ty Voliter Date: Fri, 8 Jun 2012 13:59:46 -0700 Subject: [PATCH 22/45] JSLint cleanup --- src/LiveDevelopment/LiveDevelopment.js | 1 + src/search/FindInFiles.js | 2 +- src/utils/StringUtils.js | 4 ++-- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/LiveDevelopment/LiveDevelopment.js b/src/LiveDevelopment/LiveDevelopment.js index 508e03b70..c094d833e 100644 --- a/src/LiveDevelopment/LiveDevelopment.js +++ b/src/LiveDevelopment/LiveDevelopment.js @@ -58,6 +58,7 @@ define(function LiveDevelopment(require, exports, module) { var NativeApp = require("utils/NativeApp"); var Dialogs = require("widgets/Dialogs"); var Strings = require("strings"); + var StringUtils = require("utils/StringUtils"); // Inspector var Inspector = require("LiveDevelopment/Inspector/Inspector"); diff --git a/src/search/FindInFiles.js b/src/search/FindInFiles.js index eeac09721..2d4d9d8af 100644 --- a/src/search/FindInFiles.js +++ b/src/search/FindInFiles.js @@ -142,7 +142,7 @@ define(function (require, exports, module) { var match; var lines = StringUtils.getLines(contents); while ((match = queryExpr.exec(contents)) !== null) { - var lineNum = StringUtils.offsetToLineNumForLoops(lines,match.index); + var lineNum = StringUtils.offsetToLineNumForLoops(lines, match.index); var line = lines[lineNum]; var ch = match.index - contents.lastIndexOf("\n", match.index) - 1; // 0-based index var matchLength = match[0].length; diff --git a/src/utils/StringUtils.js b/src/utils/StringUtils.js index 71a725316..a580ed03c 100644 --- a/src/utils/StringUtils.js +++ b/src/utils/StringUtils.js @@ -88,9 +88,9 @@ define(function (require, exports, module) { if (total <= offset) { // add 1 per line since /n were removed by splitting, but they needed to // contribute to the total offset count - total += lines[line].length + 1; + total += lines[line].length + 1; } else { - return line -1; + return line - 1; } } From baa95aa63f80cb541a3e2a4896d4257407ce8dd9 Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Fri, 8 Jun 2012 15:12:02 -0700 Subject: [PATCH 23/45] Code review fixes. --- src/project/SidebarView.js | 79 +++++++++++++++++--------------------- 1 file changed, 36 insertions(+), 43 deletions(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index 1a47abea2..f8f28a3a8 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -116,7 +116,6 @@ define(function (require, exports, module) { var prefs = PreferencesManager.getPreferenceStorage(PREFERENCES_CLIENT_ID, defaultPrefs); prefs.setValue("sidebarClosed", isSidebarClosed); - _setWidth(width, true, !isSidebarClosed); } @@ -146,7 +145,7 @@ define(function (require, exports, module) { // mousedown is fired first. Sidebar is already toggeled open to 1px. _setWidth(null, true, true); } else { - toggleSidebar(); + toggleSidebar(sidebarWidth); } }); $sidebarResizer.on("mousedown.sidebar", function (e) { @@ -155,7 +154,7 @@ define(function (require, exports, module) { doResize = true; isMouseDown = true; - + // take away the shadows (for performance reasons during sidebarmovement) $sidebar.find(".scroller-shadow").css("display", "none"); @@ -170,47 +169,41 @@ define(function (require, exports, module) { animationRequest = window.webkitRequestAnimationFrame(function doRedraw() { // only run this if the mouse is down so we don't constantly loop even // after we're done resizing. - if (isMouseDown) { - // if we've gone below 10 pixels on a mouse move, and the - // sidebar is shrinking, hide the sidebar automatically an - // unbind the mouse event. - if ((startX > 10) && (newWidth < 10)) { - toggleSidebar(startingSidebarPosition); - $mainView.off("mousemove.sidebar"); - - // turn off the mouseup event so that it doesn't fire twice and retoggle the - // resizing class - $mainView.off("mouseup.sidebar"); - $body.toggleClass("resizing"); - doResize = false; - startX = 0; - - // force isMouseDown so that we don't keep calling requestAnimationFrame - // this keeps the sidebar from stuttering - isMouseDown = false; - - } - - if (doResize) { - // if we've moving past 10 pixels, make the triangle visible again - // and register that the sidebar is no longer snapped closed. - var forceTriangle = null; - - if (newWidth > 10) { - forceTriangle = true; - } - // for right now, displayTriangle is always going to be false for _setWidth - // because we want to hide it when we move, and _setWidth only gets called - // on mousemove now. - _setWidth(newWidth, false, false); - } - - if (newWidth === 0) { - $mainView.off("mousemove.sidebar"); - $("body").toggleClass("resizing"); - } - animationRequest = window.webkitRequestAnimationFrame(doRedraw); + if (!isMouseDown) { + //$mainView.off("mouseup.sidebar"); + //$mainView.off("mousemove.sidebar"); + //$("body").toggleClass("resizing"); + return; } + + // if we've gone below 10 pixels on a mouse move, and the + // sidebar is shrinking, hide the sidebar automatically an + // unbind the mouse event. + if ((startX > 10) && (newWidth < 10)) { + toggleSidebar(startingSidebarPosition); + $mainView.off("mousemove.sidebar"); + + // turn off the mouseup event so that it doesn't fire twice and retoggle the + // resizing class + $mainView.off("mouseup.sidebar"); + $body.toggleClass("resizing"); + doResize = false; + startX = 0; + + // force isMouseDown so that we don't keep calling requestAnimationFrame + // this keeps the sidebar from stuttering + isMouseDown = false; + + } + + if (doResize) { + // for right now, displayTriangle is always going to be false for _setWidth + // because we want to hide it when we move, and _setWidth only gets called + // on mousemove now. + _setWidth(newWidth, false, false); + } + + animationRequest = window.webkitRequestAnimationFrame(doRedraw); }); $mainView.on("mousemove.sidebar", function (e) { From 001ebb2f88cc439df619675a0ed559c5c2bca520 Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Fri, 8 Jun 2012 15:15:05 -0700 Subject: [PATCH 24/45] Removed unused comments --- src/project/SidebarView.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index f8f28a3a8..049132d15 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -170,9 +170,6 @@ define(function (require, exports, module) { // only run this if the mouse is down so we don't constantly loop even // after we're done resizing. if (!isMouseDown) { - //$mainView.off("mouseup.sidebar"); - //$mainView.off("mousemove.sidebar"); - //$("body").toggleClass("resizing"); return; } From 6edf864a1da62e07549176bcfa4b738c8beab41a Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Fri, 8 Jun 2012 15:31:09 -0700 Subject: [PATCH 25/45] Fix for double-click not working --- src/project/SidebarView.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index 049132d15..9d5247d86 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -141,8 +141,8 @@ define(function (require, exports, module) { } $sidebarResizer.on("dblclick", function () { - if ($sidebar.width() === 1) { - // mousedown is fired first. Sidebar is already toggeled open to 1px. + if ($sidebar.width() < 10) { + //mousedown is fired first. Sidebar is already toggeled open to 1px. _setWidth(null, true, true); } else { toggleSidebar(sidebarWidth); From ffcef2e3f274c3720bf684d0718e4c02e2b52664 Mon Sep 17 00:00:00 2001 From: Ryan Stewart Date: Fri, 8 Jun 2012 15:38:18 -0700 Subject: [PATCH 26/45] Fixed bug where triangle wasn't updating correctly by toggling "scroll" events on double click show --- src/project/SidebarView.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/project/SidebarView.js b/src/project/SidebarView.js index 9d5247d86..40a00692f 100644 --- a/src/project/SidebarView.js +++ b/src/project/SidebarView.js @@ -142,8 +142,10 @@ define(function (require, exports, module) { $sidebarResizer.on("dblclick", function () { if ($sidebar.width() < 10) { - //mousedown is fired first. Sidebar is already toggeled open to 1px. + //mousedown is fired first. Sidebar is already toggeled open to at least 10px. _setWidth(null, true, true); + $projectFilesContainer.triggerHandler("scroll"); + $openFilesContainer.triggerHandler("scroll"); } else { toggleSidebar(sidebarWidth); } From 0a8ba9232834ad1d1a9ef08686e8565e1bb676ec Mon Sep 17 00:00:00 2001 From: = Date: Fri, 8 Jun 2012 22:35:51 -0400 Subject: [PATCH 27/45] Changed click handler to use $.on so runtime added menus work --- src/command/Menus.js | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/src/command/Menus.js b/src/command/Menus.js index 2e684a7df..4c5c61675 100644 --- a/src/command/Menus.js +++ b/src/command/Menus.js @@ -23,7 +23,7 @@ /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */ -/*global define, $, brackets */ +/*global define, $, brackets, document */ define(function (require, exports, module) { 'use strict'; @@ -622,23 +622,23 @@ define(function (require, exports, module) { menu.addMenuItem("menu-debug-new-window", Commands.DEBUG_NEW_BRACKETS_WINDOW); menu.addMenuItem("menu-debug-close-browser", Commands.DEBUG_CLOSE_ALL_LIVE_BROWSERS); menu.addMenuItem("menu-debug-use-tab-chars", Commands.DEBUG_USE_TAB_CHARS); - - $("#main-toolbar .dropdown") - // Prevent clicks on the top-level menu bar from taking focus - // Note, bootstrap handles this already for the menu drop downs - .mousedown(function (e) { - e.preventDefault(); - }) - // Switch menus when the mouse enters an adjacent menu - // Only open the menu if another one has already been opened - // by clicking - .mouseenter(function (e) { - var open = $(this).siblings(".open"); - if (open.length > 0) { - open.removeClass("open"); - $(this).addClass("open"); - } - }); + + // Prevent clicks on the top-level menu bar from taking focus + // Note, bootstrap handles this already for the menu drop downs + $(document).on("mousedown", "#main-toolbar .dropdown", function (e) { + e.preventDefault(); + }); + + // Switch menus when the mouse enters an adjacent menu + // Only open the menu if another one has already been opened + // by clicking + $(document).on("mouseenter", "#main-toolbar .dropdown", function (e) { + var open = $(this).siblings(".open"); + if (open.length > 0) { + open.removeClass("open"); + $(this).addClass("open"); + } + }); } // Define public API From d9bfe0877fd26d509ee7a34429033e2904cf1c30 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Mon, 11 Jun 2012 11:15:08 -0700 Subject: [PATCH 28/45] provide way for unit tests to restore commands after a reset --- src/command/CommandManager.js | 35 ++++++++++++++++++++++++-------- test/spec/CommandManager-test.js | 3 ++- 2 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/command/CommandManager.js b/src/command/CommandManager.js index 56e1125ec..e8526692e 100644 --- a/src/command/CommandManager.js +++ b/src/command/CommandManager.js @@ -37,7 +37,13 @@ define(function (require, exports, module) { * @type Object. */ var _commands = {}; - + + /** + * Map of all registered global commands + * @type Object. + */ + var _commandsOriginal = {}; + /** * @constructor * @private @@ -174,11 +180,23 @@ define(function (require, exports, module) { return command; } - - function _reset() { + /** + * Clear all commands for unit testing, but first make copy of commands so that + * they can be restored afterward + */ + function _testReset() { + _commandsOriginal = _commands; _commands = {}; } + /** + * Restore original commands after test and release copy + */ + function _testRestore(commands) { + _commands = _commandsOriginal; + _commandsOriginal = {}; + } + /** * Retrieves a Command object by id * @param {string} id @@ -204,8 +222,9 @@ define(function (require, exports, module) { } // Define public API - exports.register = register; - exports.execute = execute; - exports.get = get; - exports._reset = _reset; -}); \ No newline at end of file + exports.register = register; + exports.execute = execute; + exports.get = get; + exports._testReset = _testReset; + exports._testRestore = _testRestore; +}); diff --git a/test/spec/CommandManager-test.js b/test/spec/CommandManager-test.js index 4ae6c6a00..fb9354b43 100644 --- a/test/spec/CommandManager-test.js +++ b/test/spec/CommandManager-test.js @@ -40,10 +40,11 @@ define(function (require, exports, module) { beforeEach(function () { executed = false; - CommandManager._reset(); + CommandManager._testReset(); }); afterEach(function () { + CommandManager._testRestore(); }); it("register and get a command and validate parameters", function () { From 1b6e7de53a94d93d95c44f652d84f016c1685233 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Mon, 11 Jun 2012 11:20:06 -0700 Subject: [PATCH 29/45] make red bolder --- test/BootstrapReporter.css | 10 ++++++++-- test/SpecRunner.html | 2 +- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/test/BootstrapReporter.css b/test/BootstrapReporter.css index 0a907ed31..1e3dc3897 100644 --- a/test/BootstrapReporter.css +++ b/test/BootstrapReporter.css @@ -1,5 +1,6 @@ body { - padding-top: 60px; /* 60px to make the container go all the way to the bottom of the topbar */ + /* make the container go all the way to the bottom of the topbar */ + padding-top: 60px; } pre { overflow-x: scroll; @@ -8,4 +9,9 @@ pre { } .nav .badge { float: right; -} \ No newline at end of file +} + +/* Make fail (red) color bolder so easier to see */ +.label-important, .badge-important { + background-color: #F00; +} diff --git a/test/SpecRunner.html b/test/SpecRunner.html index c65a43ea8..29a71f6fb 100644 --- a/test/SpecRunner.html +++ b/test/SpecRunner.html @@ -27,9 +27,9 @@ Jasmine Spec Runner - + From 6ec78fe956dc1250799eaac6d2b08f0b7e28a6f4 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Mon, 11 Jun 2012 13:09:06 -0700 Subject: [PATCH 30/45] Refactor loadStyleSheet. Change error handling. --- src/utils/ExtensionLoader.js | 37 -------------------- src/utils/ExtensionUtils.js | 67 ++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 37 deletions(-) create mode 100644 src/utils/ExtensionUtils.js diff --git a/src/utils/ExtensionLoader.js b/src/utils/ExtensionLoader.js index 0f9ac9918..64041f70c 100644 --- a/src/utils/ExtensionLoader.js +++ b/src/utils/ExtensionLoader.js @@ -173,45 +173,8 @@ define(function (require, exports, module) { return _loadAll(directory, baseUrl, "unittests", testExtension); } - /** - * Loads a style sheet relative to the extension module. - * - * @param {!module} module Module provided by RequireJS - * @param {!string} path Relative path from the extension folder to a CSS file - * @return {!$.Promise} A promise object that is resolved when the CSS file and it's dependencies are loaded - */ - function loadStyleSheet(module, path) { - var url = module.uri.replace("main.js", "") + path, - $link = $(""), - onload, - onerror, - result = new $.Deferred(); - - if ($link[0].onload && $link[0].onerror) { - onload = function () { result.resolve(); }; - onerror = function () { result.reject(); }; - } - - $link.attr({ - type: "text/css", - rel: "stylesheet", - href: url, - onload: onload, - onerror: onerror - }); - - $("head").append($link); - - if (!onload) { - result.resolve(); - } - - return result; - } - exports.loadExtension = loadExtension; exports.testExtension = testExtension; exports.loadAllExtensionsInNativeDirectory = loadAllExtensionsInNativeDirectory; exports.testAllExtensionsInNativeDirectory = testAllExtensionsInNativeDirectory; - exports.loadStyleSheet = loadStyleSheet; }); diff --git a/src/utils/ExtensionUtils.js b/src/utils/ExtensionUtils.js new file mode 100644 index 000000000..063818ece --- /dev/null +++ b/src/utils/ExtensionUtils.js @@ -0,0 +1,67 @@ +/* + * 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 + * 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, + * 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 + * DEALINGS IN THE SOFTWARE. + * + */ + + +/*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50 */ +/*global define, $ */ + +/** + * ExtensionUtils defines utility methods for implementing extensions. + */ +define(function (require, exports, module) { + 'use strict'; + + /** + * Loads a style sheet relative to the extension module. + * + * @param {!module} module Module provided by RequireJS + * @param {!string} path Relative path from the extension folder to a CSS file + * @return {!$.Promise} A promise object that is resolved if the CSS file can be loaded. + */ + function loadStyleSheet(module, path) { + var url = encodeURI(module.uri.replace("main.js", "") + path), + result = new $.Deferred(); + + // Make a request for the same file in order to record success or failure. + // The link element's onload and onerror events are not consistently supported. + $.get(url).done(function () { + var $link = $(""); + + $link.attr({ + type: "text/css", + rel: "stylesheet", + href: url + }); + + $("head").append($link[0]); + + result.resolve(); + }).fail(function (err) { + result.reject(err); + }); + + return result; + } + + exports.loadStyleSheet = loadStyleSheet; +}); From 5063b6be008a2ddc9d758393865b7d69e783182f Mon Sep 17 00:00:00 2001 From: Joel Brandt Date: Wed, 6 Jun 2012 20:58:43 -0700 Subject: [PATCH 31/45] Modified ExtensionLoader to allow for retrieving the require context that loaded an extension --- src/utils/ExtensionLoader.js | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/utils/ExtensionLoader.js b/src/utils/ExtensionLoader.js index 64041f70c..87a75f96a 100644 --- a/src/utils/ExtensionLoader.js +++ b/src/utils/ExtensionLoader.js @@ -34,7 +34,19 @@ define(function (require, exports, module) { var NativeFileSystem = require("file/NativeFileSystem").NativeFileSystem, FileUtils = require("file/FileUtils"), - Async = require("utils/Async"); + Async = require("utils/Async"), + contexts = {}; + /** + * Returns the require.js require context used to load an extension + * + * @param {!string} name, used to identify the extension + * @return {!Object} A require.js require object used to load the extension, or undefined if + * there is no require object ith that name + */ + function getRequireContextForExtension(name) { + return contexts[name]; + } + /** * Loads the extension that lives at baseUrl into its own Require.js context @@ -50,6 +62,7 @@ define(function (require, exports, module) { context: name, baseUrl: baseUrl }); + contexts[name] = extensionRequire; console.log("[Extension] starting to load " + baseUrl); @@ -173,6 +186,7 @@ define(function (require, exports, module) { return _loadAll(directory, baseUrl, "unittests", testExtension); } + exports.getRequireContextForExtension = getRequireContextForExtension; exports.loadExtension = loadExtension; exports.testExtension = testExtension; exports.loadAllExtensionsInNativeDirectory = loadAllExtensionsInNativeDirectory; From 89febfb1edeb0ca66d3172bacf18eb17b77356a2 Mon Sep 17 00:00:00 2001 From: Joel Brandt Date: Wed, 6 Jun 2012 20:59:45 -0700 Subject: [PATCH 32/45] Rewrote unit tests for JavaScriptInlineEditor to get JSUtils from the require context that was used to load the extension. We only need to do this in unit tests where we rely on JSUtils to get the DocumentManager, etc. from the current window. Tests that JUST test JSUtils functionality (independent of Brackets) can load it with the standard require context. --- .../JavaScriptInlineEditor/unittests.js | 30 ++++++++++++------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/src/extensions/disabled/JavaScriptInlineEditor/unittests.js b/src/extensions/disabled/JavaScriptInlineEditor/unittests.js index e48b9cc58..4951b77c1 100644 --- a/src/extensions/disabled/JavaScriptInlineEditor/unittests.js +++ b/src/extensions/disabled/JavaScriptInlineEditor/unittests.js @@ -44,7 +44,7 @@ define(function (require, exports, module) { var extensionPath = FileUtils.getNativeModuleDirectoryPath(module); - describe("JSQuickEdit", function () { + describe("JavaScriptInlineEditor", function () { var testPath = extensionPath + "/unittest-files", testWindow, @@ -495,7 +495,6 @@ define(function (require, exports, module) { SpecRunnerUtils.closeTestWindow(); }); -/*** it("should return the correct offsets if the file has changed", function () { var didOpen = false, gotError = false; @@ -531,8 +530,11 @@ define(function (require, exports, module) { FileIndexManager.getFileInfoList("all") .done(function (fileInfos) { + var extensionRequire = brackets.getModule('utils/ExtensionLoader').getRequireContextForExtension('JavaScriptInlineEditor'); + var JSUtilsInExtension = extensionRequire("JSUtils"); + // Look for "edit2" function - JSUtils.findMatchingFunctions("edit2", fileInfos) + JSUtilsInExtension.findMatchingFunctions("edit2", fileInfos) .done(function (result) { functions = result; }); }); }); @@ -545,8 +547,7 @@ define(function (require, exports, module) { expect(functions[0].lineEnd).toBe(13); }); }); -***/ -/*** + it("should return a newly created function in an unsaved file", function () { var didOpen = false, gotError = false; @@ -563,24 +564,31 @@ define(function (require, exports, module) { runs(function () { var doc = DocumentManager.getCurrentDocument(); - // Add a new function to the file doc.setText(doc.getText() + "\n\nfunction TESTFUNCTION() {\n return true;\n}\n"); // Look for the selector we just created - JSUtils.findMatchingFunctions("TESTFUNCTION", FileIndexManager.getFileInfoList("all")) - .done(function (result) { functions = result; }); + FileIndexManager.getFileInfoList("all") + .done(function (fileInfos) { + var extensionRequire = brackets.getModule('utils/ExtensionLoader').getRequireContextForExtension('JavaScriptInlineEditor'); + var JSUtilsInExtension = extensionRequire("JSUtils"); + + // Look for "TESTFUNCTION" function + JSUtilsInExtension.findMatchingFunctions("TESTFUNCTION", fileInfos) + .done(function (result) { + functions = result; + }); + }); }); waitsFor(function () { return functions !== null; }, "JSUtils.findMatchingFunctions() timeout", 1000); runs(function () { expect(functions.length).toBe(1); - expect(functions[0].lineStart).toBe(24); - expect(functions[0].lineEnd).toBe(26); + expect(functions[0].lineStart).toBe(33); + expect(functions[0].lineEnd).toBe(35); }); }); -***/ }); }); //describe("JS Parsing") }); From 81fc60e3d991e906bb5bcadd234471398f28dc60 Mon Sep 17 00:00:00 2001 From: Ty Voliter Date: Mon, 11 Jun 2012 15:08:05 -0700 Subject: [PATCH 33/45] Code review fixes. Combined both offset to lines functions into a single function --- .../JavaScriptInlineEditor/JSUtils.js | 10 ++-- src/search/FindInFiles.js | 2 +- src/utils/StringUtils.js | 58 +++++++++---------- 3 files changed, 32 insertions(+), 38 deletions(-) diff --git a/src/extensions/disabled/JavaScriptInlineEditor/JSUtils.js b/src/extensions/disabled/JavaScriptInlineEditor/JSUtils.js index 2995c3032..354f3288f 100644 --- a/src/extensions/disabled/JavaScriptInlineEditor/JSUtils.js +++ b/src/extensions/disabled/JavaScriptInlineEditor/JSUtils.js @@ -117,8 +117,8 @@ define(function (require, exports, module) { matchingFunctions.push({ document: doc, name: funcEntry.functionName, - lineStart: StringUtils.offsetToLineNumForLoops(lines, funcEntry.offset), - lineEnd: StringUtils.offsetToLineNumForLoops(lines, endOffset) + lineStart: StringUtils.offsetToLineNum(lines, funcEntry.offset), + lineEnd: StringUtils.offsetToLineNum(lines, endOffset) }); } }); @@ -271,7 +271,7 @@ define(function (require, exports, module) { * @return {Array.<{offset:number, functionName:string}>} * Array of objects containing the start offset for each matched function name. */ - function _findAllMatchingFunctionsInText(text, functionName) { + function _findAllMatchingFunctionsInText(text, functionName) { var allFunctions = _findAllFunctionsInText(text); var result = []; var lines = text.split("\n"); @@ -281,8 +281,8 @@ define(function (require, exports, module) { var endOffset = _getFunctionEndOffset(text, funcEntry.offset); result.push({ name: funcEntry.functionName, - lineStart: StringUtils.offsetToLineNumForLoops(lines, funcEntry.offset), - lineEnd: StringUtils.offsetToLineNumForLoops(lines, endOffset) + lineStart: StringUtils.offsetToLineNum(lines, funcEntry.offset), + lineEnd: StringUtils.offsetToLineNum(lines, endOffset) }); } }); diff --git a/src/search/FindInFiles.js b/src/search/FindInFiles.js index 2d4d9d8af..d2ebaf0f8 100644 --- a/src/search/FindInFiles.js +++ b/src/search/FindInFiles.js @@ -142,7 +142,7 @@ define(function (require, exports, module) { var match; var lines = StringUtils.getLines(contents); while ((match = queryExpr.exec(contents)) !== null) { - var lineNum = StringUtils.offsetToLineNumForLoops(lines, match.index); + var lineNum = StringUtils.offsetToLineNum(lines, match.index); var line = lines[lineNum]; var ch = match.index - contents.lastIndexOf("\n", match.index) - 1; // 0-based index var matchLength = match[0].length; diff --git a/src/utils/StringUtils.js b/src/utils/StringUtils.js index a580ed03c..fb88c0c6d 100644 --- a/src/utils/StringUtils.js +++ b/src/utils/StringUtils.js @@ -22,7 +22,7 @@ */ /*jslint vars: true, plusplus: true, devel: true, nomen: true, regexp: true, indent: 4, maxerr: 50 */ -/*global define */ +/*global define, $ */ /** * Utilities functions related to string manipulation @@ -72,43 +72,38 @@ define(function (require, exports, module) { } /** - * Returns a line number corresponding to an offset in some text. This version - * is optimized for repeatidly calling the function on the same text in a loop. - * Use getLines() to divide the text into lines onces, then repeatidly call + * Returns a line number corresponding to an offset in some text. The text can + * be specified as a single string or as an array of strings that correspond to + * the lines of the string. + * + * Specify the text in lines when repeatedly calling the function on the same + * text in a loop. Use getLines() to divide the text into lines, then repeatedly call * this function to compute a line number from the offset. * - * @param {Array.} lines - an array of strings corresponding to the - * lines of text in a string. + * @param {string | Array.} textOrLines - string or array of lines from which + * to compute the line number from the offset * @param {number} offset * @return {number} line number */ - function offsetToLineNumForLoops(lines, offset) { - var line, total = 0; - for (line = 0; line < lines.length; line++) { - if (total <= offset) { - // add 1 per line since /n were removed by splitting, but they needed to - // contribute to the total offset count - total += lines[line].length + 1; - } else { - return line - 1; + function offsetToLineNum(textOrLines, offset) { + if ($.isArray(textOrLines)) { + var lines = textOrLines, + total = 0, + line; + for (line = 0; line < lines.length; line++) { + if (total <= offset) { + // add 1 per line since /n were removed by splitting, but they needed to + // contribute to the total offset count + total += lines[line].length + 1; + } else { + return line - 1; + } } + + return undefined; + } else { + return textOrLines.substr(0, offset).split("\n").length - 1; } - - return undefined; - } - - /** - * Returns the line number corresponding to an offset in text - * - * Note: When repeatidly computing line numbers for offsets in the same text - * use offsetToLineNumForLoops() instead which is performance optimized for loops - * - * @param {string} text - * @param {number} offset - * @return {number} line number - */ - function offsetToLineNum(text, offset) { - return text.substr(0, offset).split("\n").length - 1; } // Define public API @@ -117,5 +112,4 @@ define(function (require, exports, module) { exports.regexEscape = regexEscape; exports.getLines = getLines; exports.offsetToLineNum = offsetToLineNum; - exports.offsetToLineNumForLoops = offsetToLineNumForLoops; }); \ No newline at end of file From 264288be5e9505693cb7b5a1df0ce53090dce190 Mon Sep 17 00:00:00 2001 From: Joel Brandt Date: Mon, 11 Jun 2012 16:43:24 -0700 Subject: [PATCH 34/45] added a 'ready' event to brackets that fires when brackets is completely done loading --- src/brackets.js | 51 ++++++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 48 insertions(+), 3 deletions(-) diff --git a/src/brackets.js b/src/brackets.js index d316640a6..29cb34eba 100644 --- a/src/brackets.js +++ b/src/brackets.js @@ -72,6 +72,10 @@ define(function (require, exports, module) { ExtensionLoader = require("utils/ExtensionLoader"), SidebarView = require("project/SidebarView"), Async = require("utils/Async"); + + // Local variables + var bracketsReady = false, + bracketsReadyHandlers = []; //Load modules that self-register and just need to get included in the main project require("editor/CodeHintManager"); @@ -80,6 +84,34 @@ define(function (require, exports, module) { require("view/ViewCommandHandlers"); require("search/FindInFiles"); + function _callBracketsReadyHandler(handler) { + try { + handler(); + } catch (e) { + console.log("Exception when calling a 'brackets done loading' handler"); + console.log(e); + } + } + + function _onBracketsReady() { + var i; + bracketsReady = true; + for (i = 0; i < bracketsReadyHandlers.length; i++) { + _callBracketsReadyHandler(bracketsReadyHandlers[i]); + } + bracketsReadyHandlers = []; + } + + // WARNING: This event won't fire if ANY extension fails to load or throws an error during init. + // To fix this, we need to make a change to _initExtensions (filed as issue 1029) + function _registerBracketsReadyHandler(handler) { + if (bracketsReady) { + _callBracketsReadyHandler(handler); + } else { + bracketsReadyHandlers.push(handler); + } + } + // TODO: Issue 949 - the following code should be shared function _initGlobalBrackets() { @@ -119,10 +151,18 @@ define(function (require, exports, module) { // Note: we change the name to "getModule" because this won't do exactly the same thing as 'require' in AMD-wrapped // modules. The extension will only be able to load modules that have already been loaded once. brackets.getModule = require; + + // Provide a way for anyone (including code not using require) to register a handler for the brackets 'ready' event + // This event is like $(document).ready in that it will call the handler immediately if brackets is already done loading + // + // WARNING: This event won't fire if ANY extension fails to load or throws an error during init. + // To fix this, we need to make a change to _initExtensions (filed as issue 1029) + brackets.ready = _registerBracketsReadyHandler; } + // TODO: (issue 1029) Add timeout to main extension loading promise, so that we always call this function + // Making this fix will fix a warning (search for issue 1029) related to the brackets 'ready' event. function _initExtensions() { - // FUTURE (JRB): As we get more fine-grained performance measurement, move this out of core application startup return Async.doInParallel(["default", "user"], function (item) { return ExtensionLoader.loadAllExtensionsInNativeDirectory( FileUtils.getNativeBracketsDirectoryPath() + "/extensions/" + item, @@ -156,8 +196,13 @@ define(function (require, exports, module) { CSSUtils : require("language/CSSUtils"), LiveDevelopment : require("LiveDevelopment/LiveDevelopment"), Inspector : require("LiveDevelopment/Inspector/Inspector"), - NativeApp : require("utils/NativeApp") + NativeApp : require("utils/NativeApp"), + doneLoading : false }; + + brackets.ready(function () { + brackets.test.doneLoading = true; + }); } function _initDragAndDropListeners() { @@ -261,7 +306,7 @@ define(function (require, exports, module) { // finish UI initialization before loading extensions ProjectManager.loadProject().done(function () { _initTest(); - _initExtensions(); + _initExtensions().always(_onBracketsReady); }); } From 4b8af313dbe7e94e038e90ae1b945f7af6f38da0 Mon Sep 17 00:00:00 2001 From: Joel Brandt Date: Mon, 11 Jun 2012 16:44:50 -0700 Subject: [PATCH 35/45] createTestWindowAndRun now waits for brackets to be completely done loading (so extensions are loaded before tests run) --- test/spec/SpecRunnerUtils.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/spec/SpecRunnerUtils.js b/test/spec/SpecRunnerUtils.js index 179845f7e..874adb3bb 100644 --- a/test/spec/SpecRunnerUtils.js +++ b/test/spec/SpecRunnerUtils.js @@ -107,10 +107,10 @@ define(function (require, exports, module) { // FIXME (issue #249): Need an event or something a little more reliable... waitsFor( - function () { - return testWindow.brackets && testWindow.brackets.test; + function isBracketsDoneLoading() { + return testWindow.brackets && testWindow.brackets.test && testWindow.brackets.test.doneLoading; }, - 5000 + 10000 ); runs(function () { From ae7465d7e8832585849f09a99ab4177e83cb1dd6 Mon Sep 17 00:00:00 2001 From: Peter Flynn Date: Tue, 12 Jun 2012 10:20:01 -0700 Subject: [PATCH 36/45] Fix #971 (Find in Files treats \n, \r, etc. as regular expressions) -- Escape backslash too (and simplify escaping regexp to use char class instead of |). --- src/search/FindInFiles.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/search/FindInFiles.js b/src/search/FindInFiles.js index 544a0df67..0f79cacf1 100644 --- a/src/search/FindInFiles.js +++ b/src/search/FindInFiles.js @@ -271,7 +271,7 @@ define(function (require, exports, module) { // Query is a string. Turn it into a case-insensitive regexp // Escape regex special chars - query = query.replace(/(\(|\)|\{|\}|\[|\]|\.|\^|\$|\||\?|\+|\*)/g, "\\$1"); + query = query.replace(/([(){}\[\].\^$|?+*\\])/g, "\\$1"); return new RegExp(query, "gi"); } From 00ec60855f25afd7df43d9215112f6e53d16b8de Mon Sep 17 00:00:00 2001 From: Joel Brandt Date: Tue, 12 Jun 2012 10:34:37 -0700 Subject: [PATCH 37/45] added a comment explaining why we aren't using jquery deferreds for the done loading notification --- src/brackets.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/brackets.js b/src/brackets.js index 29cb34eba..6f16f68d2 100644 --- a/src/brackets.js +++ b/src/brackets.js @@ -157,6 +157,10 @@ define(function (require, exports, module) { // // WARNING: This event won't fire if ANY extension fails to load or throws an error during init. // To fix this, we need to make a change to _initExtensions (filed as issue 1029) + // + // TODO (issue 1034): We *could* use a $.Deferred for this, except deferred objects enter a broken + // state if any resolution callback throws an exception. Since third parties (e.g. extensions) may + // add callbacks to this, we need to be robust to exceptions brackets.ready = _registerBracketsReadyHandler; } From 8ab5e3a854c43b413231d75b0d232b4a1a6444e0 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Tue, 12 Jun 2012 11:06:38 -0700 Subject: [PATCH 38/45] Add ExtensionUtils to root require context --- src/brackets.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/brackets.js b/src/brackets.js index d316640a6..015637b6a 100644 --- a/src/brackets.js +++ b/src/brackets.js @@ -79,6 +79,7 @@ define(function (require, exports, module) { require("debug/DebugCommandHandlers"); require("view/ViewCommandHandlers"); require("search/FindInFiles"); + require("utils/ExtensionUtils"); // TODO: Issue 949 - the following code should be shared From 123f19f3a0640bb603bddf8e5a1335bf4205cf6a Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Tue, 12 Jun 2012 11:45:16 -0700 Subject: [PATCH 39/45] Revert .json file listing of test suites. Conflicts: test/SpecRunner.js --- test/PerformanceTestSuite.js | 30 ++++++++++++++++++++++ test/PerformanceTestSuite.json | 3 --- test/SpecRunner.js | 20 +++++++-------- test/UnitTestSuite.js | 47 ++++++++++++++++++++++++++++++++++ test/UnitTestSuite.json | 20 --------------- 5 files changed, 87 insertions(+), 33 deletions(-) create mode 100644 test/PerformanceTestSuite.js delete mode 100644 test/PerformanceTestSuite.json create mode 100644 test/UnitTestSuite.js delete mode 100644 test/UnitTestSuite.json diff --git a/test/PerformanceTestSuite.js b/test/PerformanceTestSuite.js new file mode 100644 index 000000000..6c53991d4 --- /dev/null +++ b/test/PerformanceTestSuite.js @@ -0,0 +1,30 @@ +/* + * 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 + * 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, + * 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 + * 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("perf/Performance-test"); +}); \ No newline at end of file diff --git a/test/PerformanceTestSuite.json b/test/PerformanceTestSuite.json deleted file mode 100644 index 156cfa988..000000000 --- a/test/PerformanceTestSuite.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "specs": ["perf/Performance-test.js"] -} \ No newline at end of file diff --git a/test/SpecRunner.js b/test/SpecRunner.js index 0c0ffcedb..cffe87bc1 100644 --- a/test/SpecRunner.js +++ b/test/SpecRunner.js @@ -26,8 +26,12 @@ // Set the baseUrl to brackets/src require.config({ - baseUrl: "../src"/*, - urlArgs: "bust=" + (new Date()).getTime() // cache busting */ + baseUrl: "../src", + paths: { + "test": "../test", + "perf": "../test/perf", + "spec": "../test/spec" + } }); define(function (require, exports, module) { @@ -132,21 +136,17 @@ define(function (require, exports, module) { // add performance reporting if (suite === "PerformanceTestSuite") { + require("test/PerformanceTestSuite"); jasmineEnv.addReporter(new PerformanceReporter()); + } else { + require("test/UnitTestSuite"); } localStorage.setItem("SpecRunner.suite", suite); $("#" + suite).closest("li").toggleClass("active", true); - var jsonResult = $.getJSON(suite + ".json"); - - jsonResult.done(function (data) { - // load specs and run jasmine - require(data.specs, function () { - jasmineEnv.execute(); - }); - }); + jasmineEnv.execute(); }; } diff --git a/test/UnitTestSuite.js b/test/UnitTestSuite.js new file mode 100644 index 000000000..4b8dc469b --- /dev/null +++ b/test/UnitTestSuite.js @@ -0,0 +1,47 @@ +/* + * 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 + * 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, + * 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 + * 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/LowLevelFileIO-test"); + require("spec/DocumentCommandHandlers-test"); + require("spec/NativeFileSystem-test"); + require("spec/PreferencesManager-test"); + require("spec/Editor-test"); + require("spec/EditorCommandHandlers-test"); + require("spec/ProjectManager-test"); + require("spec/WorkingSetView-test"); + require("spec/KeyBindingManager-test"); + require("spec/FileIndexManager-test"); + require("spec/CodeHintUtils-test"); + require("spec/CSSUtils-test"); + require("spec/InlineEditorProviders-test"); + require("spec/MultiRangeInlineEditor-test"); + require("spec/LiveDevelopment-test"); + require("spec/CommandManager-test"); + require("spec/ViewUtils-test"); + require("spec/Menu-test"); +}); \ No newline at end of file diff --git a/test/UnitTestSuite.json b/test/UnitTestSuite.json deleted file mode 100644 index f9a168b17..000000000 --- a/test/UnitTestSuite.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "specs": ["spec/LowLevelFileIO-test.js", - "spec/DocumentCommandHandlers-test.js", - "spec/NativeFileSystem-test.js", - "spec/PreferencesManager-test.js", - "spec/Editor-test.js", - "spec/EditorCommandHandlers-test.js", - "spec/ProjectManager-test.js", - "spec/WorkingSetView-test.js", - "spec/KeyBindingManager-test.js", - "spec/FileIndexManager-test.js", - "spec/CodeHintUtils-test.js", - "spec/CSSUtils-test.js", - "spec/InlineEditorProviders-test.js", - "spec/MultiRangeInlineEditor-test.js", - "spec/LiveDevelopment-test.js", - "spec/CommandManager-test.js", - "spec/ViewUtils-test.js", - "spec/Menu-test.js"] -} From af3510c50109f15ba11b0c6a908cab5970e52bb3 Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Tue, 12 Jun 2012 12:05:21 -0700 Subject: [PATCH 40/45] Re-implement performance suite filtering due to RequireJS changes in SpecRunner --- test/BootstrapReporter.js | 65 ++++++++++++++++++++++++++++------- test/SpecRunner.js | 33 ++++++++++++++---- test/perf/Performance-test.js | 2 ++ 3 files changed, 81 insertions(+), 19 deletions(-) diff --git a/test/BootstrapReporter.js b/test/BootstrapReporter.js index 53fccbb19..e20bbde94 100644 --- a/test/BootstrapReporter.js +++ b/test/BootstrapReporter.js @@ -3,7 +3,7 @@ (function ($) { 'use strict'; - jasmine.BootstrapReporter = function (doc) { + jasmine.BootstrapReporter = function (doc, filter) { this._paramMap = {}; this.document = doc || document; this._env = jasmine.getEnv(); @@ -20,6 +20,7 @@ } this._runAll = this._paramMap.spec === "All"; + this._topLevelFilter = filter; this._env.specFilter = this.createSpecFilter(this._paramMap.spec); this._runner = this._env.currentRunner(); @@ -45,11 +46,25 @@ }; jasmine.BootstrapReporter.prototype.createSpecFilter = function (filterString) { + var self = this; + return function (spec) { + // filterString is undefined when no top-level suite is active (e.g. "All", "HTMLUtils", etc.) + // When undefined, all specs fail this filter and no tests are ran. This is by design. + // This setup allows the SpecRunner to load initially without automatically running all tests. + if (filterString === undefined) { + return false; + } + + if (!self._topLevelFilter(spec)) { + return false; + } + if (filterString === "All") { return true; } - return spec.getFullName().indexOf(filterString) === 0; + + return (spec.getFullName().indexOf(filterString) === 0); }; }; @@ -82,7 +97,11 @@ self = this; // count specs attached directly to this suite - count = suite.specs().length; + suite.specs().forEach(function (spec, index) { + if (self._topLevelFilter(spec)) { + count++; + } + }); // recursively count child suites suite.suites().forEach(function (child, index) { @@ -110,11 +129,22 @@ return 0; }); - this.$suiteList.append(this._createSuiteListItem(null, this._runner.specs().length)); - topLevel.forEach(function (suite, index) { - self.$suiteList.append(self._createSuiteListItem(suite, self._countSpecs(suite))); + var count = self._countSpecs(suite); + + if (count > 0) { + self.$suiteList.append(self._createSuiteListItem(suite, count)); + } }); + + // count all speces + var allSpecsCount = 0; + $.each(this._topLevelSuiteMap, function (index, value) { + allSpecsCount += value.specCount; + }); + + // add an "all" top-level suite + this.$suiteList.prepend(this._createSuiteListItem(null, allSpecsCount)); }; jasmine.BootstrapReporter.prototype._showProgressBar = function (spec) { @@ -129,7 +159,8 @@ jasmine.BootstrapReporter.prototype.reportRunnerStarting = function (runner) { var i, specs = runner.specs(), - topLevelData; + topLevelData, + self = this; // create top level suite list navigation this._createSuiteList(); @@ -144,11 +175,11 @@ this._specCount = 0; this._specCompleteCount = 0; - for (i = 0; i < specs.length; i++) { - if (this._env.specFilter(specs[i])) { - this._specCount++; + specs.forEach(function (spec, index) { + if (self._env.specFilter(spec)) { + self._specCount++; } - } + }); if (this._specCount) { this._showProgressBar(); @@ -230,8 +261,16 @@ jasmine.BootstrapReporter.prototype._updateStatus = function (spec) { var data = this._getTopLevelSuiteData(spec), - allData = this._topLevelSuiteMap.All, - results = spec.results(); + allData, + results; + + // Top-level suite data will not exist if filtered + if (!data) { + return; + } + + allData = this._topLevelSuiteMap.All; + results = spec.results(); this._updateSuiteStatus(data, results); this._updateSuiteStatus(allData, results); diff --git a/test/SpecRunner.js b/test/SpecRunner.js index cffe87bc1..8c759ce53 100644 --- a/test/SpecRunner.js +++ b/test/SpecRunner.js @@ -44,6 +44,10 @@ define(function (require, exports, module) { Menus = require("command/Menus"), PerformanceReporter = require("perf/PerformanceReporter.js").PerformanceReporter; + // Load both top-level suites. Filtering is applied at the top-level as a filter to BootstrapReporter. + require("test/UnitTestSuite"); + require("test/PerformanceTestSuite"); + var suite; function getParamMap() { @@ -123,8 +127,6 @@ define(function (require, exports, module) { currentWindowOnload(); } - jasmineEnv.addReporter(new jasmine.BootstrapReporter(document)); - $("#show-dev-tools").click(function () { brackets.app.showDeveloperTools(); }); @@ -134,12 +136,31 @@ define(function (require, exports, module) { suite = getParamMap().suite || localStorage.getItem("SpecRunner.suite") || "UnitTestSuite"; + // Create a top-level filter to show/hide performance tests + var isPerfSuite = (suite === "PerformanceTestSuite"), + performanceFilter = function (spec) { + if (spec.performance === true) { + return isPerfSuite; + } + + var suite = spec.suite; + + while (suite) { + if (suite.performance === true) { + return isPerfSuite; + } + + suite = suite.parentSuite; + } + + return !isPerfSuite; + }; + + jasmineEnv.addReporter(new jasmine.BootstrapReporter(document, performanceFilter)); + // add performance reporting - if (suite === "PerformanceTestSuite") { - require("test/PerformanceTestSuite"); + if (isPerfSuite) { jasmineEnv.addReporter(new PerformanceReporter()); - } else { - require("test/UnitTestSuite"); } localStorage.setItem("SpecRunner.suite", suite); diff --git a/test/perf/Performance-test.js b/test/perf/Performance-test.js index 204f62b13..cfa48115f 100644 --- a/test/perf/Performance-test.js +++ b/test/perf/Performance-test.js @@ -43,6 +43,8 @@ define(function (require, exports, module) { describe("Performance Tests", function () { + this.performance = true; + // Note: this tests assumes that the "brackets-scenario" repo is in the same folder // as the "brackets-app" // From d74c3cd8ffc56a860c044701fb130cbd5fa5198c Mon Sep 17 00:00:00 2001 From: Narciso Jaramillo Date: Tue, 12 Jun 2012 13:15:40 -0700 Subject: [PATCH 41/45] Restore Shift-Tab to outdent behavior --- src/editor/Editor.js | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/editor/Editor.js b/src/editor/Editor.js index f5ff28eae..4d3e6ffcf 100644 --- a/src/editor/Editor.js +++ b/src/editor/Editor.js @@ -314,24 +314,25 @@ define(function (require, exports, module) { // Editor supplies some standard keyboard behavior extensions of its own var codeMirrorKeyMap = { - "Tab" : _handleTabKey, + "Tab": _handleTabKey, + "Shift-Tab": "indentLess", - "Left" : function (instance) { + "Left": function (instance) { if (!_handleSoftTabNavigation(instance, -1, "moveH")) { CodeMirror.commands.goCharLeft(instance); } }, - "Right" : function (instance) { + "Right": function (instance) { if (!_handleSoftTabNavigation(instance, 1, "moveH")) { CodeMirror.commands.goCharRight(instance); } }, - "Backspace" : function (instance) { + "Backspace": function (instance) { if (!_handleSoftTabNavigation(instance, -1, "deleteH")) { CodeMirror.commands.delCharLeft(instance); } }, - "Delete" : function (instance) { + "Delete": function (instance) { if (!_handleSoftTabNavigation(instance, 1, "deleteH")) { CodeMirror.commands.delCharRight(instance); } From ae482fb9f24d76550f2e5fefb9033fd13823b95d Mon Sep 17 00:00:00 2001 From: Ty Voliter Date: Tue, 12 Jun 2012 14:08:21 -0700 Subject: [PATCH 42/45] code review fixes --- src/utils/StringUtils.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/utils/StringUtils.js b/src/utils/StringUtils.js index fb88c0c6d..52209fdf0 100644 --- a/src/utils/StringUtils.js +++ b/src/utils/StringUtils.js @@ -25,7 +25,7 @@ /*global define, $ */ /** - * Utilities functions related to string manipulation + * Utilities functions related to string manipulation * */ define(function (require, exports, module) { @@ -91,16 +91,23 @@ define(function (require, exports, module) { total = 0, line; for (line = 0; line < lines.length; line++) { - if (total <= offset) { + if (total < offset) { // add 1 per line since /n were removed by splitting, but they needed to // contribute to the total offset count total += lines[line].length + 1; + } else if (total === offset) { + return line; } else { return line - 1; } } - return undefined; + // if offset is NOT over the total then offset is in the last line + if (offset <= total) { + return line - 1; + } else { + return undefined; + } } else { return textOrLines.substr(0, offset).split("\n").length - 1; } From 208b422c2ed0ba4dacdc960345cac9fd0de0b4e7 Mon Sep 17 00:00:00 2001 From: Glenn Ruehle Date: Tue, 12 Jun 2012 14:57:32 -0700 Subject: [PATCH 43/45] Update CodeMirror SHA --- src/thirdparty/CodeMirror2 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/thirdparty/CodeMirror2 b/src/thirdparty/CodeMirror2 index c12c3d097..5c2f30c60 160000 --- a/src/thirdparty/CodeMirror2 +++ b/src/thirdparty/CodeMirror2 @@ -1 +1 @@ -Subproject commit c12c3d097493c065911a3eb95e05f38e35a17ffe +Subproject commit 5c2f30c606fc40a5271992b94141d79ce49c9309 From c93e1e4618cdc36162fd4e8e12c39383deb4aab6 Mon Sep 17 00:00:00 2001 From: Randy Edmunds Date: Tue, 12 Jun 2012 15:00:57 -0700 Subject: [PATCH 44/45] Update comments --- src/command/CommandManager.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/command/CommandManager.js b/src/command/CommandManager.js index e8526692e..7d788fb24 100644 --- a/src/command/CommandManager.js +++ b/src/command/CommandManager.js @@ -39,7 +39,8 @@ define(function (require, exports, module) { var _commands = {}; /** - * Map of all registered global commands + * Temporary copy of commands map for restoring after testing + * TODO (issue #1039): implement separate require contexts for unit tests * @type Object. */ var _commandsOriginal = {}; @@ -182,7 +183,7 @@ define(function (require, exports, module) { /** * Clear all commands for unit testing, but first make copy of commands so that - * they can be restored afterward + * they can be restored afterward */ function _testReset() { _commandsOriginal = _commands; From 4db3e085deb2d1d76359a0254fc8d309ed6da83f Mon Sep 17 00:00:00 2001 From: Jason San Jose Date: Tue, 12 Jun 2012 17:38:40 -0700 Subject: [PATCH 45/45] code review comments --- test/BootstrapReporter.js | 14 ++++++++++++-- test/PerformanceTestSuite.js | 1 + test/UnitTestSuite.js | 22 +++++++++++----------- 3 files changed, 24 insertions(+), 13 deletions(-) diff --git a/test/BootstrapReporter.js b/test/BootstrapReporter.js index e20bbde94..c5ebefd74 100644 --- a/test/BootstrapReporter.js +++ b/test/BootstrapReporter.js @@ -20,7 +20,12 @@ } this._runAll = this._paramMap.spec === "All"; + + // _topLevelFilter is applied first before the spec filter this._topLevelFilter = filter; + + // Jasmine's runner uses the specFilter to choose which tests to run. + // This is typically a subset of all tests loaded. this._env.specFilter = this.createSpecFilter(this._paramMap.spec); this._runner = this._env.currentRunner(); @@ -45,6 +50,11 @@ this.$resultsContainer = $("#results-container"); }; + /** + * @private + * Filters specs by full name. Applies _topLevelFilter first before checking + * for a matching starting substring. + */ jasmine.BootstrapReporter.prototype.createSpecFilter = function (filterString) { var self = this; @@ -157,8 +167,7 @@ }; jasmine.BootstrapReporter.prototype.reportRunnerStarting = function (runner) { - var i, - specs = runner.specs(), + var specs = runner.specs(), topLevelData, self = this; @@ -276,6 +285,7 @@ this._updateSuiteStatus(allData, results); }; + // Jasmine calls this function for all specs, not just filtered specs. jasmine.BootstrapReporter.prototype.reportSpecResults = function (spec) { var results = spec.results(), $specLink, diff --git a/test/PerformanceTestSuite.js b/test/PerformanceTestSuite.js index 6c53991d4..addeefd4c 100644 --- a/test/PerformanceTestSuite.js +++ b/test/PerformanceTestSuite.js @@ -26,5 +26,6 @@ define(function (require, exports, module) { 'use strict'; + // Each suite or spec must have this.performance = true to be filtered properly require("perf/Performance-test"); }); \ No newline at end of file diff --git a/test/UnitTestSuite.js b/test/UnitTestSuite.js index 4b8dc469b..cb460651e 100644 --- a/test/UnitTestSuite.js +++ b/test/UnitTestSuite.js @@ -26,22 +26,22 @@ define(function (require, exports, module) { 'use strict'; - require("spec/LowLevelFileIO-test"); + require("spec/CodeHintUtils-test"); + require("spec/CommandManager-test"); + require("spec/CSSUtils-test"); require("spec/DocumentCommandHandlers-test"); - require("spec/NativeFileSystem-test"); - require("spec/PreferencesManager-test"); require("spec/Editor-test"); require("spec/EditorCommandHandlers-test"); - require("spec/ProjectManager-test"); - require("spec/WorkingSetView-test"); - require("spec/KeyBindingManager-test"); require("spec/FileIndexManager-test"); - require("spec/CodeHintUtils-test"); - require("spec/CSSUtils-test"); require("spec/InlineEditorProviders-test"); - require("spec/MultiRangeInlineEditor-test"); + require("spec/KeyBindingManager-test"); require("spec/LiveDevelopment-test"); - require("spec/CommandManager-test"); - require("spec/ViewUtils-test"); + require("spec/LowLevelFileIO-test"); require("spec/Menu-test"); + require("spec/MultiRangeInlineEditor-test"); + require("spec/NativeFileSystem-test"); + require("spec/PreferencesManager-test"); + require("spec/ProjectManager-test"); + require("spec/ViewUtils-test"); + require("spec/WorkingSetView-test"); }); \ No newline at end of file