Merge pull request #558 from adobe/pflynn/live-css-sync-from-disk

Live development: handle deleting CSS file on disk & discarding unsaved changes to CSS file

src/LiveDevelopment/Agents/CSSAgent.js

@@ -54,11 +54,20 @@ define(function CSSAgent(require, exports, module) {
     /** Reload a CSS style sheet from a document
      * @param {Document} document
      */
-    function reloadDocument(doc) {
+    function reloadCSSForDocument(doc) {
         var style = styleForURL(doc.url);
         console.assert(style, "Style Sheet for document not loaded: " + doc.url);
         Inspector.CSS.setStyleSheetText(style.styleSheetId, doc.getText());
     }
+
+    /** Empties a CSS style sheet given a document that has been deleted
+     * @param {Document} document
+     */
+    function clearCSSForDocument(doc) {
+        var style = styleForURL(doc.url);
+        console.assert(style, "Style Sheet for document not loaded: " + doc.url);
+        Inspector.CSS.setStyleSheetText(style.styleSheetId, "");
+    }
 
     /** Initialize the agent */
     function load() {
@@ -75,7 +84,8 @@ define(function CSSAgent(require, exports, module) {
     // Export public functions
     exports.styleForURL = styleForURL;
     exports.getStylesheetURLs = getStylesheetURLs;
-    exports.reloadDocument = reloadDocument;
+    exports.reloadCSSForDocument = reloadCSSForDocument;
+    exports.clearCSSForDocument = clearCSSForDocument;
     exports.load = load;
     exports.unload = unload;
 });

src/LiveDevelopment/Documents/CSSDocument.js

@@ -19,6 +19,12 @@
  * CSSDocument supports highlighting nodes from the HighlightAgent and
  * highlighting all DOMNode corresponding to the rule at the cursor position
  * in the editor.
+ *
+ * # EVENTS
+ *
+ * CSSDocument dispatches these events:
+ *  deleted - When the file for the underlying Document has been deleted. The
+ *      2nd argument to the listener will be this CSSDocument.
  */
 define(function CSSDocumentModule(require, exports, module) {
     'use strict';
@@ -47,7 +53,9 @@ define(function CSSDocumentModule(require, exports, module) {
         // Add a ref to the doc since we're listening for change events
         this.doc.addRef();
         this.onChange = this.onChange.bind(this);
+        this.onDeleted = this.onDeleted.bind(this);
         $(this.doc).on("change", this.onChange);
+        $(this.doc).on("deleted", this.onDeleted);
 
 /*
         $(this.editor).on("cursorActivity", this.onCursorActivity);
@@ -65,13 +73,14 @@ define(function CSSDocumentModule(require, exports, module) {
 
         // If the CSS document is dirty, push the changes into the browser now
         if (doc.isDirty) {
-            CSSAgent.reloadDocument(this.doc);
+            CSSAgent.reloadCSSForDocument(this.doc);
         }
     };
 
     /** Close the document */
     CSSDocument.prototype.close = function close() {
         $(this.doc).off("change", this.onChange);
+        $(this.doc).off("deleted", this.onDeleted);
         this.doc.releaseRef();
 /*
         Inspector.off("HighlightAgent.highlight", this.onHighlight);
@@ -109,10 +118,19 @@ define(function CSSDocumentModule(require, exports, module) {
         }
     };
 
-    /** Triggered on change of the editor */
+    /** Triggered whenever the Document is edited */
     CSSDocument.prototype.onChange = function onChange(event, editor, change) {
         // brute force: update the CSS
-        CSSAgent.reloadDocument(this.doc);
+        CSSAgent.reloadCSSForDocument(this.doc);
+    };
+    /** Triggered if the Document's file is deleted */
+    CSSDocument.prototype.onDeleted = function onDeleted(event, editor, change) {
+        // clear the CSS
+        CSSAgent.clearCSSForDocument(this.doc);
+
+        // shut down, since our Document is now dead
+        this.close();
+        $(this).triggerHandler("deleted", [this]);
     };
 
     /** Triggered by the HighlightAgent to highlight a node in the editor */

src/LiveDevelopment/LiveDevelopment.js

@@ -133,11 +133,25 @@ define(function LiveDevelopment(require, exports, module) {
         if (_relatedDocuments) {
             _relatedDocuments.forEach(function (liveDoc) {
                 liveDoc.close();
+                $(liveDoc).off("deleted", _handleRelatedDocumentDeleted);
             });
             _relatedDocuments = undefined;
         }
     }
 
+    /**
+     * Removes the given CSS/JSDocument from _relatedDocuments. Signals that the
+     * given file is no longer associated with the HTML document that is live (e.g.
+     * if the related file has been deleted on disk).
+     */
+    function _handleRelatedDocumentDeleted(event, liveDoc) {
+        var index = _relatedDocuments.indexOf(liveDoc);
+        if (index !== -1) {
+            $(liveDoc).on("deleted", _handleRelatedDocumentDeleted);
+            _relatedDocuments.splice(index, 1);
+        }
+    }
+
     /** Create a live version of a Brackets document */
     function _createDocument(doc, editor) {
         var DocClass = _classForDocument(doc);
@@ -179,6 +193,7 @@ define(function LiveDevelopment(require, exports, module) {
                     var liveDoc = _createDocument(doc);
                     if (liveDoc) {
                         _relatedDocuments.push(liveDoc);
+                        $(liveDoc).on("deleted", _handleRelatedDocumentDeleted);
                     }
                 });
         });

src/document/DocumentCommandHandlers.js

@@ -338,6 +338,31 @@ define(function (require, exports, module) {
         );
     }
 
+    /**
+     * Reverts the Document to the current contents of its file on disk. Discards any unsaved changes
+     * in the Document.
+     * @param {Document} doc
+     * @return {$.Promise} a Promise that's resolved when done, or rejected with a FileError if the
+     *      file cannot be read (after showing an error dialog to the user).
+     */
+    function doRevert(doc) {
+        var result = new $.Deferred();
+
+        FileUtils.readAsText(doc.file)
+            .done(function (text, readTimestamp) {
+                doc.refreshText(text, readTimestamp);
+                result.resolve();
+            })
+            .fail(function (error) {
+                FileUtils.showFileOpenError(error.code, doc.file.fullPath)
+                    .always(function () {
+                        result.reject(error);
+                    });
+            });
+
+        return result.promise();
+    }
+
 
     /**
      * Closes the specified file: removes it from the working set, and closes the main editor if one
@@ -395,6 +420,7 @@ define(function (require, exports, module) {
                 if (id === Dialogs.DIALOG_BTN_CANCEL) {
                     result.reject();
                 } else if (id === Dialogs.DIALOG_BTN_OK) {
+                    // "Save" case: wait until we confirm save has succeeded before closing
                     doSave(doc)
                         .done(function () {
                             doClose(file);
@@ -404,13 +430,18 @@ define(function (require, exports, module) {
                             result.reject();
                         });
                 } else {
-                    // This is the "Don't Save" case--we can just go ahead and close the file.
-                    // FUTURE: If other views of the Document will remain open, we need to revert the
-                    // Document to the clean content on disk. Currently secondary Editors all lose
-                    // sync & close if you refresh the whole doc's text, so we just fake this rather
-                    // than pointlessly load text from disk. See hack in Document._makeNonEditable().
+                    // "Don't Save" case: even though we're closing the main editor, other views of
+                    // the Document may remain in the UI. So we need to revert the Document to a clean
+                    // copy of whatever's on disk.
                     doClose(file);
-                    result.resolve();
+
+                    // Only reload from disk if other views still exist
+                    if (DocumentManager.getOpenDocumentForPath(file.fullPath)) {
+                        doRevert(doc)
+                            .pipe(result.resolve, result.reject);
+                    } else {
+                        result.resolve();
+                    }
                 }
             });
             result.always(function () {

src/document/DocumentManager.js

@@ -440,15 +440,6 @@ define(function (require, exports, module) {
         } else {
             this._text = this.getText();
             this._masterEditor = null;
-
-            // FUTURE: If main editor was closed without saving changes, we should revert _text to
-            // what's on disk. But since we currently close all secondary editors when anyone else
-            // touches the Document content, there's no point in doing that yet. Just change the text
-            // to a dummy value to trigger that closing. Ultimately, the nicer "revert" behavior
-            // should probably live in DocumentCommandHandlers.handleFileClose() (see note there).
-            if (this.isDirty) {
-                this.refreshText("");
-            }
         }
     };
 
@@ -644,6 +635,11 @@ define(function (require, exports, module) {
         if (doc) {
             $(doc).triggerHandler("deleted");
         }
+
+        // At this point, all those other views SHOULD have released the Doc
+        if (doc && doc._refCount > 0) {
+            console.log("WARNING: deleted Document still has " + doc._refCount + " references. Did someone addRef() without listening for 'deleted'?");
+        }
     }
 
 

test/spec/InlineEditorProviders-test.js

@@ -528,7 +528,7 @@ define(function (require, exports, module) {
                 });
 
                 // FUTURE: Eventually we'll instead want it to stay open and revert to the content on disk.
-                // See notes in Document._makeNonEditable() & DocumentCommandHandlers.handleFileClose().
+                // Editor's syncing code can't yet handle blowing away the whole Document like that, though.
                 it("should close inline when file is closed without saving changes", function () {
                     initInlineTest("test1.html", 1);