From b5c9a90ae00ec44e91782b0d6e30be7ca2fea11c Mon Sep 17 00:00:00 2001 From: Einar Boson Date: Sun, 5 Jul 2015 01:04:15 +0200 Subject: [PATCH] :bug: Ask user to 'save as' if save fails when closing tab, fixes #7708 --- spec/pane-spec.coffee | 98 +++++++++++++++++++++++++++++++++++++++++++ src/pane.coffee | 58 +++++++++++++++++-------- 2 files changed, 139 insertions(+), 17 deletions(-) diff --git a/spec/pane-spec.coffee b/spec/pane-spec.coffee index 3453ac44d..bdeb9615a 100644 --- a/spec/pane-spec.coffee +++ b/spec/pane-spec.coffee @@ -671,6 +671,104 @@ describe "Pane", -> expect(item1.save).not.toHaveBeenCalled() expect(pane.isDestroyed()).toBe false + it "does not destroy the pane if save fails and user clicks cancel", -> + pane = new Pane(items: [new Item("A"), new Item("B")]) + [item1, item2] = pane.getItems() + + item1.shouldPromptToSave = -> true + item1.getURI = -> "/test/path" + + item1.save = jasmine.createSpy("save").andCallFake -> + error = new Error("EACCES, permission denied '/test/path'") + error.path = '/test/path' + error.code = 'EACCES' + throw error + + confirmations = 0 + spyOn(atom, 'confirm').andCallFake -> + confirmations++ + if confirmations is 1 + return 0 + else + return 1 + + pane.close() + + expect(atom.confirm).toHaveBeenCalled() + expect(confirmations).toBe(2) + expect(item1.save).toHaveBeenCalled() + expect(pane.isDestroyed()).toBe false + + it "does destroy the pane if the user saves the file under a new name", -> + pane = new Pane(items: [new Item("A"), new Item("B")]) + [item1, item2] = pane.getItems() + + item1.shouldPromptToSave = -> true + item1.getURI = -> "/test/path" + + item1.save = jasmine.createSpy("save").andCallFake -> + error = new Error("EACCES, permission denied '/test/path'") + error.path = '/test/path' + error.code = 'EACCES' + throw error + + item1.saveAs = jasmine.createSpy("saveAs").andReturn(true) + + confirmations = 0 + spyOn(atom, 'confirm').andCallFake -> + confirmations++ + return 0 + + spyOn(atom, 'showSaveDialogSync').andReturn("new/path") + + pane.close() + + expect(atom.confirm).toHaveBeenCalled() + expect(confirmations).toBe(2) + expect(atom.showSaveDialogSync).toHaveBeenCalled() + expect(item1.save).toHaveBeenCalled() + expect(item1.saveAs).toHaveBeenCalled() + expect(pane.isDestroyed()).toBe true + + it "asks again if the saveAs also fails", -> + pane = new Pane(items: [new Item("A"), new Item("B")]) + [item1, item2] = pane.getItems() + + item1.shouldPromptToSave = -> true + item1.getURI = -> "/test/path" + + item1.save = jasmine.createSpy("save").andCallFake -> + error = new Error("EACCES, permission denied '/test/path'") + error.path = '/test/path' + error.code = 'EACCES' + throw error + + item1.saveAs = jasmine.createSpy("saveAs").andCallFake -> + error = new Error("EACCES, permission denied '/test/path'") + error.path = '/test/path' + error.code = 'EACCES' + throw error + + + confirmations = 0 + spyOn(atom, 'confirm').andCallFake -> + confirmations++ + if confirmations < 3 + return 0 + return 2 + + spyOn(atom, 'showSaveDialogSync').andReturn("new/path") + + pane.close() + + expect(atom.confirm).toHaveBeenCalled() + expect(confirmations).toBe(3) + expect(atom.showSaveDialogSync).toHaveBeenCalled() + expect(item1.save).toHaveBeenCalled() + expect(item1.saveAs).toHaveBeenCalled() + expect(pane.isDestroyed()).toBe true + + describe "::destroy()", -> [container, pane1, pane2] = [] diff --git a/src/pane.coffee b/src/pane.coffee index f77da9d58..759146564 100644 --- a/src/pane.coffee +++ b/src/pane.coffee @@ -454,13 +454,26 @@ class Pane extends Model else return true + saveError = (error) => + if error + chosen = atom.confirm + message: @getSaveErrorMessage(error) + detailedMessage: "Your changes will be lost if you close this item without saving." + buttons: ["Save as", "Cancel", "Don't save"] + switch chosen + when 0 then @saveItemAs item, saveError + when 1 then false + when 2 then true + else + true + chosen = atom.confirm message: "'#{item.getTitle?() ? uri}' has changes, do you want to save them?" detailedMessage: "Your changes will be lost if you close this item without saving." buttons: ["Save", "Cancel", "Don't Save"] switch chosen - when 0 then @saveItem(item, -> true) + when 0 then @saveItem(item, saveError) when 1 then false when 2 then true @@ -479,8 +492,10 @@ class Pane extends Model # Public: Save the given item. # # * `item` The item to save. - # * `nextAction` (optional) {Function} which will be called after the item is - # successfully saved. + # * `nextAction` (optional) {Function} which will be called with no argument + # after the item is successfully saved, or with the error if it failed. + # The return value will be that of `nextAction` or `undefined` if it was not + # provided saveItem: (item, nextAction) -> if typeof item?.getURI is 'function' itemURI = item.getURI() @@ -490,9 +505,9 @@ class Pane extends Model if itemURI? try item.save?() + nextAction?() catch error - @handleSaveError(error) - nextAction?() + (nextAction ? @handleSaveError)(error) else @saveItemAs(item, nextAction) @@ -500,8 +515,10 @@ class Pane extends Model # path they select. # # * `item` The item to save. - # * `nextAction` (optional) {Function} which will be called after the item is - # successfully saved. + # * `nextAction` (optional) {Function} which will be called with no argument + # after the item is successfully saved, or with the error if it failed. + # The return value will be that of `nextAction` or `undefined` if it was not + # provided saveItemAs: (item, nextAction) -> return unless item?.saveAs? @@ -511,9 +528,9 @@ class Pane extends Model if newItemPath try item.saveAs(newItemPath) + nextAction?() catch error - @handleSaveError(error) - nextAction?() + (nextAction ? @handleSaveError)(error) # Public: Save all items. saveItems: -> @@ -676,22 +693,29 @@ class Pane extends Model return false unless @promptToSaveItem(item) true - handleSaveError: (error) -> + # Translate an error object to a human readable string + getSaveErrorMessage: (error) -> if error.code is 'EISDIR' or error.message.endsWith('is a directory') - atom.notifications.addWarning("Unable to save file: #{error.message}") + "Unable to save file: #{error.message}." else if error.code is 'EACCES' and error.path? - atom.notifications.addWarning("Unable to save file: Permission denied '#{error.path}'") + "Unable to save file: Permission denied '#{error.path}'." else if error.code in ['EPERM', 'EBUSY', 'UNKNOWN', 'EEXIST'] and error.path? - atom.notifications.addWarning("Unable to save file '#{error.path}'", detail: error.message) + "Unable to save file '#{error.path}': #{error.message}." else if error.code is 'EROFS' and error.path? - atom.notifications.addWarning("Unable to save file: Read-only file system '#{error.path}'") + "Unable to save file: Read-only file system '#{error.path}'." else if error.code is 'ENOSPC' and error.path? - atom.notifications.addWarning("Unable to save file: No space left on device '#{error.path}'") + "Unable to save file: No space left on device '#{error.path}'." else if error.code is 'ENXIO' and error.path? - atom.notifications.addWarning("Unable to save file: No such device or address '#{error.path}'") + "Unable to save file: No such device or address '#{error.path}'." else if errorMatch = /ENOTDIR, not a directory '([^']+)'/.exec(error.message) fileName = errorMatch[1] - atom.notifications.addWarning("Unable to save file: A directory in the path '#{fileName}' could not be written to") + "Unable to save file: A directory in the path '#{fileName}' could not be written to." + + # Display a popup warning to the user + handleSaveError: (error) -> + errorMessage = Pane::getSaveErrorMessage(error) + if errorMessage? + atom.notifications.addWarning(errorMessage) else throw error