From f6837d1f9790bb3a178733b7fc02433ac7041582 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 10:19:01 -0500 Subject: [PATCH 1/6] openLocations() flag to require a path to be an existing directory --- spec/atom-environment-spec.js | 15 +++++++++++++++ src/atom-environment.js | 4 ++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/spec/atom-environment-spec.js b/spec/atom-environment-spec.js index d90001205..991524853 100644 --- a/spec/atom-environment-spec.js +++ b/spec/atom-environment-spec.js @@ -669,6 +669,21 @@ describe('AtomEnvironment', () => { expect(atom.workspace.getTextEditors().map(e => e.getPath())).toEqual([pathToOpen]) expect(atom.project.getPaths()).toEqual([]) }) + + it('may be required to be an existing directory', async () => { + const nonExistent = path.join(__dirname, 'no') + const existingFile = __filename + const existingDir = path.join(__dirname, 'fixtures') + + await atom.openLocations([ + {pathToOpen: nonExistent, mustBeDirectory: true}, + {pathToOpen: existingFile, mustBeDirectory: true}, + {pathToOpen: existingDir, mustBeDirectory: true} + ]) + + expect(atom.workspace.getTextEditors()).toEqual([]) + expect(atom.project.getPaths()).toEqual([existingDir]) + }) }) describe('when the opened path is handled by a registered directory provider', () => { diff --git a/src/atom-environment.js b/src/atom-environment.js index 03871ab74..4e3d7fb30 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -1386,7 +1386,7 @@ or use Pane::saveItemAs for programmatic saving.`) if (stats.isDirectory()) { // Directory: add as a project folder foldersToAddToProject.add(this.project.getDirectoryForProjectPath(pathToOpen).getPath()) - } else if (stats.isFile()) { + } else if (stats.isFile() && !location.mustBeDirectory) { // File: add as a file location fileLocationsToOpen.push(location) } @@ -1397,7 +1397,7 @@ or use Pane::saveItemAs for programmatic saving.`) if (directory) { // Found: add as a project folder foldersToAddToProject.add(directory.getPath()) - } else { + } else if (!location.mustBeDirectory) { // Not found: open as a new file fileLocationsToOpen.push(location) } From 02e1ae4b0ff34673913fc301b33e263ccf7c6ec3 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 10:39:50 -0500 Subject: [PATCH 2/6] Notify about missing project folders --- spec/atom-environment-spec.js | 7 ++++++ src/atom-environment.js | 43 +++++++++++++++++++++++++++++++---- 2 files changed, 46 insertions(+), 4 deletions(-) diff --git a/spec/atom-environment-spec.js b/spec/atom-environment-spec.js index 991524853..4fdae6c3c 100644 --- a/spec/atom-environment-spec.js +++ b/spec/atom-environment-spec.js @@ -671,6 +671,8 @@ describe('AtomEnvironment', () => { }) it('may be required to be an existing directory', async () => { + spyOn(atom.notifications, 'addWarning') + const nonExistent = path.join(__dirname, 'no') const existingFile = __filename const existingDir = path.join(__dirname, 'fixtures') @@ -683,6 +685,11 @@ describe('AtomEnvironment', () => { expect(atom.workspace.getTextEditors()).toEqual([]) expect(atom.project.getPaths()).toEqual([existingDir]) + + expect(atom.notifications.addWarning).toHaveBeenCalledWith( + 'Unable to open project folders', + {description: `The directories \`${nonExistent}\` and \`${existingFile}\` do not exist.`} + ) }) }) diff --git a/src/atom-environment.js b/src/atom-environment.js index 4e3d7fb30..e53d2b936 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -1364,6 +1364,7 @@ or use Pane::saveItemAs for programmatic saving.`) const needsProjectPaths = this.project && this.project.getPaths().length === 0 const foldersToAddToProject = new Set() const fileLocationsToOpen = [] + const missingFolders = [] // Asynchronously fetch stat information about each requested path to open. const locationStats = await Promise.all( @@ -1386,9 +1387,13 @@ or use Pane::saveItemAs for programmatic saving.`) if (stats.isDirectory()) { // Directory: add as a project folder foldersToAddToProject.add(this.project.getDirectoryForProjectPath(pathToOpen).getPath()) - } else if (stats.isFile() && !location.mustBeDirectory) { - // File: add as a file location - fileLocationsToOpen.push(location) + } else if (stats.isFile()) { + if (!location.mustBeDirectory) { + // File: add as a file location + fileLocationsToOpen.push(location) + } else { + missingFolders.push(location) + } } } else { // Path does not exist @@ -1397,7 +1402,10 @@ or use Pane::saveItemAs for programmatic saving.`) if (directory) { // Found: add as a project folder foldersToAddToProject.add(directory.getPath()) - } else if (!location.mustBeDirectory) { + } else if (location.mustBeDirectory) { + // Not found and must be a directory: add to missing list + missingFolders.push(location) + } else { // Not found: open as a new file fileLocationsToOpen.push(location) } @@ -1430,6 +1438,33 @@ or use Pane::saveItemAs for programmatic saving.`) await Promise.all(fileOpenPromises) } + if (missingFolders.length > 0) { + let message = 'Unable to open project folder' + if (missingFolders.length > 1) { + message += 's' + } + + let description = 'The ' + if (missingFolders.length === 1) { + description += 'directory `' + description += missingFolders[0].pathToOpen + description += '` does not exist.' + } else if (missingFolders.length === 2) { + description += `directories \`${missingFolders[0].pathToOpen}\` ` + description += `and \`${missingFolders[1].pathToOpen}\` do not exist.` + } else { + description += 'directories ' + description += (missingFolders + .slice(0, -1) + .map(location => location.pathToOpen) + .map(pathToOpen => '`' + pathToOpen + '`, ') + .join('')) + description += 'and `' + missingFolders[missingFolders.length - 1].pathToOpen + '` do not exist.' + } + + this.notifications.addWarning(message, {description}) + } + ipcRenderer.send('window-command', 'window:locations-opened') } From 1ecff536c7ad683dbcdbe082190ca4b298b93b46 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 11:07:00 -0500 Subject: [PATCH 3/6] Pass persisted window sessions as foldersToOpen --- src/main-process/atom-application.js | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/main-process/atom-application.js b/src/main-process/atom-application.js index 04f7fba5c..072981fe6 100644 --- a/src/main-process/atom-application.js +++ b/src/main-process/atom-application.js @@ -210,6 +210,7 @@ class AtomApplication extends EventEmitter { const { pathsToOpen, executedFrom, + foldersToOpen, urlsToOpen, benchmark, benchmarkTest, @@ -248,9 +249,10 @@ class AtomApplication extends EventEmitter { timeout, env }) - } else if (pathsToOpen.length > 0) { + } else if (pathsToOpen.length > 0 || foldersToOpen.length > 0) { return this.openPaths({ pathsToOpen, + foldersToOpen, executedFrom, pidToKillWhenClosed, devMode, @@ -806,6 +808,7 @@ class AtomApplication extends EventEmitter { // // options - // :pathsToOpen - The array of file paths to open + // :foldersToOpen - An array of additional paths to open that must be existing directories // :pidToKillWhenClosed - The integer of the pid to kill // :devMode - Boolean to control the opened window's dev mode. // :safeMode - Boolean to control the opened window's safe mode. @@ -814,6 +817,7 @@ class AtomApplication extends EventEmitter { // :addToLastWindow - Boolean of whether this should be opened in last focused window. openPaths ({ pathsToOpen, + foldersToOpen, executedFrom, pidToKillWhenClosed, devMode, @@ -825,8 +829,10 @@ class AtomApplication extends EventEmitter { addToLastWindow, env } = {}) { - if (!pathsToOpen || pathsToOpen.length === 0) return if (!env) env = process.env + if (!pathsToOpen) pathsToOpen = [] + if (!foldersToOpen) foldersToOpen = [] + devMode = Boolean(devMode) safeMode = Boolean(safeMode) clearWindowState = Boolean(clearWindowState) @@ -837,6 +843,22 @@ class AtomApplication extends EventEmitter { hasWaitSession: pidToKillWhenClosed != null }) }) + + for (const folderToOpen of foldersToOpen) { + locationsToOpen.push({ + pathToOpen: folderToOpen, + initialLine: null, + initialColumn: null, + mustBeDirectory: true, + forceAddToWindow: addToLastWindow, + hasWaitSession: pidToKillWhenClosed != null + }) + } + + if (locationsToOpen.length === 0) { + return + } + const normalizedPathsToOpen = locationsToOpen.map(location => location.pathToOpen).filter(Boolean) let existingWindow @@ -966,7 +988,7 @@ class AtomApplication extends EventEmitter { const states = await this.storageFolder.load('application.json') if (states) { return states.map(state => ({ - pathsToOpen: state.initialPaths, + foldersToOpen: state.initialPaths, urlsToOpen: [], devMode: this.devMode, safeMode: this.safeMode From 07dc40362fde258060d1e2ca142749c4a4e9efc1 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 11:32:39 -0500 Subject: [PATCH 4/6] Flip conditional for clarity --- src/atom-environment.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/atom-environment.js b/src/atom-environment.js index e53d2b936..bbf8311d7 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -1388,11 +1388,12 @@ or use Pane::saveItemAs for programmatic saving.`) // Directory: add as a project folder foldersToAddToProject.add(this.project.getDirectoryForProjectPath(pathToOpen).getPath()) } else if (stats.isFile()) { - if (!location.mustBeDirectory) { + if (location.mustBeDirectory) { + // File: no longer a directory + missingFolders.push(location) + } else { // File: add as a file location fileLocationsToOpen.push(location) - } else { - missingFolders.push(location) } } } else { From 88d7c6dbdd65c7483e9884e6de16b61e94741c98 Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 11:33:44 -0500 Subject: [PATCH 5/6] Include now-missing project folders in initial state key computation --- spec/atom-environment-spec.js | 21 +++++++++++++++++++++ src/atom-environment.js | 10 +++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/spec/atom-environment-spec.js b/spec/atom-environment-spec.js index 4fdae6c3c..5e9617374 100644 --- a/spec/atom-environment-spec.js +++ b/spec/atom-environment-spec.js @@ -742,6 +742,27 @@ describe('AtomEnvironment', () => { expect(atom.project.getPaths()).toEqual([]) }) + it('includes missing mandatory project folders in computation of initial state key', async () => { + const existingDir = path.join(__dirname, 'fixtures') + const missingDir = path.join(__dirname, 'no') + + atom.loadState.andCallFake(function (key) { + if (key === `${existingDir}:${missingDir}`) { + return Promise.resolve(state) + } else { + return Promise.resolve(null) + } + }) + + await atom.openLocations([ + {pathToOpen: existingDir}, + {pathToOpen: missingDir, mustBeDirectory: true} + ]) + + expect(atom.attemptRestoreProjectStateForPaths).toHaveBeenCalledWith(state, [existingDir], []) + expect(atom.project.getPaths(), [existingDir]) + }) + it('opens the specified files', async () => { await atom.openLocations([{pathToOpen: __dirname}, {pathToOpen: __filename}]) expect(atom.attemptRestoreProjectStateForPaths).toHaveBeenCalledWith(state, [__dirname], [__filename]) diff --git a/src/atom-environment.js b/src/atom-environment.js index bbf8311d7..98a89fe45 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -1404,7 +1404,7 @@ or use Pane::saveItemAs for programmatic saving.`) // Found: add as a project folder foldersToAddToProject.add(directory.getPath()) } else if (location.mustBeDirectory) { - // Not found and must be a directory: add to missing list + // Not found and must be a directory: add to missing list and use to derive state key missingFolders.push(location) } else { // Not found: open as a new file @@ -1416,8 +1416,12 @@ or use Pane::saveItemAs for programmatic saving.`) } let restoredState = false - if (foldersToAddToProject.size > 0) { - const state = await this.loadState(this.getStateKey(Array.from(foldersToAddToProject))) + if (foldersToAddToProject.size > 0 || missingFolders.length > 0) { + // Include missing folders in the state key so that sessions restored with no-longer-present project root folders + // don't lose data. + const foldersForStateKey = Array.from(foldersToAddToProject) + .concat(missingFolders.map(location => location.pathToOpen)) + const state = await this.loadState(this.getStateKey(Array.from(foldersForStateKey))) // only restore state if this is the first path added to the project if (state && needsProjectPaths) { From f68a511a9000e27477f12552338af5474ab839ae Mon Sep 17 00:00:00 2001 From: Ash Wilson Date: Thu, 24 Jan 2019 15:17:26 -0500 Subject: [PATCH 6/6] pathsToOpen and foldersToOpen may be undefined --- src/main-process/atom-application.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main-process/atom-application.js b/src/main-process/atom-application.js index 072981fe6..f6ada6137 100644 --- a/src/main-process/atom-application.js +++ b/src/main-process/atom-application.js @@ -249,7 +249,7 @@ class AtomApplication extends EventEmitter { timeout, env }) - } else if (pathsToOpen.length > 0 || foldersToOpen.length > 0) { + } else if ((pathsToOpen && pathsToOpen.length > 0) || (foldersToOpen && foldersToOpen.length > 0)) { return this.openPaths({ pathsToOpen, foldersToOpen,