After extension is updated/removed, reload Brackets instead of quitting #6487

lkcampbell · 2014-01-12T01:16:12Z

This PR is a fix for issue #6350. After extension is updated/removed, Brackets will automatically reload with the new extension changes in place.

…tting

lkcampbell · 2014-01-12T01:18:08Z

@dangoor and @njx, let me know what you want to do with those string constants. Keep them as is or change them.

TomMalbran · 2014-01-12T01:32:12Z

There is one issue with this. If you remove the Debug extension, then it will not work, and in Edge Code there is no Debug extension. Maybe it could be time to move the reload commands to the core code.

lkcampbell · 2014-01-12T01:37:17Z

@TomMalbran, good point and your solution is the only one that makes sense.

Bleh, I was hoping this one was going to be really easy. Oh well...back to the drawing board.

lkcampbell · 2014-01-12T01:47:55Z

@TomMalbran, since you are around tonight, any suggestions on where to move the Reload code?

TomMalbran · 2014-01-12T02:11:04Z

The menu items could probably stay in the Debug menu, just the code would need to be moved, but not sure where to move the code.

Moving the reload code would mean that we could also move the change language menu to Help or View.

redmunds · 2014-01-12T17:18:05Z

All of the "quit" commands are handled in DocumentCommandHandlers, so I think that's the best place to put the "reload" commands.

redmunds · 2014-01-12T17:19:28Z

src/nls/root/strings.js

