From a42505b6ae2a0378d7a49d11ea2a113846390d4e Mon Sep 17 00:00:00 2001 From: Ben Ogle Date: Mon, 15 Dec 2014 17:08:46 -0800 Subject: [PATCH 1/6] Handle `is a directory` error Closes #4616 --- spec/workspace-view-spec.coffee | 24 ++++++++++++++++++++++++ src/workspace.coffee | 13 +++++++++++-- 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/spec/workspace-view-spec.coffee b/spec/workspace-view-spec.coffee index 56c8dc376..9fbe4d5d3 100644 --- a/spec/workspace-view-spec.coffee +++ b/spec/workspace-view-spec.coffee @@ -3,6 +3,7 @@ Q = require 'q' path = require 'path' temp = require 'temp' TextEditorView = require '../src/text-editor-view' +Pane = require '../src/pane' PaneView = require '../src/pane-view' Workspace = require '../src/workspace' @@ -294,3 +295,26 @@ describe "WorkspaceView", -> modalContainer = workspaceElement.querySelector('atom-panel-container.modal') expect(modalContainer.parentNode).toBe workspaceElement + + describe "saving the active item", -> + describe "saveActivePaneItem", -> + describe "when there is an error", -> + beforeEach -> + + it "emits a warning notification when the file cannot be saved", -> + spyOn(Pane::, 'saveActiveItem').andCallFake -> + throw new Error("'/some/file' is a directory") + + atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() + + atom.workspace.saveActivePaneItem() + + expect(addedSpy).toHaveBeenCalled() + expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' + + it "emits a warning notification when the file cannot be saved", -> + spyOn(Pane::, 'saveActiveItem').andCallFake -> + throw new Error("no one knows") + + save = -> atom.workspace.saveActivePaneItem() + expect(save).toThrow() diff --git a/src/workspace.coffee b/src/workspace.coffee index 7b70fa110..7776785bf 100644 --- a/src/workspace.coffee +++ b/src/workspace.coffee @@ -555,7 +555,7 @@ class Workspace extends Model # {::saveActivePaneItemAs} # will be called instead. This method does nothing # if the active item does not implement a `.save` method. saveActivePaneItem: -> - @getActivePane().saveActiveItem() + @saveActivePaneItemAndReportErrors('saveActiveItem') # Prompt the user for a path and save the active pane item to it. # @@ -563,7 +563,16 @@ class Workspace extends Model # `.saveAs` on the item with the selected path. This method does nothing if # the active item does not implement a `.saveAs` method. saveActivePaneItemAs: -> - @getActivePane().saveActiveItemAs() + @saveActivePaneItemAndReportErrors('saveActiveItemAs') + + saveActivePaneItemAndReportErrors: (method) -> + try + @getActivePane()[method]() + catch error + if error.message.endsWith('is a directory') + atom.notifications.addWarning("Unable to save file: #{error.message}") + else + throw error # Destroy (close) the active pane item. # From 67b39845c8de5888a479c54cd909087ba1c9a318 Mon Sep 17 00:00:00 2001 From: Ben Ogle Date: Mon, 15 Dec 2014 17:19:54 -0800 Subject: [PATCH 2/6] Add a better error message when a directory cannot be written to. Closes #4607 --- spec/workspace-view-spec.coffee | 13 ++++++++++++- src/workspace.coffee | 3 +++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/spec/workspace-view-spec.coffee b/spec/workspace-view-spec.coffee index 9fbe4d5d3..2bc0ca6c6 100644 --- a/spec/workspace-view-spec.coffee +++ b/spec/workspace-view-spec.coffee @@ -296,7 +296,7 @@ describe "WorkspaceView", -> modalContainer = workspaceElement.querySelector('atom-panel-container.modal') expect(modalContainer.parentNode).toBe workspaceElement - describe "saving the active item", -> + fdescribe "saving the active item", -> describe "saveActivePaneItem", -> describe "when there is an error", -> beforeEach -> @@ -312,6 +312,17 @@ describe "WorkspaceView", -> expect(addedSpy).toHaveBeenCalled() expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' + it "emits a warning notification when the directory cannot be written to", -> + spyOn(Pane::, 'saveActiveItem').andCallFake -> + throw new Error("ENOTDIR, not a directory '/Some/dir/and-a-file.js'") + + atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() + + atom.workspace.saveActivePaneItem() + + expect(addedSpy).toHaveBeenCalled() + expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' + it "emits a warning notification when the file cannot be saved", -> spyOn(Pane::, 'saveActiveItem').andCallFake -> throw new Error("no one knows") diff --git a/src/workspace.coffee b/src/workspace.coffee index 7776785bf..03f5732cb 100644 --- a/src/workspace.coffee +++ b/src/workspace.coffee @@ -571,6 +571,9 @@ class Workspace extends Model catch error if error.message.endsWith('is a directory') atom.notifications.addWarning("Unable to save file: #{error.message}") + 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.") else throw error From 265601cbdbff82bc17b3d790db608c0ce4db9406 Mon Sep 17 00:00:00 2001 From: Ben Ogle Date: Mon, 15 Dec 2014 17:20:13 -0800 Subject: [PATCH 3/6] Nof --- spec/workspace-view-spec.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/workspace-view-spec.coffee b/spec/workspace-view-spec.coffee index 2bc0ca6c6..69e9e2d7e 100644 --- a/spec/workspace-view-spec.coffee +++ b/spec/workspace-view-spec.coffee @@ -296,7 +296,7 @@ describe "WorkspaceView", -> modalContainer = workspaceElement.querySelector('atom-panel-container.modal') expect(modalContainer.parentNode).toBe workspaceElement - fdescribe "saving the active item", -> + describe "saving the active item", -> describe "saveActivePaneItem", -> describe "when there is an error", -> beforeEach -> From f199c71fa8bd68f3bc3485145bc520ccd80c9633 Mon Sep 17 00:00:00 2001 From: Ben Ogle Date: Mon, 15 Dec 2014 17:42:27 -0800 Subject: [PATCH 4/6] Specs for the eacces error --- spec/workspace-view-spec.coffee | 8 ++++++++ src/workspace.coffee | 4 +++- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/spec/workspace-view-spec.coffee b/spec/workspace-view-spec.coffee index 69e9e2d7e..448284545 100644 --- a/spec/workspace-view-spec.coffee +++ b/spec/workspace-view-spec.coffee @@ -319,7 +319,15 @@ describe "WorkspaceView", -> atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() atom.workspace.saveActivePaneItem() + expect(addedSpy).toHaveBeenCalled() + expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' + it "emits a warning notification when the user does not have permission", -> + spyOn(Pane::, 'saveActiveItem').andCallFake -> + throw new Error("EACCES, permission denied '/Some/dir/and-a-file.js'") + + atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() + atom.workspace.saveActivePaneItem() expect(addedSpy).toHaveBeenCalled() expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' diff --git a/src/workspace.coffee b/src/workspace.coffee index 03f5732cb..98630f7c2 100644 --- a/src/workspace.coffee +++ b/src/workspace.coffee @@ -571,9 +571,11 @@ class Workspace extends Model catch error if error.message.endsWith('is a directory') atom.notifications.addWarning("Unable to save file: #{error.message}") + else if error.message.startsWith('EACCES,') + atom.notifications.addWarning("Unable to save file: #{error.message.replace('EACCES, ', '')}") 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.") + atom.notifications.addWarning("Unable to save file: A directory in the path '#{fileName}' could not be written to") else throw error From 5ff7a286fc1c9cda45bc85a482ca0bd145946105 Mon Sep 17 00:00:00 2001 From: Ben Ogle Date: Mon, 15 Dec 2014 17:42:38 -0800 Subject: [PATCH 5/6] :lipstick: --- spec/workspace-view-spec.coffee | 5 ----- 1 file changed, 5 deletions(-) diff --git a/spec/workspace-view-spec.coffee b/spec/workspace-view-spec.coffee index 448284545..cdb5ce443 100644 --- a/spec/workspace-view-spec.coffee +++ b/spec/workspace-view-spec.coffee @@ -299,16 +299,12 @@ describe "WorkspaceView", -> describe "saving the active item", -> describe "saveActivePaneItem", -> describe "when there is an error", -> - beforeEach -> - it "emits a warning notification when the file cannot be saved", -> spyOn(Pane::, 'saveActiveItem').andCallFake -> throw new Error("'/some/file' is a directory") atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() - atom.workspace.saveActivePaneItem() - expect(addedSpy).toHaveBeenCalled() expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' @@ -317,7 +313,6 @@ describe "WorkspaceView", -> throw new Error("ENOTDIR, not a directory '/Some/dir/and-a-file.js'") atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() - atom.workspace.saveActivePaneItem() expect(addedSpy).toHaveBeenCalled() expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' From 27174b2880b6b76648007c48f44be1fd88d56086 Mon Sep 17 00:00:00 2001 From: Ben Ogle Date: Tue, 16 Dec 2014 09:07:44 -0800 Subject: [PATCH 6/6] Betta spec description --- spec/workspace-view-spec.coffee | 57 ++++++++++++++++----------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/spec/workspace-view-spec.coffee b/spec/workspace-view-spec.coffee index cdb5ce443..09de0bb26 100644 --- a/spec/workspace-view-spec.coffee +++ b/spec/workspace-view-spec.coffee @@ -296,39 +296,38 @@ describe "WorkspaceView", -> modalContainer = workspaceElement.querySelector('atom-panel-container.modal') expect(modalContainer.parentNode).toBe workspaceElement - describe "saving the active item", -> - describe "saveActivePaneItem", -> - describe "when there is an error", -> - it "emits a warning notification when the file cannot be saved", -> - spyOn(Pane::, 'saveActiveItem').andCallFake -> - throw new Error("'/some/file' is a directory") + describe "::saveActivePaneItem()", -> + describe "when there is an error", -> + it "emits a warning notification when the file cannot be saved", -> + spyOn(Pane::, 'saveActiveItem').andCallFake -> + throw new Error("'/some/file' is a directory") - atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() - atom.workspace.saveActivePaneItem() - expect(addedSpy).toHaveBeenCalled() - expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' + atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() + atom.workspace.saveActivePaneItem() + expect(addedSpy).toHaveBeenCalled() + expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' - it "emits a warning notification when the directory cannot be written to", -> - spyOn(Pane::, 'saveActiveItem').andCallFake -> - throw new Error("ENOTDIR, not a directory '/Some/dir/and-a-file.js'") + it "emits a warning notification when the directory cannot be written to", -> + spyOn(Pane::, 'saveActiveItem').andCallFake -> + throw new Error("ENOTDIR, not a directory '/Some/dir/and-a-file.js'") - atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() - atom.workspace.saveActivePaneItem() - expect(addedSpy).toHaveBeenCalled() - expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' + atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() + atom.workspace.saveActivePaneItem() + expect(addedSpy).toHaveBeenCalled() + expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' - it "emits a warning notification when the user does not have permission", -> - spyOn(Pane::, 'saveActiveItem').andCallFake -> - throw new Error("EACCES, permission denied '/Some/dir/and-a-file.js'") + it "emits a warning notification when the user does not have permission", -> + spyOn(Pane::, 'saveActiveItem').andCallFake -> + throw new Error("EACCES, permission denied '/Some/dir/and-a-file.js'") - atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() - atom.workspace.saveActivePaneItem() - expect(addedSpy).toHaveBeenCalled() - expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' + atom.notifications.onDidAddNotification addedSpy = jasmine.createSpy() + atom.workspace.saveActivePaneItem() + expect(addedSpy).toHaveBeenCalled() + expect(addedSpy.mostRecentCall.args[0].getType()).toBe 'warning' - it "emits a warning notification when the file cannot be saved", -> - spyOn(Pane::, 'saveActiveItem').andCallFake -> - throw new Error("no one knows") + it "emits a warning notification when the file cannot be saved", -> + spyOn(Pane::, 'saveActiveItem').andCallFake -> + throw new Error("no one knows") - save = -> atom.workspace.saveActivePaneItem() - expect(save).toThrow() + save = -> atom.workspace.saveActivePaneItem() + expect(save).toThrow()