Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Commit

Permalink
Merge pull request #6002 from adobe/couzteau/fix-5986
Browse files Browse the repository at this point in the history 
Fix Unit tests, Move async file.exists test out of EditorManager into DocumentCommandHandlers
  • Loading branch information
@RaymondLim
RaymondLim committed Nov 15, 2013
2 parents 30eccd5 + 0aba29b commit efbd1dc
Show file tree
Hide file tree
Showing 3 changed files with 117 additions and 131 deletions.
76 changes: 50 additions & 26 deletions src/document/DocumentCommandHandlers.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ define(function (require, exports, module) {
DocumentManager = require("document/DocumentManager"),
EditorManager = require("editor/EditorManager"),
FileSystem = require("filesystem/FileSystem"),
FileSystemError = require("filesystem/FileSystemError"),
FileUtils = require("file/FileUtils"),
FileViewController = require("project/FileViewController"),
InMemoryFile = require("document/InMemoryFile"),
Expand Down Expand Up @@ -178,7 +179,43 @@ define(function (require, exports, module) {
*/
function doOpen(fullPath, silent) {
var result = new $.Deferred();


// workaround for https://github.com/adobe/brackets/issues/6001
// TODO should be removed once bug is closed.
// if we are already displaying a file do nothing but resolve immediately.
// this fixes timing issues in test cases.
if (EditorManager.getCurrentlyViewedPath() === fullPath) {
result.resolve(DocumentManager.getCurrentDocument());
return result.promise();
}

function _cleanup(fullFilePath) {
if (!fullFilePath || EditorManager.showingCustomViewerForPath(fullFilePath)) {
// We get here only after the user renames a file that makes it no longer belong to a
// custom viewer but the file is still showing in the current custom viewer. This only
// occurs on Mac since opening a non-text file always fails on Mac and triggers an error
// message that in turn calls _cleanup() after the user clicks OK in the message box.
// So we need to explicitly close the currently viewing image file whose filename is
// no longer valid. Calling notifyPathDeleted will close the image vieer and then select
// the previously opened text file or show no-editor if none exists.
EditorManager.notifyPathDeleted(fullFilePath);
} else {
// For performance, we do lazy checking of file existence, so it may be in working set
DocumentManager.removeFromWorkingSet(FileSystem.getFileForPath(fullFilePath));
EditorManager.focusEditor();
}
result.reject();
}
function _showErrorAndCleanUp(fileError, fullFilePath) {
if (silent) {
_cleanup(fullFilePath);
} else {
FileUtils.showFileOpenError(fileError, fullFilePath).done(function () {
_cleanup(fullFilePath);
});
}
}

if (!fullPath) {
console.error("doOpen() called without fullPath");
result.reject();
Expand All @@ -190,8 +227,17 @@ define(function (require, exports, module) {

var viewProvider = EditorManager.getCustomViewerForPath(fullPath);
if (viewProvider) {
EditorManager.showCustomViewer(viewProvider, fullPath);
result.resolve();
var file = FileSystem.getFileForPath(fullPath);
file.exists(function (fileError, fileExists) {
if (fileExists) {
EditorManager.showCustomViewer(viewProvider, fullPath);
result.resolve();
} else {
fileError = fileError || FileSystemError.NOT_FOUND;
_showErrorAndCleanUp(fileError);
}
});

} else {
// Load the file if it was never open before, and then switch to it in the UI
DocumentManager.getDocumentForPath(fullPath)
Expand All @@ -200,29 +246,7 @@ define(function (require, exports, module) {
result.resolve(doc);
})
.fail(function (fileError) {
function _cleanup() {
if (EditorManager.showingCustomViewerForPath(fullPath)) {
// We get here only after the user renames a file that makes it no longer belong to a
// custom viewer but the file is still showing in the current custom viewer. This only
// occurs on Mac since opening a non-text file always fails on Mac and triggers an error
// message that in turn calls _cleanup() after the user clicks OK in the message box.
// So we need to explicitly close the currently viewing image file whose filename is
// no longer valid. Calling notifyPathDeleted will close the image vieer and then select
// the previously opened text file or show no-editor if none exists.
EditorManager.notifyPathDeleted(fullPath);
} else {
// For performance, we do lazy checking of file existence, so it may be in working set
DocumentManager.removeFromWorkingSet(FileSystem.getFileForPath(fullPath));
EditorManager.focusEditor();
}
result.reject();
}

if (silent) {
_cleanup();
} else {
FileUtils.showFileOpenError(fileError, fullPath).done(_cleanup);
}
_showErrorAndCleanUp(fileError, fullPath);
});
}
}
Expand Down
150 changes: 51 additions & 99 deletions src/editor/EditorManager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,7 @@ define(function (require, exports, module) {
InlineTextEditor = require("editor/InlineTextEditor").InlineTextEditor,
ImageViewer = require("editor/ImageViewer"),
Strings = require("strings"),
LanguageManager = require("language/LanguageManager"),
FileSystem = require("filesystem/FileSystem"),
FileSystemError = require("filesystem/FileSystemError"),
FileUtils = require("file/FileUtils");
LanguageManager = require("language/LanguageManager");

/** @type {jQueryObject} DOM node that contains all editors (visible and hidden alike) */
var _editorHolder = null;
Expand All @@ -81,8 +78,6 @@ define(function (require, exports, module) {
var _$currentCustomViewer = null;
/** @type {?Object} view provider */
var _currentViewProvider = null;
/** Helper function defined as var to satisfy JSLint order constraints */
var _checkFileExists;

/**
* Currently focused Editor (full-size, inline, or otherwise)
Expand Down Expand Up @@ -630,70 +625,13 @@ define(function (require, exports, module) {

/** Remove existing custom view if present */
function _removeCustomViewer() {
window.removeEventListener("focus", _checkFileExists);
$(exports).triggerHandler("removeCustomViewer");
if (_$currentCustomViewer) {
_$currentCustomViewer.remove();
}
_$currentCustomViewer = null;
_currentViewProvider = null;
}

/**
* Clears custom viewer for a file with a given path and displays
* an alternate file or the no editor view.
* If no param fullpath is passed an alternate file will be opened
* regardless of the current value of _currentlyViewedPath.
* If param fullpath is provided then only if fullpath matches
* the currently viewed file an alternate file will be opened.
* @param {?string} fullPath - file path of deleted file.
*/
function notifyPathDeleted(fullPath) {
function openAlternateFile() {
var fileToOpen = DocumentManager.getNextPrevFile(1);
if (fileToOpen) {
CommandManager.execute(Commands.FILE_OPEN, {fullPath: fileToOpen.fullPath});
} else {
_removeCustomViewer();
_showNoEditor();
_setCurrentlyViewedPath();
}
}
if (!fullPath || _currentlyViewedPath === fullPath) {
openAlternateFile();
}
}

/*
* show a generic error or File Not Found in modal error dialog
*/
function _showErrorAndNotify(err, fullPath) {
var errorToShow = err || FileSystemError.NOT_FOUND;
FileUtils.showFileOpenError(errorToShow, fullPath).done(
function () {
notifyPathDeleted();
}
);
}

/*
* callback function passed to file.exists. If file in view does
* not exist the current view will be replaced.
*/
function _removeViewIfFileDeleted(err, fileExists) {
if (!fileExists) {
notifyPathDeleted();
}
}

/**
* Makes sure that the file in view is present in the file system
* Close and warn if file is gone.
*/
_checkFileExists = function () {
var file = FileSystem.getFileForPath(getCurrentlyViewedPath());
file.exists(_removeViewIfFileDeleted);
};

/**
* Closes the customViewer currently displayed, shows the NoEditor view
Expand All @@ -704,50 +642,39 @@ define(function (require, exports, module) {
_setCurrentlyViewedPath();
_showNoEditor();
}

/**
* Append custom view to editor-holder
* @param {!Object} provider custom view provider
* @param {!string} fullPath path to the file displayed in the custom view
*/
function showCustomViewer(provider, fullPath) {
function _doShow(err, fileExists) {
if (!fileExists) {
_showErrorAndNotify(err, fullPath);
} else {
// Don't show the same custom view again if file path
// and view provider are still the same.
if (_currentlyViewedPath === fullPath &&
_currentViewProvider === provider) {
return;
}

// Clean up currently viewing document or custom viewer
DocumentManager._clearCurrentDocument();
_removeCustomViewer();

// Hide the not-editor or reset current editor
$("#not-editor").css("display", "none");
_nullifyEditor();
// Don't show the same custom view again if file path
// and view provider are still the same.
if (_currentlyViewedPath === fullPath &&
_currentViewProvider === provider) {
return;
}

_currentViewProvider = provider;
_$currentCustomViewer = provider.getCustomViewHolder(fullPath);
// Clean up currently viewing document or custom viewer
DocumentManager._clearCurrentDocument();
_removeCustomViewer();

// Hide the not-editor or reset current editor
$("#not-editor").css("display", "none");
_nullifyEditor();

_currentViewProvider = provider;
_$currentCustomViewer = provider.getCustomViewHolder(fullPath);

// place in window
$("#editor-holder").append(_$currentCustomViewer);

// place in window
$("#editor-holder").append(_$currentCustomViewer);

// add path, dimensions and file size to the view after loading image
provider.render(fullPath);
// make sure the file in display is still there when window gets focus.
// close and warn if the file is gone.
window.addEventListener("focus", _checkFileExists);
_setCurrentlyViewedPath(fullPath);
}
}
var file = FileSystem.getFileForPath(fullPath);
file.exists(_doShow);
// add path, dimensions and file size to the view after loading image
provider.render(fullPath);

_setCurrentlyViewedPath(fullPath);
}


/**
* Check whether the given file is currently open in a custom viewer.
Expand Down Expand Up @@ -789,6 +716,31 @@ define(function (require, exports, module) {
return null;
}

/**
* Clears custom viewer for a file with a given path and displays
* an alternate file or the no editor view.
* If no param fullpath is passed an alternate file will be opened
* regardless of the current value of _currentlyViewedPath.
* If param fullpath is provided then only if fullpath matches
* the currently viewed file an alternate file will be opened.
* @param {?string} fullPath - file path of deleted file.
*/
function notifyPathDeleted(fullPath) {
function openAlternateFile() {
var fileToOpen = DocumentManager.getNextPrevFile(1);
if (fileToOpen) {
CommandManager.execute(Commands.FILE_OPEN, {fullPath: fileToOpen.fullPath});
} else {
_removeCustomViewer();
_showNoEditor();
_setCurrentlyViewedPath();
}
}
if (!fullPath || _currentlyViewedPath === fullPath) {
openAlternateFile();
}
}

/** Handles changes to DocumentManager.getCurrentDocument() */
function _onCurrentDocumentChange() {
var doc = DocumentManager.getCurrentDocument(),
Expand Down Expand Up @@ -1063,4 +1015,4 @@ define(function (require, exports, module) {
exports.notifyPathDeleted = notifyPathDeleted;
exports.closeCustomViewer = closeCustomViewer;
exports.showingCustomViewerForPath = showingCustomViewerForPath;
});
});
22 changes: 16 additions & 6 deletions test/spec/DocumentCommandHandlers-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1068,8 +1068,14 @@ define(function (require, exports, module) {

describe("Opens image file and validates EditorManager APIs", function () {
it("should return null after opening an image", function () {
var path = testPath + "/couz.png";
CommandManager.execute(Commands.FILE_OPEN, { fullPath: path }).done(function (result) {
var path = testPath + "/couz.png",
promise;
runs(function () {
promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: path });
waitsForDone(promise, Commands.FILE_OPEN);
});

runs(function () {
expect(EditorManager.getActiveEditor()).toEqual(null);
expect(EditorManager.getCurrentFullEditor()).toEqual(null);
expect(EditorManager.getFocusedEditor()).toEqual(null);
Expand All @@ -1082,12 +1088,10 @@ define(function (require, exports, module) {

describe("Open image file while a text file is open", function () {
it("should fire currentDocumentChange and activeEditorChange events", function () {

var promise,
docChangeListener = jasmine.createSpy(),
activeEditorChangeListener = jasmine.createSpy();


runs(function () {
_$(DocumentManager).on("currentDocumentChange", docChangeListener);
_$(EditorManager).on("activeEditorChange", activeEditorChangeListener);
Expand Down Expand Up @@ -1184,8 +1188,14 @@ define(function (require, exports, module) {

describe("Opens text file and validates EditorManager APIs", function () {
it("should return an editor after opening a text file", function () {
var path = testPath + "/test.js";
CommandManager.execute(Commands.FILE_OPEN, { fullPath: path }).done(function (result) {
var path = testPath + "/test.js",
promise;
runs(function () {
promise = CommandManager.execute(Commands.FILE_OPEN, { fullPath: path });
waitsForDone(promise, Commands.FILE_OPEN);
});

runs(function () {
var e = EditorManager.getActiveEditor();
expect(e.document.file.fullPath).toBe(path);

Expand Down

0 comments on commit efbd1dc

Please sign in to comment.