From c61eb62b06d9a9d1c21c1df11e16f27ff9eec868 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Fri, 17 Nov 2017 23:32:07 +0100 Subject: [PATCH 01/20] Add async version of atom.confirm --- src/application-delegate.coffee | 44 ++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/src/application-delegate.coffee b/src/application-delegate.coffee index 70b0f91bc..bfa94556c 100644 --- a/src/application-delegate.coffee +++ b/src/application-delegate.coffee @@ -130,26 +130,36 @@ class ApplicationDelegate getUserDefault: (key, type) -> remote.systemPreferences.getUserDefault(key, type) - confirm: ({message, detailedMessage, buttons}) -> - buttons ?= {} - if Array.isArray(buttons) - buttonLabels = buttons + confirm: ({message, detailedMessage, buttons}, callback) -> + if typeof callback is 'function' + # Async version: buttons is required to be an array + remote.dialog.showMessageBox(remote.getCurrentWindow(), { + type: 'info' + message: message + detail: detailedMessage + buttons: buttons + normalizeAccessKeys: true + }, callback) else - buttonLabels = Object.keys(buttons) + buttons ?= {} + if Array.isArray(buttons) + buttonLabels = buttons + else + buttonLabels = Object.keys(buttons) - chosen = remote.dialog.showMessageBox(remote.getCurrentWindow(), { - type: 'info' - message: message - detail: detailedMessage - buttons: buttonLabels - normalizeAccessKeys: true - }) + chosen = remote.dialog.showMessageBox(remote.getCurrentWindow(), { + type: 'info' + message: message + detail: detailedMessage + buttons: buttonLabels + normalizeAccessKeys: true + }) - if Array.isArray(buttons) - chosen - else - callback = buttons[buttonLabels[chosen]] - callback?() + if Array.isArray(buttons) + chosen + else + callback = buttons[buttonLabels[chosen]] + callback?() showMessageDialog: (params) -> From 47963ef2e531689c0823e57ebf5409665c5b29b7 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Fri, 17 Nov 2017 23:32:53 +0100 Subject: [PATCH 02/20] And test it with editor:checkout-head-revision --- src/workspace.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/src/workspace.js b/src/workspace.js index defb43df0..8de7adc65 100644 --- a/src/workspace.js +++ b/src/workspace.js @@ -1990,25 +1990,22 @@ module.exports = class Workspace extends Model { checkoutHeadRevision (editor) { if (editor.getPath()) { - const checkoutHead = () => { - return this.project.repositoryForDirectory(new Directory(editor.getDirectoryPath())) - .then(repository => repository && repository.checkoutHeadForEditor(editor)) + const checkoutHead = async () => { + const repository = await this.project.repositoryForDirectory(new Directory(editor.getDirectoryPath())) + if (repository) repository.checkoutHeadForEditor(editor) } if (this.config.get('editor.confirmCheckoutHeadRevision')) { this.applicationDelegate.confirm({ message: 'Confirm Checkout HEAD Revision', detailedMessage: `Are you sure you want to discard all changes to "${editor.getFileName()}" since the last Git commit?`, - buttons: { - OK: checkoutHead, - Cancel: null - } + buttons: ['OK', 'Cancel'] + }, response => { + if (response === 0) checkoutHead() }) } else { - return checkoutHead() + checkoutHead() } - } else { - return Promise.resolve(false) } } } From 1401c58e8e6d3e73b4815607537e814762717b76 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Fri, 17 Nov 2017 23:43:35 +0100 Subject: [PATCH 03/20] Document async atom.confirm --- src/atom-environment.js | 53 +++++++++++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 12 deletions(-) diff --git a/src/atom-environment.js b/src/atom-environment.js index 663bb6c00..93e6e865e 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -911,29 +911,58 @@ class AtomEnvironment { // Essential: A flexible way to open a dialog akin to an alert dialog. // + // While both async and sync versions are provided, it is recommended to use the async version + // such that the renderer process is not blocked while the dialog box is open. + // // If the dialog is closed (via `Esc` key or `X` in the top corner) without selecting a button // the first button will be clicked unless a "Cancel" or "No" button is provided. // // ## Examples // - // ```coffee - // atom.confirm - // message: 'How you feeling?' - // detailedMessage: 'Be honest.' - // buttons: - // Good: -> window.alert('good to hear') - // Bad: -> window.alert('bummer') + // ```js + // // Async version (recommended) + // atom.confirm({ + // message: 'How you feeling?', + // detailedMessage: 'Be honest.', + // buttons: ['Good', 'Bad'] + // }, response => { + // if (response === 0) { + // window.alert('good to hear') + // } else { + // window.alert('bummer') + // } + // }) + // + // ```js + // // Sync version + // const chosen = atom.confirm({ + // message: 'How you feeling?', + // detailedMessage: 'Be honest.', + // buttons: { + // Good: () => window.alert('good to hear'), + // Bad: () => window.alert('bummer') + // } + // }) // ``` // // * `options` An {Object} with the following keys: // * `message` The {String} message to display. // * `detailedMessage` (optional) The {String} detailed message to display. - // * `buttons` (optional) Either an array of strings or an object where keys are - // button names and the values are callbacks to invoke when clicked. + // * `buttons` (optional) Either an {Array} of {String}s or an {Object} where keys are + // button names and the values are callback {Function}s to invoke when clicked. + // * `callback` (optional) A {Function} that will be called with the index of the chosen option. + // If a callback is supplied, `buttons` (if supplied) must be an {Array}, + // and the renderer process will not be paused while the dialog box is open. // - // Returns the chosen button index {Number} if the buttons option is an array or the return value of the callback if the buttons option is an object. - confirm (params = {}) { - return this.applicationDelegate.confirm(params) + // Returns the chosen button index {Number} if the buttons option is an array + // or the return value of the callback if the buttons option is an object. + confirm (params = {}, callback) { + if (callback) { + // Async: no return value + this.applicationDelegate.confirm(params, callback) + } else { + return this.applicationDelegate.confirm(params) + } } /* From efdba69b99d2c5edcf5eaf41ea45541d3fde5978 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Fri, 17 Nov 2017 23:44:09 +0100 Subject: [PATCH 04/20] Assume that if callback exists it is a function --- src/application-delegate.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/application-delegate.coffee b/src/application-delegate.coffee index bfa94556c..99463455d 100644 --- a/src/application-delegate.coffee +++ b/src/application-delegate.coffee @@ -131,7 +131,7 @@ class ApplicationDelegate remote.systemPreferences.getUserDefault(key, type) confirm: ({message, detailedMessage, buttons}, callback) -> - if typeof callback is 'function' + if callback? # Async version: buttons is required to be an array remote.dialog.showMessageBox(remote.getCurrentWindow(), { type: 'info' From 3591f17738636f07cf49246983de00b813f4a3ed Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Fri, 17 Nov 2017 23:45:26 +0100 Subject: [PATCH 05/20] Convert CommandInstaller dialogs to async versions --- src/command-installer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/command-installer.js b/src/command-installer.js index 2c032d6c5..225547ef4 100644 --- a/src/command-installer.js +++ b/src/command-installer.js @@ -24,7 +24,7 @@ class CommandInstaller { this.applicationDelegate.confirm({ message: 'Failed to install shell commands', detailedMessage: error.message - }) + }, () => {}) } this.installAtomCommand(true, error => { @@ -34,7 +34,7 @@ class CommandInstaller { this.applicationDelegate.confirm({ message: 'Commands installed.', detailedMessage: 'The shell commands `atom` and `apm` are installed.' - }) + }, () => {}) }) }) } From c3bc4973f9f34d108570d4b94dcb69f5ffc75101 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sat, 18 Nov 2017 00:21:40 +0100 Subject: [PATCH 06/20] Convert promptToSaveItem --- src/pane.js | 94 +++++++++++++++++++++++++---------------------------- 1 file changed, 45 insertions(+), 49 deletions(-) diff --git a/src/pane.js b/src/pane.js index 0305b39dd..502c1e125 100644 --- a/src/pane.js +++ b/src/pane.js @@ -790,57 +790,53 @@ class Pane { } promptToSaveItem (item, options = {}) { - if (typeof item.shouldPromptToSave !== 'function' || !item.shouldPromptToSave(options)) { - return Promise.resolve(true) - } - - let uri - if (typeof item.getURI === 'function') { - uri = item.getURI() - } else if (typeof item.getUri === 'function') { - uri = item.getUri() - } else { - return Promise.resolve(true) - } - - const title = (typeof item.getTitle === 'function' && item.getTitle()) || uri - - const saveDialog = (saveButtonText, saveFn, message) => { - const chosen = this.applicationDelegate.confirm({ - message, - detailedMessage: 'Your changes will be lost if you close this item without saving.', - buttons: [saveButtonText, 'Cancel', "&Don't Save"]} - ) - - switch (chosen) { - case 0: - return new Promise(resolve => { - return saveFn(item, error => { - if (error instanceof SaveCancelledError) { - resolve(false) - } else if (error) { - saveDialog( - 'Save as', - this.saveItemAs, - `'${title}' could not be saved.\nError: ${this.getMessageForErrorCode(error.code)}` - ).then(resolve) - } else { - resolve(true) - } - }) - }) - case 1: - return Promise.resolve(false) - case 2: - return Promise.resolve(true) + return new Promise((resolve, reject) => { + if (typeof item.shouldPromptToSave !== 'function' || !item.shouldPromptToSave(options)) { + return resolve(true) } - } - return saveDialog( - 'Save', - this.saveItem, - `'${title}' has changes, do you want to save them?` - ) + let uri + if (typeof item.getURI === 'function') { + uri = item.getURI() + } else if (typeof item.getUri === 'function') { + uri = item.getUri() + } else { + return resolve(true) + } + + const title = (typeof item.getTitle === 'function' && item.getTitle()) || uri + + const saveDialog = (saveButtonText, saveFn, message) => { + this.applicationDelegate.confirm({ + message, + detailedMessage: 'Your changes will be lost if you close this item without saving.', + buttons: [saveButtonText, 'Cancel', "&Don't Save"] + }, response => { + switch (response) { + case 0: + return saveFn(item, error => { + if (error instanceof SaveCancelledError) { + resolve(false) + } else if (error) { + saveDialog( + 'Save as', + this.saveItemAs, + `'${title}' could not be saved.\nError: ${this.getMessageForErrorCode(error.code)}` + ) + } else { + resolve(true) + } + }) + case 1: + return resolve(false) + case 2: + return resolve(true) + } + }) + } + + saveDialog('Save', this.saveItem, `'${title}' has changes, do you want to save them?`) + }) } // Public: Save the active item. From 8f81831ad4f8f5ef4aa5e8f79aa51eb67d41f56d Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sat, 18 Nov 2017 00:56:22 +0100 Subject: [PATCH 07/20] Convert large file warning --- src/workspace.js | 47 +++++++++++++++++++++++++++++++---------------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/src/workspace.js b/src/workspace.js index 8de7adc65..8e9d3b2df 100644 --- a/src/workspace.js +++ b/src/workspace.js @@ -1158,16 +1158,17 @@ module.exports = class Workspace extends Model { // * `uri` A {String} containing a URI. // // Returns a {Promise} that resolves to the {TextEditor} (or other item) for the given URI. - createItemForURI (uri, options) { + async createItemForURI (uri, options) { if (uri != null) { - for (let opener of this.getOpeners()) { + for (const opener of this.getOpeners()) { const item = opener(uri, options) - if (item != null) return Promise.resolve(item) + if (item != null) return item } } try { - return this.openTextFile(uri, options) + const item = await this.openTextFile(uri, options) + return item } catch (error) { switch (error.code) { case 'CANCELLED': @@ -1197,7 +1198,7 @@ module.exports = class Workspace extends Model { } } - openTextFile (uri, options) { + async openTextFile (uri, options) { const filePath = this.project.resolvePath(uri) if (filePath != null) { @@ -1214,23 +1215,37 @@ module.exports = class Workspace extends Model { const fileSize = fs.getSizeSync(filePath) const largeFileMode = fileSize >= (2 * 1048576) // 2MB - if (fileSize >= (this.config.get('core.warnOnLargeFileLimit') * 1048576)) { // 20MB by default - const choice = this.applicationDelegate.confirm({ + + let resolveConfirmFileOpenPromise, rejectConfirmFileOpenPromise = [] + const confirmFileOpenPromise = new Promise((resolve, reject) => { + resolveConfirmFileOpenPromise = resolve + rejectConfirmFileOpenPromise = reject + }) + if (fileSize >= (this.config.get('core.warnOnLargeFileLimit') * 1048576)) { // 40MB by default + this.applicationDelegate.confirm({ message: 'Atom will be unresponsive during the loading of very large files.', detailedMessage: 'Do you still want to load this file?', buttons: ['Proceed', 'Cancel'] + }, response => { + if (response === 1) { + rejectConfirmFileOpenPromise() + } else { + resolveConfirmFileOpenPromise() + } }) - if (choice === 1) { - const error = new Error() - error.code = 'CANCELLED' - throw error - } + } else { + resolveConfirmFileOpenPromise() } - return this.project.bufferForPath(filePath, options) - .then(buffer => { - return this.textEditorRegistry.build(Object.assign({buffer, largeFileMode, autoHeight: false}, options)) - }) + try { + await confirmFileOpenPromise + const buffer = await this.project.bufferForPath(filePath, options) + return this.textEditorRegistry.build(Object.assign({buffer, largeFileMode, autoHeight: false}, options)) + } catch (e) { + const error = new Error() + error.code = 'CANCELLED' + throw error + } } handleGrammarUsed (grammar) { From b6a6961f4531e076fa4911a1c1ea668090bcd84d Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sat, 18 Nov 2017 01:22:58 +0100 Subject: [PATCH 08/20] Rework async version to pass all options to Electron for future-compat --- src/application-delegate.coffee | 16 +++++++--------- src/atom-environment.js | 21 +++++++++++++-------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/application-delegate.coffee b/src/application-delegate.coffee index 99463455d..be17f1863 100644 --- a/src/application-delegate.coffee +++ b/src/application-delegate.coffee @@ -130,17 +130,15 @@ class ApplicationDelegate getUserDefault: (key, type) -> remote.systemPreferences.getUserDefault(key, type) - confirm: ({message, detailedMessage, buttons}, callback) -> + confirm: (options, callback) -> if callback? - # Async version: buttons is required to be an array - remote.dialog.showMessageBox(remote.getCurrentWindow(), { - type: 'info' - message: message - detail: detailedMessage - buttons: buttons - normalizeAccessKeys: true - }, callback) + # Async version: pass options directly to Electron but set sane defaults + options = Object.assign({type: 'info', normalizeAccessKeys: true}, options) + remote.dialog.showMessageBox(remote.getCurrentWindow(), options, callback) else + # Legacy sync version: options can only have `message`, + # `detailedMessage` (optional), and buttons array or object (optional) + {message, detailedMessage, buttons} = options buttons ?= {} if Array.isArray(buttons) buttonLabels = buttons diff --git a/src/atom-environment.js b/src/atom-environment.js index 93e6e865e..8328f53bc 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -914,6 +914,9 @@ class AtomEnvironment { // While both async and sync versions are provided, it is recommended to use the async version // such that the renderer process is not blocked while the dialog box is open. // + // The async version accepts the same options as Electron's `dialog.showMessageBox`. + // For convenience, it sets `type` to `'info'` and `normalizeAccessKeys` to `true` by default. + // // If the dialog is closed (via `Esc` key or `X` in the top corner) without selecting a button // the first button will be clicked unless a "Cancel" or "No" button is provided. // @@ -923,7 +926,7 @@ class AtomEnvironment { // // Async version (recommended) // atom.confirm({ // message: 'How you feeling?', - // detailedMessage: 'Be honest.', + // detail: 'Be honest.', // buttons: ['Good', 'Bad'] // }, response => { // if (response === 0) { @@ -934,7 +937,7 @@ class AtomEnvironment { // }) // // ```js - // // Sync version + // // Legacy sync version // const chosen = atom.confirm({ // message: 'How you feeling?', // detailedMessage: 'Be honest.', @@ -945,23 +948,25 @@ class AtomEnvironment { // }) // ``` // - // * `options` An {Object} with the following keys: + // * `options` An options {Object}. If the callback argument is also supplied, see the documentation at + // https://electronjs.org/docs/api/dialog#dialogshowmessageboxbrowserwindow-options-callback for the list of + // available options. Otherwise, only the following keys are accepted: // * `message` The {String} message to display. // * `detailedMessage` (optional) The {String} detailed message to display. // * `buttons` (optional) Either an {Array} of {String}s or an {Object} where keys are // button names and the values are callback {Function}s to invoke when clicked. // * `callback` (optional) A {Function} that will be called with the index of the chosen option. - // If a callback is supplied, `buttons` (if supplied) must be an {Array}, - // and the renderer process will not be paused while the dialog box is open. + // If a callback is supplied, the dialog will be non-blocking. This argument is recommended. // // Returns the chosen button index {Number} if the buttons option is an array // or the return value of the callback if the buttons option is an object. - confirm (params = {}, callback) { + // If a callback function is supplied, returns `undefined`. + confirm (options = {}, callback) { if (callback) { // Async: no return value - this.applicationDelegate.confirm(params, callback) + this.applicationDelegate.confirm(options, callback) } else { - return this.applicationDelegate.confirm(params) + return this.applicationDelegate.confirm(options) } } From b881edac06188148669db136999b651c98033e0a Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sat, 18 Nov 2017 01:45:19 +0100 Subject: [PATCH 09/20] Convert attemptRestoreProjectStateForPaths --- src/atom-environment.js | 42 ++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/atom-environment.js b/src/atom-environment.js index 8328f53bc..5f620d424 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -1056,7 +1056,7 @@ class AtomEnvironment { } } - attemptRestoreProjectStateForPaths (state, projectPaths, filesToOpen = []) { + async attemptRestoreProjectStateForPaths (state, projectPaths, filesToOpen = []) { const center = this.workspace.getCenter() const windowIsUnused = () => { for (let container of this.workspace.getPaneContainers()) { @@ -1075,30 +1075,38 @@ class AtomEnvironment { this.restoreStateIntoThisEnvironment(state) return Promise.all(filesToOpen.map(file => this.workspace.open(file))) } else { + let resolveDiscardStatePromise = null + const discardStatePromise = new Promise((resolve) => { + resolveDiscardStatePromise = resolve + }) const nouns = projectPaths.length === 1 ? 'folder' : 'folders' - const choice = this.confirm({ + this.confirm({ message: 'Previous automatically-saved project state detected', - detailedMessage: `There is previously saved state for the selected ${nouns}. ` + + detail: `There is previously saved state for the selected ${nouns}. ` + `Would you like to add the ${nouns} to this window, permanently discarding the saved state, ` + `or open the ${nouns} in a new window, restoring the saved state?`, buttons: [ '&Open in new window and recover state', '&Add to this window and discard state' - ]}) - if (choice === 0) { - this.open({ - pathsToOpen: projectPaths.concat(filesToOpen), - newWindow: true, - devMode: this.inDevMode(), - safeMode: this.inSafeMode() - }) - return Promise.resolve(null) - } else if (choice === 1) { - for (let selectedPath of projectPaths) { - this.project.addPath(selectedPath) + ]}, response => { + if (choice === 0) { + this.open({ + pathsToOpen: projectPaths.concat(filesToOpen), + newWindow: true, + devMode: this.inDevMode(), + safeMode: this.inSafeMode() + }) + resolveDiscardStatePromise(Promise.resolve(null)) + } else if (choice === 1) { + for (let selectedPath of projectPaths) { + this.project.addPath(selectedPath) + } + resolveDiscardStatePromise(Promise.all(filesToOpen.map(file => this.workspace.open(file)))) + } } - return Promise.all(filesToOpen.map(file => this.workspace.open(file))) - } + ) + + return discardStatePromise } } From 3d9f6bc6646f72411accfe7627d7cef9374db6c7 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sat, 18 Nov 2017 01:46:39 +0100 Subject: [PATCH 10/20] Update other uses of .confirm for new async API --- src/command-installer.js | 4 ++-- src/pane.js | 2 +- src/workspace.js | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/command-installer.js b/src/command-installer.js index 225547ef4..85360da17 100644 --- a/src/command-installer.js +++ b/src/command-installer.js @@ -23,7 +23,7 @@ class CommandInstaller { const showErrorDialog = (error) => { this.applicationDelegate.confirm({ message: 'Failed to install shell commands', - detailedMessage: error.message + detail: error.message }, () => {}) } @@ -33,7 +33,7 @@ class CommandInstaller { if (error) return showErrorDialog(error) this.applicationDelegate.confirm({ message: 'Commands installed.', - detailedMessage: 'The shell commands `atom` and `apm` are installed.' + detail: 'The shell commands `atom` and `apm` are installed.' }, () => {}) }) }) diff --git a/src/pane.js b/src/pane.js index 502c1e125..cfa281041 100644 --- a/src/pane.js +++ b/src/pane.js @@ -809,7 +809,7 @@ class Pane { const saveDialog = (saveButtonText, saveFn, message) => { this.applicationDelegate.confirm({ message, - detailedMessage: 'Your changes will be lost if you close this item without saving.', + detail: 'Your changes will be lost if you close this item without saving.', buttons: [saveButtonText, 'Cancel', "&Don't Save"] }, response => { switch (response) { diff --git a/src/workspace.js b/src/workspace.js index 8e9d3b2df..e2e7f6165 100644 --- a/src/workspace.js +++ b/src/workspace.js @@ -1224,7 +1224,7 @@ module.exports = class Workspace extends Model { if (fileSize >= (this.config.get('core.warnOnLargeFileLimit') * 1048576)) { // 40MB by default this.applicationDelegate.confirm({ message: 'Atom will be unresponsive during the loading of very large files.', - detailedMessage: 'Do you still want to load this file?', + detail: 'Do you still want to load this file?', buttons: ['Proceed', 'Cancel'] }, response => { if (response === 1) { @@ -2013,7 +2013,7 @@ module.exports = class Workspace extends Model { if (this.config.get('editor.confirmCheckoutHeadRevision')) { this.applicationDelegate.confirm({ message: 'Confirm Checkout HEAD Revision', - detailedMessage: `Are you sure you want to discard all changes to "${editor.getFileName()}" since the last Git commit?`, + detail: `Are you sure you want to discard all changes to "${editor.getFileName()}" since the last Git commit?`, buttons: ['OK', 'Cancel'] }, response => { if (response === 0) checkoutHead() From 131c13db3e9df41d23c23b4ef403dd8f78460548 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sun, 19 Nov 2017 00:35:40 +0100 Subject: [PATCH 11/20] :art: --- src/atom-environment.js | 30 +++++++++++++++--------------- src/workspace.js | 2 +- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/atom-environment.js b/src/atom-environment.js index 5f620d424..4e8aaec7e 100644 --- a/src/atom-environment.js +++ b/src/atom-environment.js @@ -1088,23 +1088,23 @@ class AtomEnvironment { buttons: [ '&Open in new window and recover state', '&Add to this window and discard state' - ]}, response => { - if (choice === 0) { - this.open({ - pathsToOpen: projectPaths.concat(filesToOpen), - newWindow: true, - devMode: this.inDevMode(), - safeMode: this.inSafeMode() - }) - resolveDiscardStatePromise(Promise.resolve(null)) - } else if (choice === 1) { - for (let selectedPath of projectPaths) { - this.project.addPath(selectedPath) - } - resolveDiscardStatePromise(Promise.all(filesToOpen.map(file => this.workspace.open(file)))) + ] + }, response => { + if (response === 0) { + this.open({ + pathsToOpen: projectPaths.concat(filesToOpen), + newWindow: true, + devMode: this.inDevMode(), + safeMode: this.inSafeMode() + }) + resolveDiscardStatePromise(Promise.resolve(null)) + } else if (response === 1) { + for (let selectedPath of projectPaths) { + this.project.addPath(selectedPath) } + resolveDiscardStatePromise(Promise.all(filesToOpen.map(file => this.workspace.open(file)))) } - ) + }) return discardStatePromise } diff --git a/src/workspace.js b/src/workspace.js index e2e7f6165..865f6c29a 100644 --- a/src/workspace.js +++ b/src/workspace.js @@ -1216,7 +1216,7 @@ module.exports = class Workspace extends Model { const largeFileMode = fileSize >= (2 * 1048576) // 2MB - let resolveConfirmFileOpenPromise, rejectConfirmFileOpenPromise = [] + let [resolveConfirmFileOpenPromise, rejectConfirmFileOpenPromise] = [] const confirmFileOpenPromise = new Promise((resolve, reject) => { resolveConfirmFileOpenPromise = resolve rejectConfirmFileOpenPromise = reject From f4bdbe87a0307d138c67a85651c2a0a38f3b8204 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sun, 19 Nov 2017 01:01:45 +0100 Subject: [PATCH 12/20] Update message box mocking --- spec/main-process/atom-application.test.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/spec/main-process/atom-application.test.js b/spec/main-process/atom-application.test.js index 7c19efb9c..9b33cedf4 100644 --- a/spec/main-process/atom-application.test.js +++ b/spec/main-process/atom-application.test.js @@ -513,14 +513,14 @@ describe('AtomApplication', function () { }) // Choosing "Cancel" - mockElectronShowMessageBox({choice: 1}) + mockElectronShowMessageBox({response: 1}) electron.app.quit() await atomApplication.lastBeforeQuitPromise assert(!electron.app.hasQuitted()) assert.equal(electron.app.quit.callCount, 1) // Ensure choosing "Cancel" doesn't try to quit the electron app more than once (regression) // Choosing "Don't save" - mockElectronShowMessageBox({choice: 2}) + mockElectronShowMessageBox({response: 2}) electron.app.quit() await atomApplication.lastBeforeQuitPromise assert(electron.app.hasQuitted()) @@ -561,9 +561,9 @@ describe('AtomApplication', function () { } } - function mockElectronShowMessageBox ({choice}) { - electron.dialog.showMessageBox = function () { - return choice + function mockElectronShowMessageBox ({response}) { + electron.dialog.showMessageBox = function (window, options, callback) { + callback(response) } } From 58f351e5985c2f2a3e022adc4afd3b7a7294e139 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sun, 19 Nov 2017 01:02:10 +0100 Subject: [PATCH 13/20] Update AtomEnvironment specs for async confirm --- spec/atom-environment-spec.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/spec/atom-environment-spec.js b/spec/atom-environment-spec.js index 84b415eab..2b6c48b3f 100644 --- a/spec/atom-environment-spec.js +++ b/spec/atom-environment-spec.js @@ -1,4 +1,4 @@ -const {it, fit, ffit, fffit, beforeEach, afterEach} = require('./async-spec-helpers') +const {it, fit, ffit, beforeEach, afterEach, conditionPromise} = require('./async-spec-helpers') const _ = require('underscore-plus') const path = require('path') const temp = require('temp').track() @@ -515,27 +515,31 @@ describe('AtomEnvironment', () => { }) }) - it('prompts the user to restore the state in a new window, discarding it and adding folder to current window', () => { - spyOn(atom, 'confirm').andReturn(1) + it('prompts the user to restore the state in a new window, discarding it and adding folder to current window', async () => { + jasmine.useRealClock() + spyOn(atom, 'confirm').andCallFake((options, callback) => callback(1)) spyOn(atom.project, 'addPath') spyOn(atom.workspace, 'open') const state = Symbol() atom.attemptRestoreProjectStateForPaths(state, [__dirname], [__filename]) expect(atom.confirm).toHaveBeenCalled() - expect(atom.project.addPath.callCount).toBe(1) + await conditionPromise(() => atom.project.addPath.callCount === 1) + expect(atom.project.addPath).toHaveBeenCalledWith(__dirname) expect(atom.workspace.open.callCount).toBe(1) expect(atom.workspace.open).toHaveBeenCalledWith(__filename) }) - it('prompts the user to restore the state in a new window, opening a new window', () => { - spyOn(atom, 'confirm').andReturn(0) + fit('prompts the user to restore the state in a new window, opening a new window', () => { + jasmine.useRealClock() + spyOn(atom, 'confirm').andCallFake((options, callback) => callback(0)) spyOn(atom, 'open') const state = Symbol() atom.attemptRestoreProjectStateForPaths(state, [__dirname], [__filename]) expect(atom.confirm).toHaveBeenCalled() + await conditionPromise(() => atom.open.callCount === 1) expect(atom.open).toHaveBeenCalledWith({ pathsToOpen: [__dirname, __filename], newWindow: true, From b5c4336a3037a7521bc6219f54506a8f5cd41345 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sun, 19 Nov 2017 01:02:23 +0100 Subject: [PATCH 14/20] Update CommandInstaller specs for async confirm --- spec/command-installer-spec.js | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/spec/command-installer-spec.js b/spec/command-installer-spec.js index a2ecb6743..6a2a31e77 100644 --- a/spec/command-installer-spec.js +++ b/spec/command-installer-spec.js @@ -35,9 +35,9 @@ describe('CommandInstaller on #darwin', () => { installer.installShellCommandsInteractively() - expect(appDelegate.confirm).toHaveBeenCalledWith({ + expect(appDelegate.confirm.mostRecentCall.args[0]).toEqual({ message: 'Failed to install shell commands', - detailedMessage: 'an error' + detail: 'an error' }) appDelegate.confirm.reset() @@ -46,9 +46,9 @@ describe('CommandInstaller on #darwin', () => { installer.installShellCommandsInteractively() - expect(appDelegate.confirm).toHaveBeenCalledWith({ + expect(appDelegate.confirm.mostRecentCall.args[0]).toEqual({ message: 'Failed to install shell commands', - detailedMessage: 'another error' + detail: 'another error' }) }) @@ -61,9 +61,9 @@ describe('CommandInstaller on #darwin', () => { installer.installShellCommandsInteractively() - expect(appDelegate.confirm).toHaveBeenCalledWith({ + expect(appDelegate.confirm.mostRecentCall.args[0]).toEqual({ message: 'Commands installed.', - detailedMessage: 'The shell commands `atom` and `apm` are installed.' + detail: 'The shell commands `atom` and `apm` are installed.' }) }) From f6abe9a5554f77459c51c48b664e38484f45ce28 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sun, 19 Nov 2017 01:05:53 +0100 Subject: [PATCH 15/20] Oops --- spec/atom-environment-spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/atom-environment-spec.js b/spec/atom-environment-spec.js index 2b6c48b3f..67171d378 100644 --- a/spec/atom-environment-spec.js +++ b/spec/atom-environment-spec.js @@ -531,7 +531,7 @@ describe('AtomEnvironment', () => { expect(atom.workspace.open).toHaveBeenCalledWith(__filename) }) - fit('prompts the user to restore the state in a new window, opening a new window', () => { + it('prompts the user to restore the state in a new window, opening a new window', async () => { jasmine.useRealClock() spyOn(atom, 'confirm').andCallFake((options, callback) => callback(0)) spyOn(atom, 'open') From f960b43782c0020176fd1ad2686583647523d9e4 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sun, 19 Nov 2017 01:06:20 +0100 Subject: [PATCH 16/20] Update PaneContainer specs for async confirm --- spec/pane-container-spec.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/pane-container-spec.js b/spec/pane-container-spec.js index 1918364f9..060808d0b 100644 --- a/spec/pane-container-spec.js +++ b/spec/pane-container-spec.js @@ -5,7 +5,7 @@ describe('PaneContainer', () => { let confirm, params beforeEach(() => { - confirm = spyOn(atom.applicationDelegate, 'confirm').andReturn(0) + confirm = spyOn(atom.applicationDelegate, 'confirm').andCallFake((options, callback) => callback(0)) params = { location: 'center', config: atom.config, @@ -280,14 +280,14 @@ describe('PaneContainer', () => { }) it('returns true if the user saves all modified files when prompted', async () => { - confirm.andReturn(0) + confirm.andCallFake((options, callback) => callback(0)) const saved = await container.confirmClose() expect(confirm).toHaveBeenCalled() expect(saved).toBeTruthy() }) it('returns false if the user cancels saving any modified file', async () => { - confirm.andReturn(1) + confirm.andCallFake((options, callback) => callback(1)) const saved = await container.confirmClose() expect(confirm).toHaveBeenCalled() expect(saved).toBeFalsy() From 4fdee7bb8f658923cefe0f04379a05a194fb1c51 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sun, 19 Nov 2017 01:12:23 +0100 Subject: [PATCH 17/20] Update Pane specs for async confirm --- spec/pane-spec.js | 33 +++++++++++++++++---------------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/spec/pane-spec.js b/spec/pane-spec.js index e448f992f..ba6cdadb9 100644 --- a/spec/pane-spec.js +++ b/spec/pane-spec.js @@ -564,7 +564,7 @@ describe('Pane', () => { describe('when the item has a uri', () => { it('saves the item before destroying it', async () => { itemURI = 'test' - confirm.andReturn(0) + confirm.andCallFake((options, callback) => callback(0)) const success = await pane.destroyItem(item1) expect(item1.save).toHaveBeenCalled() @@ -579,7 +579,7 @@ describe('Pane', () => { itemURI = null showSaveDialog.andReturn('/selected/path') - confirm.andReturn(0) + confirm.andCallFake((options, callback) => callback(0)) const success = await pane.destroyItem(item1) expect(showSaveDialog).toHaveBeenCalledWith({}) @@ -593,7 +593,7 @@ describe('Pane', () => { describe("if the [Don't Save] option is selected", () => { it('removes and destroys the item without saving it', async () => { - confirm.andReturn(2) + confirm.andCallFake((options, callback) => callback(2)) const success = await pane.destroyItem(item1) expect(item1.save).not.toHaveBeenCalled() @@ -605,7 +605,7 @@ describe('Pane', () => { describe('if the [Cancel] option is selected', () => { it('does not save, remove, or destroy the item', async () => { - confirm.andReturn(1) + confirm.andCallFake((options, callback) => callback(1)) const success = await pane.destroyItem(item1) expect(item1.save).not.toHaveBeenCalled() @@ -1210,7 +1210,7 @@ describe('Pane', () => { item1.getURI = () => '/test/path' item1.save = jasmine.createSpy('save') - confirm.andReturn(0) + confirm.andCallFake((options, callback) => callback(0)) await pane.close() expect(confirm).toHaveBeenCalled() expect(item1.save).toHaveBeenCalled() @@ -1225,7 +1225,7 @@ describe('Pane', () => { item1.getURI = () => '/test/path' item1.save = jasmine.createSpy('save') - confirm.andReturn(1) + confirm.andCallFake((options, callback) => callback(1)) await pane.close() expect(confirm).toHaveBeenCalled() @@ -1240,7 +1240,7 @@ describe('Pane', () => { item1.shouldPromptToSave = () => true item1.saveAs = jasmine.createSpy('saveAs') - confirm.andReturn(0) + confirm.andCallFake((options, callback) => callback(0)) showSaveDialog.andReturn(undefined) await pane.close() @@ -1270,12 +1270,12 @@ describe('Pane', () => { it('does not destroy the pane if save fails and user clicks cancel', async () => { let confirmations = 0 - confirm.andCallFake(() => { + confirm.andCallFake((options, callback) => { confirmations++ if (confirmations === 1) { - return 0 // click save + callback(0) // click save } else { - return 1 + callback(1) } }) // click cancel @@ -1290,9 +1290,9 @@ describe('Pane', () => { item1.saveAs = jasmine.createSpy('saveAs').andReturn(true) let confirmations = 0 - confirm.andCallFake(() => { + confirm.andCallFake((options, callback) => { confirmations++ - return 0 + callback(0) }) // save and then save as showSaveDialog.andReturn('new/path') @@ -1315,13 +1315,14 @@ describe('Pane', () => { }) let confirmations = 0 - confirm.andCallFake(() => { + confirm.andCallFake((options, callback) => { confirmations++ if (confirmations < 3) { - return 0 // save, save as, save as + callback(0) // save, save as, save as + } else { + callback(2) // don't save } - return 2 - }) // don't save + }) showSaveDialog.andReturn('new/path') From 0ba6517a415f3bb5fbf9d10e8082aad5c8ee81ed Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Sun, 19 Nov 2017 01:21:15 +0100 Subject: [PATCH 18/20] Update Workspace specs for async confirm --- spec/workspace-spec.js | 47 ++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 25 deletions(-) diff --git a/spec/workspace-spec.js b/spec/workspace-spec.js index 1bde0e6fe..af9a543d9 100644 --- a/spec/workspace-spec.js +++ b/spec/workspace-spec.js @@ -8,7 +8,7 @@ const _ = require('underscore-plus') const fstream = require('fstream') const fs = require('fs-plus') const AtomEnvironment = require('../src/atom-environment') -const {it, fit, ffit, fffit, beforeEach, afterEach} = require('./async-spec-helpers') +const {it, fit, ffit, fffit, beforeEach, afterEach, conditionPromise} = require('./async-spec-helpers') describe('Workspace', () => { let workspace @@ -668,31 +668,27 @@ describe('Workspace', () => { }) describe('when the file is over user-defined limit', () => { - const shouldPromptForFileOfSize = (size, shouldPrompt) => { + const shouldPromptForFileOfSize = async (size, shouldPrompt) => { spyOn(fs, 'getSizeSync').andReturn(size * 1048577) - atom.applicationDelegate.confirm.andCallFake(() => selectedButtonIndex) + + let selectedButtonIndex = 1 // cancel + atom.applicationDelegate.confirm.andCallFake((options, callback) => callback(selectedButtonIndex)) atom.applicationDelegate.confirm() - var selectedButtonIndex = 1 // cancel - let editor = null - waitsForPromise(() => workspace.open('sample.js').then(e => { editor = e })) + let editor = await workspace.open('sample.js') if (shouldPrompt) { - runs(() => { - expect(editor).toBeUndefined() - expect(atom.applicationDelegate.confirm).toHaveBeenCalled() + expect(editor).toBeUndefined() + expect(atom.applicationDelegate.confirm).toHaveBeenCalled() - atom.applicationDelegate.confirm.reset() - selectedButtonIndex = 0 - }) // open the file + atom.applicationDelegate.confirm.reset() + selectedButtonIndex = 0 // open the file - waitsForPromise(() => workspace.open('sample.js').then(e => { editor = e })) + editor = await workspace.open('sample.js') - runs(() => { - expect(atom.applicationDelegate.confirm).toHaveBeenCalled() - expect(editor.largeFileMode).toBe(true) - }) + expect(atom.applicationDelegate.confirm).toHaveBeenCalled() + expect(editor.largeFileMode).toBe(true) } else { - runs(() => expect(editor).not.toBeUndefined()) + expect(editor).not.toBeUndefined() } } @@ -2823,29 +2819,30 @@ i = /test/; #FIXME\ describe('.checkoutHeadRevision()', () => { let editor = null - beforeEach(() => { + beforeEach(async () => { + jasmine.useRealClock() atom.config.set('editor.confirmCheckoutHeadRevision', false) - waitsForPromise(() => atom.workspace.open('sample-with-comments.js').then(o => { editor = o })) + editor = await atom.workspace.open('sample-with-comments.js') }) - it('reverts to the version of its file checked into the project repository', () => { + it('reverts to the version of its file checked into the project repository', async () => { editor.setCursorBufferPosition([0, 0]) editor.insertText('---\n') expect(editor.lineTextForBufferRow(0)).toBe('---') - waitsForPromise(() => atom.workspace.checkoutHeadRevision(editor)) + atom.workspace.checkoutHeadRevision(editor) - runs(() => expect(editor.lineTextForBufferRow(0)).toBe('')) + await conditionPromise(() => editor.lineTextForBufferRow(0) === '') }) describe("when there's no repository for the editor's file", () => { - it("doesn't do anything", () => { + it("doesn't do anything", async () => { editor = new TextEditor() editor.setText('stuff') atom.workspace.checkoutHeadRevision(editor) - waitsForPromise(() => atom.workspace.checkoutHeadRevision(editor)) + atom.workspace.checkoutHeadRevision(editor) }) }) }) From 7fcfdcec00725960e742e2c9daf4439616be2ae5 Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Mon, 4 Dec 2017 22:58:59 +0100 Subject: [PATCH 19/20] Test assertions correctly --- spec/workspace-spec.js | 15 +++++++-------- src/workspace.js | 1 + 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/spec/workspace-spec.js b/spec/workspace-spec.js index 58ceb6a4d..4b115e594 100644 --- a/spec/workspace-spec.js +++ b/spec/workspace-spec.js @@ -659,13 +659,12 @@ describe('Workspace', () => { }) }) - describe('when the file is over user-defined limit', () => { + describe('when the file size is over the limit defined in `core.warnOnLargeFileLimit`', () => { const shouldPromptForFileOfSize = async (size, shouldPrompt) => { spyOn(fs, 'getSizeSync').andReturn(size * 1048577) let selectedButtonIndex = 1 // cancel atom.applicationDelegate.confirm.andCallFake((options, callback) => callback(selectedButtonIndex)) - atom.applicationDelegate.confirm() let editor = await workspace.open('sample.js') if (shouldPrompt) { @@ -683,19 +682,19 @@ describe('Workspace', () => { } } - it('prompts the user to make sure they want to open a file this big', () => { + it('prompts before opening the file', async () => { atom.config.set('core.warnOnLargeFileLimit', 20) - shouldPromptForFileOfSize(20, true) + await shouldPromptForFileOfSize(20, true) }) - it("doesn't prompt on files below the limit", () => { + it("doesn't prompt on files below the limit", async () => { atom.config.set('core.warnOnLargeFileLimit', 30) - shouldPromptForFileOfSize(20, false) + await shouldPromptForFileOfSize(20, false) }) - it('prompts for smaller files with a lower limit', () => { + it('prompts for smaller files with a lower limit', async () => { atom.config.set('core.warnOnLargeFileLimit', 5) - shouldPromptForFileOfSize(10, true) + await shouldPromptForFileOfSize(10, true) }) }) diff --git a/src/workspace.js b/src/workspace.js index 564fa3652..127168748 100644 --- a/src/workspace.js +++ b/src/workspace.js @@ -1221,6 +1221,7 @@ module.exports = class Workspace extends Model { resolveConfirmFileOpenPromise = resolve rejectConfirmFileOpenPromise = reject }) + if (fileSize >= (this.config.get('core.warnOnLargeFileLimit') * 1048576)) { // 40MB by default this.applicationDelegate.confirm({ message: 'Atom will be unresponsive during the loading of very large files.', From 18ee2e6f111c0f9424d9754fb54949010aabd20c Mon Sep 17 00:00:00 2001 From: Wliu <50Wliu@users.noreply.github.com> Date: Tue, 9 Jan 2018 14:32:19 -0500 Subject: [PATCH 20/20] Oops --- src/application-delegate.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/application-delegate.js b/src/application-delegate.js index b52071986..cdcc03546 100644 --- a/src/application-delegate.js +++ b/src/application-delegate.js @@ -183,7 +183,7 @@ class ApplicationDelegate { } else { // Legacy sync version: options can only have `message`, // `detailedMessage` (optional), and buttons array or object (optional) - ({message, detailedMessage, buttons} = options) + let {message, detailedMessage, buttons} = options let buttonLabels if (!buttons) buttons = {}