@@ -402,23 +402,23 @@ define({
    "EXTENSION_ERROR"                      : "Extension error",
    "EXTENSION_KEYWORDS"                   : "Keywords",
    "EXTENSION_INSTALLED"                  : "Installed",
-    "EXTENSION_UPDATE_INSTALLED"           : "This extension update has been downloaded and will be installed when you quit {APP_NAME}.",
+    "EXTENSION_UPDATE_INSTALLED"           : "This extension update has been downloaded and will be installed after {APP_NAME} automatically reloads.",


I don't think that the word "automatically" is necessary in any of these messages.

lkcampbell · 2014-01-12T19:07:28Z

@redmunds, also, what do you think about changing the name from browserReload to appReload or applicationReload (seems a little long). Since we (hopefully) reload everything in the brackets-shell along with the browser window at this point, we are really reloading more than the browser when in native mode.

I also considered keeping the concept of browserReload around as well, but that makes the functionality platform-dependent. A browserReload in HTML mode is not the same as a browserReload in native mode. We should probably abstract up a level and call the whole thing appReload. The behavior is the same as closing and opening the application without the user having to do anything. Same on all platforms.

redmunds · 2014-01-12T19:16:27Z

what do you think about changing the name from browserReload to appReload

If you create a second window using Debug > New Brackets Window, the calling browserReload only reloads the current window, not entire app. So, I think browserReload is probably more accurate.

lkcampbell · 2014-01-12T19:49:38Z

@redmunds, DocumentCommandHandlers does not currently have a public API. Do you want me to keep it this way and register Reload and Reload Without Extensions through the CommandManager, or do you want me to add a new reloadAPI method to the module?

redmunds · 2014-01-12T20:46:02Z

Yes, register Reload and Reload Without Extensions through the CommandManager.

lkcampbell · 2014-01-12T23:45:56Z

src/nls/cs/strings.js

+    "CHANGE_AND_RELOAD_MESSAGE"            : "Pro aktualizaci nebo odstranění označených doplňků musíte ukončit a restartovat aplikaci {APP_NAME}. Budete vyzváni k uložení změn.",
+    "REMOVE_AND_RELOAD"                    : "Odstranit doplňky a ukončit program",
+    "CHANGE_AND_RELOAD"                    : "Změnit doplňky a ukončit program",
+    "UPDATE_AND_RELOAD"                    : "Aktualizovat doplňky a ukončit program",


@redmunds, I hope I did this correctly. I changed all of the language files. I don't know how the automated translation services work so, are you sure changing the constant names won't mess anything up? Will the service still be able to flag the strings that need updating even if we changed the constant names?

You should only edit the nls/root/strings.js file, so remove changes to all of the other nls/*/strings.js files.

FYI, for fr and ja, the automated system will remove the old ids, add the new ids, and then re-translate. All of the other languages must be updated manually. They will default to the root (English) string until then.

When we change something that is not a translation, many times we change all the languages besides the fr and ja, so that they work, since several translations are not updated for every sprint.

Yes, I forgot to mention that other possibility: do a quick translation using something such as Google Translate.

FYI, we have an internal event going on today, and last day of sprint was pushed to tomorrow, so I may not get to reviewing this today.

I didn't noticed that the string also changed, so it might be better to not touch it, unless we are sure that the Google translations are accurate enough.

lkcampbell · 2014-01-12T23:48:21Z

@redmunds, updates committed.

redmunds · 2014-01-13T01:05:21Z

src/document/DocumentCommandHandlers.js

@@ -23,11 +23,13 @@


 /*jslint vars: true, plusplus: true, devel: true, nomen: true, indent: 4, maxerr: 50, regexp: true */
-/*global define, $, brackets, window */
+/*global define, $, brackets, window, WebSocket */


I don't think this and a few other changes to this file were not made by you, so it looks like a bad merge.

Nevermind -- I see you had to also move the disableCache() code.

You should remove ~~"Mustache"~~ "WebSocket" from the jslint global define list in DebugCommands/main.js.

I think you mean "WebSocket" since "Mustache" is still required in DebugCommands.

You mean "WebSocket", correct?

Yes, "WebSocket".

lkcampbell · 2014-01-13T02:22:00Z

@redmunds, incorrect changes rolled back.

lkcampbell · 2014-01-13T13:03:42Z

@redmunds, code review changes committed.

redmunds · 2014-01-13T23:47:04Z

src/document/DocumentCommandHandlers.js

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


Doesn't seem like there's any benefit to putting "status-user-exts" in a local variable since it's only used once.

Oh, I see it was already like this and you just moved it, so I'll leave it to your discretion.

No, that's all me, old file and new file. I wrote it like that out of force of habit, throwing hard-wired strings into a constant. I can change it.

redmunds · 2014-01-14T00:21:39Z

There are 2 ExtensionManager unit tests that no longer pass:

should remove extensions and quit if the user hits Remove and Quit on the removal confirmation dialog
should update extensions and quit if the user hits Update and Quit on the removal confirmation dialog

lkcampbell · 2014-01-14T03:24:11Z

@redmunds, code review changes committed.

redmunds · 2014-01-14T16:45:47Z

Looks good. Merging.

After extension is updated/removed, reload Brackets instead of quitting

After an extension is updated/removed, reload Brackets instead of qui…

7875aa7

…tting

redmunds reviewed Jan 12, 2014
View reviewed changes

lkcampbell added 2 commits January 12, 2014 15:06

Small cleanups. Move reload functions to DocumentCommandHandlers.

7d8ce45

Small cleanups. Update string constant names.

a4347e5

lkcampbell reviewed Jan 12, 2014
View reviewed changes

redmunds reviewed Jan 13, 2014
View reviewed changes

Roll back incorrect changes on translation strings

f86246f

Small code clean up

5adb2ea

redmunds reviewed Jan 13, 2014
View reviewed changes

Update ExtensionManager unit tests and small clean up

a583cd4

ghost assigned redmunds Jan 14, 2014

redmunds added a commit that referenced this pull request Jan 14, 2014

Merge pull request #6487 from lkcampbell/fix-6350

1c026e1

After extension is updated/removed, reload Brackets instead of quitting

redmunds merged commit 1c026e1 into adobe:master Jan 14, 2014

lkcampbell deleted the fix-6350 branch January 14, 2014 16:54

core-ai-bot mentioned this pull request Aug 30, 2021

[CLOSED] After extension is updated/removed, reload Brackets instead of quitting brackets-archive/bracketsIssues#5895

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

After extension is updated/removed, reload Brackets instead of quitting #6487

After extension is updated/removed, reload Brackets instead of quitting #6487

lkcampbell commented Jan 12, 2014

lkcampbell commented Jan 12, 2014

TomMalbran commented Jan 12, 2014

lkcampbell commented Jan 12, 2014

lkcampbell commented Jan 12, 2014

TomMalbran commented Jan 12, 2014

redmunds commented Jan 12, 2014

redmunds Jan 12, 2014

lkcampbell commented Jan 12, 2014

redmunds commented Jan 12, 2014

lkcampbell commented Jan 12, 2014

redmunds commented Jan 12, 2014

lkcampbell Jan 12, 2014

redmunds Jan 13, 2014

TomMalbran Jan 13, 2014

redmunds Jan 13, 2014

TomMalbran Jan 13, 2014

lkcampbell commented Jan 12, 2014

redmunds Jan 13, 2014

redmunds Jan 13, 2014

redmunds Jan 13, 2014

TomMalbran Jan 13, 2014

lkcampbell Jan 13, 2014

redmunds Jan 13, 2014

lkcampbell commented Jan 13, 2014

lkcampbell commented Jan 13, 2014

redmunds Jan 13, 2014

redmunds Jan 14, 2014

lkcampbell Jan 14, 2014

redmunds commented Jan 14, 2014

lkcampbell commented Jan 14, 2014

redmunds commented Jan 14, 2014

After extension is updated/removed, reload Brackets instead of quitting #6487

After extension is updated/removed, reload Brackets instead of quitting #6487

Conversation

lkcampbell commented Jan 12, 2014

lkcampbell commented Jan 12, 2014

TomMalbran commented Jan 12, 2014

lkcampbell commented Jan 12, 2014

lkcampbell commented Jan 12, 2014

TomMalbran commented Jan 12, 2014

redmunds commented Jan 12, 2014

Choose a reason for hiding this comment

lkcampbell commented Jan 12, 2014

redmunds commented Jan 12, 2014

lkcampbell commented Jan 12, 2014

redmunds commented Jan 12, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lkcampbell commented Jan 12, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lkcampbell commented Jan 13, 2014

lkcampbell commented Jan 13, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

redmunds commented Jan 14, 2014

lkcampbell commented Jan 14, 2014

redmunds commented Jan 14, 2014