-
Notifications
You must be signed in to change notification settings - Fork 7.6k
Reload Without User Extensions #6334
Changes from 3 commits
9dff969
3e335ad
dacd9bb
7b40ba8
8182f60
d85f302
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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"); | ||
|
@@ -54,15 +57,16 @@ 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"; | ||
|
||
|
||
|
||
|
@@ -92,6 +96,114 @@ 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 | ||
* @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 handleFileReload(commandData) { | ||
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"); | ||
} | ||
|
||
if (href.indexOf("?") !== -1) { | ||
href = href.substring(0, href.indexOf("?")); | ||
} | ||
|
||
if (!params.isEmpty()) { | ||
href += "?" + params; | ||
} | ||
|
||
_browserReload(href); | ||
} | ||
|
||
function _handleReloadWithoutUserExts() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The "User Extensions Disabled" indicator in the status bar can only be seen if there is a file open in editor. To provide 1 more way to help us troubleshoot user problems, what do you think about also posting a message to console log when user extensions are disabled? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this is the JavaScriptey way to do it, but I'd rather see explicit conversions such as:
Same comment in previous function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. |
||
_browserReload(href); | ||
} | ||
|
||
function _handleNewBracketsWindow() { | ||
window.open(window.location.href); | ||
} | ||
|
||
function _handleShowPerfData() { | ||
var templateVars = { | ||
delimitedPerfData: PerfUtils.getDelimitedPerfData(), | ||
|
@@ -135,10 +247,6 @@ define(function (require, exports, module) { | |
}); | ||
} | ||
|
||
function _handleNewBracketsWindow() { | ||
window.open(window.location.href); | ||
} | ||
|
||
function _handleSwitchLanguage() { | ||
var stringsPath = FileUtils.getNativeBracketsDirectoryPath() + "/nls"; | ||
|
||
|
@@ -225,72 +333,15 @@ 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, handleFileReload); | ||
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); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see that this existed before you made any changes to this file, but functions are not named consistently -- some are preceded with underscores and some are not. Since extensions can not (yet, anyway) export functions, all functions are, by definition, private -- so there's no reason to precede names with underscores. I can go either way as long as it's consistent. Also, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does export There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I missed that one. It should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done. Removed underscores from all methods except |
||
|
||
// 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) | ||
|
@@ -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); | ||
|
@@ -327,4 +379,24 @@ 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 menu, toolbar, and status bar | ||
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); | ||
} else { | ||
CommandManager.get(Commands.FILE_EXTENSION_MANAGER).setEnabled(true); | ||
// Toolbar and status bar reload back to default states, no need to set | ||
} | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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", | ||
|
@@ -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", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @redmunds @lkcampbell 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I support @peterflynn in this. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lkcampbell We first had this: Then I changed it to this: 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", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -311,6 +312,8 @@ 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(); | ||
|
@@ -319,7 +322,13 @@ define(function (require, exports, module) { | |
if (!paths) { | ||
paths = ["default", "dev", getUserExtensionPath()]; | ||
} | ||
|
||
|
||
params.parse(); | ||
|
||
if (params.get("reloadWithoutUserExts") === "true") { | ||
paths = ["default"]; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Other than unit tests, the 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... There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.