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

Reload Without User Extensions #6334

Merged
merged 6 commits into from
Jan 4, 2014
Merged

Reload Without User Extensions #6334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9dff969
Add Debug Menu command: Reload Without User Extensions
lkcampbell Dec 29, 2013
3e335ad
Small code cleanup
lkcampbell Dec 29, 2013
dacd9bb
Reload should not load extensions in dev directory
lkcampbell Dec 30, 2013
7b40ba8
Code cleanup. Replace CollectionUtils with lodash. Add console log.
lkcampbell Dec 31, 2013
8182f60
Small comment update
lkcampbell Dec 31, 2013
d85f302
Make ExtensionLoader paths code more robust
lkcampbell Jan 4, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/command/Menus.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ define(function (require, exports, module) {
StringUtils = require("utils/StringUtils"),
CommandManager = require("command/CommandManager"),
PopUpManager = require("widgets/PopUpManager"),
ViewUtils = require("utils/ViewUtils"),
CollectionUtils = require("utils/CollectionUtils");
ViewUtils = require("utils/ViewUtils");

/**
* Brackets Application Menu Constants
Expand Down Expand Up @@ -949,7 +948,7 @@ define(function (require, exports, module) {
// Remove all of the menu items in the menu
menu = getMenu(id);

CollectionUtils.forEach(menuItemMap, function (value, key) {
_.forEach(menuItemMap, function (value, key) {
if (key.substring(0, id.length) === id) {
if (value.isDivider) {
menu.removeMenuDivider(key);
Expand Down
235 changes: 154 additions & 81 deletions src/extensions/default/DebugCommands/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ define(function (require, exports, module) {
StringUtils = brackets.getModule("utils/StringUtils"),
Dialogs = brackets.getModule("widgets/Dialogs"),
Strings = brackets.getModule("strings"),
AppInit = brackets.getModule("utils/AppInit"),
UrlParams = brackets.getModule("utils/UrlParams").UrlParams,
StatusBar = brackets.getModule("widgets/StatusBar"),
NodeDebugUtils = require("NodeDebugUtils"),
PerfDialogTemplate = require("text!htmlContent/perf-dialog.html"),
LanguageDialogTemplate = require("text!htmlContent/language-dialog.html");
Expand All @@ -54,19 +57,20 @@ define(function (require, exports, module) {
var DEBUG_MENU = "debug-menu";

/** @const {string} Debug commands IDs */
var DEBUG_REFRESH_WINDOW = "debug.refreshWindow", // string must MATCH string in native code (brackets_extensions)
DEBUG_SHOW_DEVELOPER_TOOLS = "debug.showDeveloperTools",
DEBUG_RUN_UNIT_TESTS = "debug.runUnitTests",
DEBUG_SHOW_PERF_DATA = "debug.showPerfData",
DEBUG_NEW_BRACKETS_WINDOW = "debug.newBracketsWindow",
DEBUG_SWITCH_LANGUAGE = "debug.switchLanguage",
DEBUG_ENABLE_NODE_DEBUGGER = "debug.enableNodeDebugger",
DEBUG_LOG_NODE_STATE = "debug.logNodeState",
DEBUG_RESTART_NODE = "debug.restartNode";
var DEBUG_REFRESH_WINDOW = "debug.refreshWindow", // string must MATCH string in native code (brackets_extensions)
DEBUG_SHOW_DEVELOPER_TOOLS = "debug.showDeveloperTools",
DEBUG_RUN_UNIT_TESTS = "debug.runUnitTests",
DEBUG_SHOW_PERF_DATA = "debug.showPerfData",
DEBUG_RELOAD_WITHOUT_USER_EXTS = "debug.reloadWithoutUserExts",
DEBUG_NEW_BRACKETS_WINDOW = "debug.newBracketsWindow",
DEBUG_SWITCH_LANGUAGE = "debug.switchLanguage",
DEBUG_ENABLE_NODE_DEBUGGER = "debug.enableNodeDebugger",
DEBUG_LOG_NODE_STATE = "debug.logNodeState",
DEBUG_RESTART_NODE = "debug.restartNode";



function handleShowDeveloperTools(commandData) {
function handleShowDeveloperTools() {
brackets.app.showDeveloperTools();
}

Expand All @@ -92,7 +96,115 @@ define(function (require, exports, module) {
}
}

function _handleShowPerfData() {
/**
* Disables Brackets' cache via the remote debugging protocol.
* @return {$.Promise} A jQuery promise that will be resolved when the cache is disabled and be rejected in any other case
*/
function disableCache() {
var result = new $.Deferred();

if (brackets.inBrowser) {
result.resolve();
} else {
var Inspector = brackets.getModule("LiveDevelopment/Inspector/Inspector");
var port = brackets.app.getRemoteDebuggingPort ? brackets.app.getRemoteDebuggingPort() : 9234;
Inspector.getDebuggableWindows("127.0.0.1", port)
.fail(result.reject)
.done(function (response) {
var page = response[0];
if (!page || !page.webSocketDebuggerUrl) {
result.reject();
return;
}
var _socket = new WebSocket(page.webSocketDebuggerUrl);
// Disable the cache
_socket.onopen = function _onConnect() {
_socket.send(JSON.stringify({ id: 1, method: "Network.setCacheDisabled", params: { "cacheDisabled": true } }));
};
// The first message will be the confirmation => disconnected to allow remote debugging of Brackets
_socket.onmessage = function _onMessage(e) {
_socket.close();
result.resolve();
};
// In case of an error
_socket.onerror = result.reject;
});
}

return result.promise();
}

/**
* Does a full reload of the browser window
* @param {string} href The url to reload into the window
*/
function browserReload(href) {
return CommandManager.execute(Commands.FILE_CLOSE_ALL, { promptOnly: true }).done(function () {
// Give everyone a chance to save their state - but don't let any problems block
// us from quitting
try {
$(ProjectManager).triggerHandler("beforeAppClose");
} catch (ex) {
console.error(ex);
}

// Disable the cache to make reloads work
disableCache().always(function () {
window.location.href = href;
});
});
}

function handleReload() {
var href = window.location.href,
params = new UrlParams();

// Make sure the Reload Without User Extensions parameter is removed
params.parse();

if (params.get("reloadWithoutUserExts")) {
params.remove("reloadWithoutUserExts");
Copy link
Contributor

Choose a reason for hiding this comment

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

If this comment is implemented, we may also want to display message in console log when user extensions are no longer disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I put the code to display the message here, it gets cleared out of the console log as soon as the browser reloads, so the user can never see it.

The only other place I could add it is in the AppInit(), but then the message will be displayed every time the browser loads: start up and reload. That might be more spammy than helpful.

I'm not sure there is an easy way to distinguish between browser loads that occur after extensions have been disabled versus regular browser loads.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, you can forget about this for now.

}

if (href.indexOf("?") !== -1) {
href = href.substring(0, href.indexOf("?"));
}

if (!params.isEmpty()) {
href += "?" + params.toString();
}

browserReload(href);
}

function handleReloadWithoutUserExts() {
var href = window.location.href,
params = new UrlParams();

// Remove all menus to assure extension menus and menu items are removed
_.forEach(Menus.getAllMenus(), function (value, key) {
Menus.removeMenu(key);
});

params.parse();

if (!params.get("reloadWithoutUserExts")) {
params.put("reloadWithoutUserExts", true);
}

if (href.indexOf("?") !== -1) {
href = href.substring(0, href.indexOf("?"));
}

href += "?" + params.toString();
browserReload(href);
}

function handleNewBracketsWindow() {
window.open(window.location.href);
}

function handleShowPerfData() {
var templateVars = {
delimitedPerfData: PerfUtils.getDelimitedPerfData(),
perfData: []
Expand Down Expand Up @@ -135,11 +247,7 @@ define(function (require, exports, module) {
});
}

function _handleNewBracketsWindow() {
window.open(window.location.href);
}

function _handleSwitchLanguage() {
function handleSwitchLanguage() {
var stringsPath = FileUtils.getNativeBracketsDirectoryPath() + "/nls";

FileSystem.getDirectoryForPath(stringsPath).getContents(function (err, entries) {
Expand Down Expand Up @@ -206,7 +314,7 @@ define(function (require, exports, module) {
});
}

function _enableRunTestsMenuItem() {
function enableRunTestsMenuItem() {
if (brackets.inBrowser) {
return;
}
Expand All @@ -225,86 +333,29 @@ define(function (require, exports, module) {
}
});
}


/**
* Disables Brackets' cache via the remote debugging protocol.
* @return {$.Promise} A jQuery promise that will be resolved when the cache is disabled and be rejected in any other case
*/
function _disableCache() {
var result = new $.Deferred();

if (brackets.inBrowser) {
result.resolve();
} else {
var Inspector = brackets.getModule("LiveDevelopment/Inspector/Inspector");
var port = brackets.app.getRemoteDebuggingPort ? brackets.app.getRemoteDebuggingPort() : 9234;
Inspector.getDebuggableWindows("127.0.0.1", port)
.fail(result.reject)
.done(function (response) {
var page = response[0];
if (!page || !page.webSocketDebuggerUrl) {
result.reject();
return;
}
var _socket = new WebSocket(page.webSocketDebuggerUrl);
// Disable the cache
_socket.onopen = function _onConnect() {
_socket.send(JSON.stringify({ id: 1, method: "Network.setCacheDisabled", params: { "cacheDisabled": true } }));
};
// The first message will be the confirmation => disconnected to allow remote debugging of Brackets
_socket.onmessage = function _onMessage(e) {
_socket.close();
result.resolve();
};
// In case of an error
_socket.onerror = result.reject;
});
}

return result.promise();
}

/** Does a full reload of the browser window */
function handleFileReload(commandData) {
return CommandManager.execute(Commands.FILE_CLOSE_ALL, { promptOnly: true }).done(function () {
// Give everyone a chance to save their state - but don't let any problems block
// us from quitting
try {
$(ProjectManager).triggerHandler("beforeAppClose");
} catch (ex) {
console.error(ex);
}

// Disable the cache to make reloads work
_disableCache().always(function () {
window.location.reload(true);
});
});
}


/* Register all the command handlers */

// Show Developer Tools (optionally enabled)
CommandManager.register(Strings.CMD_SHOW_DEV_TOOLS, DEBUG_SHOW_DEVELOPER_TOOLS, handleShowDeveloperTools)
CommandManager.register(Strings.CMD_SHOW_DEV_TOOLS, DEBUG_SHOW_DEVELOPER_TOOLS, handleShowDeveloperTools)
.setEnabled(!!brackets.app.showDeveloperTools);
CommandManager.register(Strings.CMD_REFRESH_WINDOW, DEBUG_REFRESH_WINDOW, handleFileReload);
CommandManager.register(Strings.CMD_NEW_BRACKETS_WINDOW, DEBUG_NEW_BRACKETS_WINDOW, _handleNewBracketsWindow);
CommandManager.register(Strings.CMD_REFRESH_WINDOW, DEBUG_REFRESH_WINDOW, handleReload);
CommandManager.register(Strings.CMD_RELOAD_WITHOUT_USER_EXTS, DEBUG_RELOAD_WITHOUT_USER_EXTS, handleReloadWithoutUserExts);
CommandManager.register(Strings.CMD_NEW_BRACKETS_WINDOW, DEBUG_NEW_BRACKETS_WINDOW, handleNewBracketsWindow);

// Start with the "Run Tests" item disabled. It will be enabled later if the test file can be found.
CommandManager.register(Strings.CMD_RUN_UNIT_TESTS, DEBUG_RUN_UNIT_TESTS, _runUnitTests)
.setEnabled(false);

CommandManager.register(Strings.CMD_SHOW_PERF_DATA, DEBUG_SHOW_PERF_DATA, _handleShowPerfData);
CommandManager.register(Strings.CMD_SWITCH_LANGUAGE, DEBUG_SWITCH_LANGUAGE, _handleSwitchLanguage);
CommandManager.register(Strings.CMD_SHOW_PERF_DATA, DEBUG_SHOW_PERF_DATA, handleShowPerfData);
CommandManager.register(Strings.CMD_SWITCH_LANGUAGE, DEBUG_SWITCH_LANGUAGE, handleSwitchLanguage);

// Node-related Commands
CommandManager.register(Strings.CMD_ENABLE_NODE_DEBUGGER, DEBUG_ENABLE_NODE_DEBUGGER, NodeDebugUtils.enableDebugger);
CommandManager.register(Strings.CMD_LOG_NODE_STATE, DEBUG_LOG_NODE_STATE, NodeDebugUtils.logNodeState);
CommandManager.register(Strings.CMD_RESTART_NODE, DEBUG_RESTART_NODE, NodeDebugUtils.restartNode);

_enableRunTestsMenuItem();
enableRunTestsMenuItem();


/*
Expand All @@ -313,6 +364,7 @@ define(function (require, exports, module) {
var menu = Menus.addMenu(Strings.DEBUG_MENU, DEBUG_MENU, Menus.BEFORE, Menus.AppMenuBar.HELP_MENU);
menu.addMenuItem(DEBUG_SHOW_DEVELOPER_TOOLS, KeyboardPrefs.showDeveloperTools);
menu.addMenuItem(DEBUG_REFRESH_WINDOW, KeyboardPrefs.refreshWindow);
menu.addMenuItem(DEBUG_RELOAD_WITHOUT_USER_EXTS);
menu.addMenuItem(DEBUG_NEW_BRACKETS_WINDOW);
menu.addMenuDivider();
menu.addMenuItem(DEBUG_SWITCH_LANGUAGE);
Expand All @@ -327,4 +379,25 @@ define(function (require, exports, module) {

// exposed for convenience, but not official API
exports._runUnitTests = _runUnitTests;

AppInit.htmlReady(function () {
// If in Reload Without User Extensions mode, update UI and log console message
var USER_EXT_STATUS_ID = "status-user-exts";

var params = new UrlParams(),
$icon = $("#toolbar-extension-manager"),
$indicator = $("<div>" + Strings.STATUSBAR_USER_EXTENSIONS_DISABLED + "</div>");

params.parse();

if (params.get("reloadWithoutUserExts") === "true") {
CommandManager.get(Commands.FILE_EXTENSION_MANAGER).setEnabled(false);
$icon.css({display: "none"});
StatusBar.addIndicator(USER_EXT_STATUS_ID, $indicator, true);
console.log("Brackets reloaded with user extensions disabled");
} else {
CommandManager.get(Commands.FILE_EXTENSION_MANAGER).setEnabled(true);
// Toolbar and status bar reload back to default states, no need to set
}
});
});
2 changes: 2 additions & 0 deletions src/nls/root/strings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ define({
"STATUSBAR_TAB_SIZE" : "Tab Size:",
"STATUSBAR_LINE_COUNT_SINGULAR" : "\u2014 {0} Line",
"STATUSBAR_LINE_COUNT_PLURAL" : "\u2014 {0} Lines",
"STATUSBAR_USER_EXTENSIONS_DISABLED" : "User Extensions Disabled",

// CodeInspection: errors/warnings
"ERRORS_PANEL_TITLE" : "{0} Errors",
Expand Down Expand Up @@ -436,6 +437,7 @@ define({
"DEBUG_MENU" : "Debug",
"CMD_SHOW_DEV_TOOLS" : "Show Developer Tools",
"CMD_REFRESH_WINDOW" : "Reload {APP_NAME}",
"CMD_RELOAD_WITHOUT_USER_EXTS" : "Reload Without User Extensions",
Copy link
Member

Choose a reason for hiding this comment

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

@redmunds @lkcampbell
Independent of the thread above, I wonder if we should consider relabeling this. "User Extensions" seems easy to misinterpret as referring to the "extensions/user" folder, which is wrong since this command also affects the "src/extensions/dev" folder.

I think we should simply say "Extensions" in these strings -- as far as most users are concerned, the only "extensions" that exist are the ones that show up in Extension Manager. The average end user has no idea that, as an implementation detail, certain non-optional core functionality happens to be structured as extensions too. So I think we may be trying to parse hairs more than needed in the labels here.

Copy link
Contributor

Choose a reason for hiding this comment

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

I support @peterflynn in this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like this idea as well mainly because it is a shorter length and fits in the menu better, while retaining the important semantics.

@SAplayer, @WebsiteDeveloper I know you guys were discussing this as part of the really long German translation (pull request #6366). Since I can't read German, what was the final translation, in English, that you guys agreed on?

Copy link
Contributor

Choose a reason for hiding this comment

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

@lkcampbell
Yes, we were talking about the same thing.

We first had this:
Reload without custom (meaning installed/User) extensions

Then I changed it to this:
Reload without extensions

But our reason to change this wasn't because it hadn't included the dev folder, it was just too long.

"CMD_NEW_BRACKETS_WINDOW" : "New {APP_NAME} Window",
"CMD_SWITCH_LANGUAGE" : "Switch Language",
"CMD_RUN_UNIT_TESTS" : "Run Tests",
Expand Down
19 changes: 14 additions & 5 deletions src/utils/ExtensionLoader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,10 @@ define(function (require, exports, module) {

require("utils/Global");

var FileSystem = require("filesystem/FileSystem"),
FileUtils = require("file/FileUtils"),
Async = require("utils/Async");
var FileSystem = require("filesystem/FileSystem"),
FileUtils = require("file/FileUtils"),
Async = require("utils/Async"),
UrlParams = require("utils/UrlParams").UrlParams;

// default async initExtension timeout
var INIT_EXTENSION_TIMEOUT = 10000;
Expand Down Expand Up @@ -311,15 +312,23 @@ define(function (require, exports, module) {
* @return {!$.Promise} A promise object that is resolved when all extensions complete loading.
*/
function init(paths) {
var params = new UrlParams();

if (_init) {
// Only init once. Return a resolved promise.
return new $.Deferred().resolve().promise();
}

if (!paths) {
paths = ["default", "dev", getUserExtensionPath()];
params.parse();

if (params.get("reloadWithoutUserExts") === "true") {
paths = ["default"];
} else {
paths = ["default", "dev", getUserExtensionPath()];
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This block of code assumes that the only possible values for the paths array is ["default", "dev", getUserExtensionPath()] or ["default"]. This seems to currently be the case, but this code would be more robust if it were to remove the "dev" and getUserExtensionPath() elements instead.

Copy link
Member

Choose a reason for hiding this comment

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

Other than unit tests, the paths argument is always null -- so the standard list is already hardcoded right here on line 323. So one alternative would be to just move this if up into the !paths case (i.e. if a list of paths is explicitly provided by unit tests, we ignore the reloadWithoutUserExts flag and use the list verbatim; the flag would only be respected when using the default set of paths.

I like that a little better since it's actually hard to guess what the correct response is when the array contains unexpected additional elements -- are those elements more similar to "default" (i.e. we should still load them even in safe mode) or are they more similar to "dev"/"user" (we shouldn't load them)? Impossible to say if we don't know anything about them...

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that is good enough for now, and it's a simpler fix.


// Load extensions before restoring the project

// Get a Directory for the user extension directory and create it if it doesn't exist.
Expand Down