From ec9b747c280cf2d2eb9254d9c9a0279fb1ba355c Mon Sep 17 00:00:00 2001 From: abose Date: Sun, 19 Jan 2025 20:28:35 +0530 Subject: [PATCH 1/2] fix: file recovery not working if file has comma in it. don't save not be treated as crash We should not show file recovery dialog if the user selected dont save in when exiting phcode Our integrity checks saves the file as `checksum,filecontent`. but if file content itself has a comma, out logic used to break due to improper `string.split(",")` usage. --- src/document/DocumentCommandHandlers.js | 17 ++++++++++---- .../NavigationAndHistory/FileRecovery.js | 23 +++++++++++++++---- .../Extn-NavigationAndHistory-integ-test.js | 16 ++++++------- 3 files changed, 39 insertions(+), 17 deletions(-) diff --git a/src/document/DocumentCommandHandlers.js b/src/document/DocumentCommandHandlers.js index a4e62a5910..773166668a 100644 --- a/src/document/DocumentCommandHandlers.js +++ b/src/document/DocumentCommandHandlers.js @@ -1616,6 +1616,7 @@ define(function (require, exports, module) { * @private - tracks our closing state if we get called again */ var _windowGoingAway = false; + let exitWaitPromises = []; /** * @private @@ -1634,12 +1635,15 @@ define(function (require, exports, module) { return CommandManager.execute(Commands.FILE_CLOSE_ALL, { promptOnly: true }) .done(function () { + exitWaitPromises = []; _windowGoingAway = true; // Give everyone a chance to save their state - but don't let any problems block // us from quitting try { - ProjectManager.trigger("beforeAppClose"); + // if someone wats to do any deferred tasks, they should add + // their promise to the wait promises list. + ProjectManager.trigger("beforeAppClose", exitWaitPromises); } catch (ex) { console.error(ex); } @@ -2016,10 +2020,13 @@ define(function (require, exports, module) { _isReloading = true; return CommandManager.execute(Commands.FILE_CLOSE_ALL, { promptOnly: true }).done(function () { + exitWaitPromises = []; // Give everyone a chance to save their state - but don't let any problems block // us from quitting try { - ProjectManager.trigger("beforeAppClose"); + // if someone wats to do any deferred tasks, they should add + // their promise to the wait promises list. + ProjectManager.trigger("beforeAppClose", exitWaitPromises); } catch (ex) { console.error(ex); } @@ -2037,7 +2044,8 @@ define(function (require, exports, module) { // Defer for a more successful reload - issue #11539 window.setTimeout(function () { - raceAgainstTime(window.PhStore.flushDB()) // wither wait for flush or time this out + exitWaitPromises.push(window.PhStore.flushDB()); + raceAgainstTime(Promise.all(exitWaitPromises)) // wither wait for flush or time this out .finally(()=>{ raceAgainstTime(_safeNodeTerminate(), 4000) .finally(()=>{ @@ -2210,7 +2218,8 @@ define(function (require, exports, module) { event.preventDefault(); _handleWindowGoingAway(null, closeSuccess=>{ console.log('close success: ', closeSuccess); - raceAgainstTime(_safeFlushDB()) + exitWaitPromises.push(_safeFlushDB()); + raceAgainstTime(Promise.all(exitWaitPromises)) .finally(()=>{ raceAgainstTime(_safeNodeTerminate()) .finally(()=>{ diff --git a/src/extensionsIntegrated/NavigationAndHistory/FileRecovery.js b/src/extensionsIntegrated/NavigationAndHistory/FileRecovery.js index de5d41cab3..70a563389a 100644 --- a/src/extensionsIntegrated/NavigationAndHistory/FileRecovery.js +++ b/src/extensionsIntegrated/NavigationAndHistory/FileRecovery.js @@ -162,20 +162,25 @@ define(function (require, exports, module) { if(!input){ return null; } - const parts = input.split(',', 2); + // Find the first comma + const firstCommaIndex = input.indexOf(','); - if (parts.length !== 2) { + // If there's no comma or it's at the very beginning, the input is invalid + if (firstCommaIndex === -1 || firstCommaIndex === 0) { return null; } + // Extract the parts + const expectedLengthPart = input.slice(0, firstCommaIndex); + const actualString = input.slice(firstCommaIndex + 1); + // Parse the length part (should be the first part before the comma) - const expectedLength = parseInt(parts[0], 10); + const expectedLength = parseInt(expectedLengthPart, 10); if (isNaN(expectedLength)) { return null; } - // The second part is the actual string after the comma - const actualString = parts[1]; + // The second part is the actual string after the commas if (actualString.length === expectedLength) { return actualString; } @@ -471,6 +476,14 @@ define(function (require, exports, module) { // for the startup project. So we call manually. projectOpened(null, currentProjectRoot); } + ProjectManager.on("beforeAppClose", (_evt, waitPromises)=>{ + // this is a safe exit, we should delete all restore data. + let exitProjectRoot = ProjectManager.getProjectRoot(); + if(exitProjectRoot) { + const restoreRoot = getProjectRestoreRoot(exitProjectRoot.fullPath); + waitPromises.push(silentlyRemoveDirectory(restoreRoot)); + } + }); } function init() { diff --git a/test/spec/Extn-NavigationAndHistory-integ-test.js b/test/spec/Extn-NavigationAndHistory-integ-test.js index 7f23d2dd4f..3c0c96b8f6 100644 --- a/test/spec/Extn-NavigationAndHistory-integ-test.js +++ b/test/spec/Extn-NavigationAndHistory-integ-test.js @@ -33,7 +33,6 @@ define(function (require, exports, module) { const tempRestorePath = SpecRunnerUtils.getTestRoot() + "/navigationTestsRestore/"; let FileViewController, // loaded from brackets.test - ProjectManager, // loaded from brackets.test; CommandManager, Commands, testWindow, @@ -57,7 +56,6 @@ define(function (require, exports, module) { brackets = testWindow.brackets; $ = testWindow.$; FileViewController = brackets.test.FileViewController; - ProjectManager = brackets.test.ProjectManager; CommandManager = brackets.test.CommandManager; Commands = brackets.test.Commands; EditorManager = brackets.test.EditorManager; @@ -76,7 +74,6 @@ define(function (require, exports, module) { afterAll(async function () { FileViewController = null; - ProjectManager = null; testWindow = null; brackets = null; await deletePath(testPath); @@ -124,13 +121,16 @@ define(function (require, exports, module) { } + // multiple , in file tested as we have , for `size,"content"` integrity checks + const unsavedTextFragment = "hello,this is ,a test"; + it("Should create restore folders and backup files", async function () { await initFileRestorer("file.js"); let projectRestorePath = testWindow._FileRecoveryExtensionForTests.getProjectRestoreRoot(testPath); // now edit a file so that its backup is created let editor = EditorManager.getActiveEditor(); - editor.document.setText("hello"); + editor.document.setText(unsavedTextFragment); await SpecRunnerUtils.waitTillPathExists(projectRestorePath.fullPath, true); await SpecRunnerUtils.waitTillPathExists(projectRestorePath.fullPath + "file.js", false); await closeSession(); @@ -142,7 +142,7 @@ define(function (require, exports, module) { // now edit a file so that its backup is created let editor = EditorManager.getActiveEditor(); - editor.document.setText("hello"); + editor.document.setText(unsavedTextFragment); await SpecRunnerUtils.waitTillPathExists(projectRestorePath.fullPath + "toDelete1/file.js", false); await awaitsForDone(CommandManager.execute(Commands.FILE_SAVE_ALL), "saving all file"); await SpecRunnerUtils.waitTillPathNotExists(projectRestorePath.fullPath + "toDelete1/file.js", false); @@ -154,7 +154,7 @@ define(function (require, exports, module) { let projectRestorePath = testWindow._FileRecoveryExtensionForTests.getProjectRestoreRoot(testPath); // now edit a file so that its backup is created - const unsavedText = "hello" + Math.random(); + const unsavedText = unsavedTextFragment + Math.random(); let editor = EditorManager.getActiveEditor(); editor.document.setText(unsavedText); await SpecRunnerUtils.waitTillPathExists(projectRestorePath.fullPath + "toDelete1/file.js", false); @@ -186,7 +186,7 @@ define(function (require, exports, module) { let projectRestorePath = testWindow._FileRecoveryExtensionForTests.getProjectRestoreRoot(testPath); // now edit a file so that its backup is created - const unsavedText = "hello" + Math.random(); + const unsavedText = unsavedTextFragment + Math.random(); let editor = EditorManager.getActiveEditor(); editor.document.setText(unsavedText); await SpecRunnerUtils.waitTillPathExists(projectRestorePath.fullPath + fileNameToRestore, false); @@ -343,7 +343,7 @@ define(function (require, exports, module) { await openFile(file3); await _expectNavButton(false, true, "nav forward disabled due to new file open"); } - + it("Should navigate back and forward between text files", async function () { await navigateResetStack(); await _validateNavForFiles("test.css", "test.html", "test.js"); From a0562f7693a690985e27cb0c1f2cb88271197f8e Mon Sep 17 00:00:00 2001 From: abose Date: Sun, 19 Jan 2025 20:49:33 +0530 Subject: [PATCH 2/2] chore: defaulting to bigger size for new project dialog for small screens for better UX --- .../Phoenix/html/new-project-template.html | 2 +- src/styles/brackets.less | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/extensionsIntegrated/Phoenix/html/new-project-template.html b/src/extensionsIntegrated/Phoenix/html/new-project-template.html index 529d020375..9e9dfb9baf 100644 --- a/src/extensionsIntegrated/Phoenix/html/new-project-template.html +++ b/src/extensionsIntegrated/Phoenix/html/new-project-template.html @@ -1,4 +1,4 @@ -
+
diff --git a/src/styles/brackets.less b/src/styles/brackets.less index c9e80cf975..7a23eee08c 100644 --- a/src/styles/brackets.less +++ b/src/styles/brackets.less @@ -3548,3 +3548,12 @@ label input { opacity: 1; } } + +.new-project-modal-dialog{ + background-color:#3C3F41; + width: 750px; + height: 570px; + display: flex; + flex-direction: column; + justify-content: space-between +}