Merge pull request #1037 from atom/cj-buffer-serialization

When opening a window, only use serialized buffer text if the contents on disk have not changed.

spec/text-buffer-replication-spec.coffee

@@ -5,7 +5,6 @@ describe "TextBuffer replication", ->
 
   beforeEach ->
     buffer1 = project.buildBufferSync('sample.js')
-    buffer1.insert([0, 0], 'changed\n')
     doc1 = buffer1.getState()
     doc2 = doc1.clone(new Site(2))
     doc1.connect(doc2)
@@ -14,6 +13,9 @@ describe "TextBuffer replication", ->
     waitsFor ->
       buffer1.loaded and buffer2.loaded
 
+    runs ->
+      buffer1.insert([0, 0], 'changed\n')
+
   afterEach ->
     buffer1.destroy()
     buffer2.destroy()

spec/text-buffer-spec.coffee

@@ -949,24 +949,48 @@ describe 'TextBuffer', ->
           expect(buffer2.getText()).toBe(buffer.getText())
 
     describe "when the serialized buffer had unsaved changes", ->
-      it "restores the previous unsaved state of the buffer", ->
-        previousText = buffer.getText()
-        buffer.setText("abc")
+      describe "when the disk contents were changed since serialization", ->
+        it "loads the disk contents instead of the previous unsaved state", ->
+          buffer.release()
 
-        state = buffer.serialize()
-        expect(state.getObject('text')).toBe 'abc'
+          filePath = temp.openSync('atom').path
+          fs.writeSync(filePath, "words")
+          {buffer} = project.openSync(filePath)
+          buffer.setText("BUFFER CHANGE")
 
-        buffer2 = deserialize(state, {project})
+          state = buffer.serialize()
+          expect(state.getObject('text')).toBe 'BUFFER CHANGE'
+          fs.writeSync(filePath, "DISK CHANGE")
 
-        waitsFor ->
-          buffer2.cachedDiskContents
+          buffer2 = deserialize(state, {project})
 
-        runs ->
-          expect(buffer2.getPath()).toBe(buffer.getPath())
-          expect(buffer2.getText()).toBe(buffer.getText())
-          expect(buffer2.isModified()).toBeTruthy()
-          buffer2.setText(previousText)
-          expect(buffer2.isModified()).toBeFalsy()
+          waitsFor ->
+            buffer2.cachedDiskContents
+
+          runs ->
+            expect(buffer2.getPath()).toBe(buffer.getPath())
+            expect(buffer2.getText()).toBe("DISK CHANGE")
+            expect(buffer2.isModified()).toBeFalsy()
+
+      describe "when the disk contents are the same since serialization", ->
+        it "restores the previous unsaved state of the buffer", ->
+          previousText = buffer.getText()
+          buffer.setText("abc")
+
+          state = buffer.serialize()
+          expect(state.getObject('text')).toBe 'abc'
+
+          buffer2 = deserialize(state, {project})
+
+          waitsFor ->
+            buffer2.cachedDiskContents
+
+          runs ->
+            expect(buffer2.getPath()).toBe(buffer.getPath())
+            expect(buffer2.getText()).toBe(buffer.getText())
+            expect(buffer2.isModified()).toBeTruthy()
+            buffer2.setText(previousText)
+            expect(buffer2.isModified()).toBeFalsy()
 
     describe "when the serialized buffer was unsaved and had no path", ->
       it "restores the previous unsaved state of the buffer", ->

spec/tokenized-buffer-spec.coffee

@@ -350,7 +350,8 @@ describe "TokenizedBuffer", ->
   describe "when the buffer contains surrogate pairs", ->
     beforeEach ->
       atom.activatePackage('language-javascript', sync: true)
-      buffer = project.buildBufferSync 'sample-with-pairs.js',  """
+      buffer = project.buildBufferSync 'sample-with-pairs.js'
+      buffer.setText """
         'abc\uD835\uDF97def'
         //\uD835\uDF97xyz
       """
@@ -389,7 +390,8 @@ describe "TokenizedBuffer", ->
       atom.activatePackage('language-ruby-on-rails', sync: true)
       atom.activatePackage('language-ruby', sync: true)
 
-      buffer = project.bufferForPathSync(null, "<div class='name'><%= User.find(2).full_name %></div>")
+      buffer = project.bufferForPathSync()
+      buffer.setText "<div class='name'><%= User.find(2).full_name %></div>"
       tokenizedBuffer = new TokenizedBuffer({buffer})
       tokenizedBuffer.setGrammar(syntax.selectGrammar('test.erb'))
       fullyTokenize(tokenizedBuffer)

src/file.coffee

@@ -1,3 +1,4 @@
+crypto = require 'crypto'
 path = require 'path'
 pathWatcher = require 'pathwatcher'
 Q = require 'q'
@@ -68,6 +69,9 @@ class File
     else
       @cachedContents
 
+    @setDigest(@cachedContents)
+    @cachedContents
+
   # Public: Reads the contents of the file.
   #
   # * flushCache:
@@ -101,12 +105,19 @@ class File
       promise = Q(@cachedContents)
 
     promise.then (contents) =>
+      @setDigest(contents)
       @cachedContents = contents
 
   # Public: Returns whether the file exists.
   exists: ->
     fsUtils.exists(@getPath())
 
+  setDigest: (contents) ->
+    @digest = crypto.createHash('sha1').update(contents ? '').digest('hex')
+
+  getDigest: ->
+    @digest ? @setDigest(@readSync())
+
   # Private:
   handleNativeChangeEvent: (eventType, path) ->
     if eventType is "delete"

src/project.coffee

@@ -224,13 +224,13 @@ class Project
     existingBuffer?.isModified()
 
   # Private: Only to be used in specs
-  bufferForPathSync: (filePath, text) ->
+  bufferForPathSync: (filePath) ->
     absoluteFilePath = @resolve(filePath)
 
     if filePath
       existingBuffer = _.find @buffers, (buffer) -> buffer.getPath() == absoluteFilePath
 
-    existingBuffer ? @buildBufferSync(absoluteFilePath, text)
+    existingBuffer ? @buildBufferSync(absoluteFilePath)
 
   # Private: Given a file path, this retrieves or creates a new {TextBuffer}.
   #
@@ -238,23 +238,22 @@ class Project
   # `text` is used as the contents of the new buffer.
   #
   # filePath - A {String} representing a path. If `null`, an "Untitled" buffer is created.
-  # text - The {String} text to use as a buffer, if the file doesn't have any contents
   #
   # Returns a promise that resolves to the {TextBuffer}.
-  bufferForPath: (filePath, text) ->
+  bufferForPath: (filePath) ->
     absoluteFilePath = @resolve(filePath)
     if absoluteFilePath
       existingBuffer = _.find @buffers, (buffer) -> buffer.getPath() == absoluteFilePath
 
-    Q(existingBuffer ? @buildBuffer(absoluteFilePath, text))
+    Q(existingBuffer ? @buildBuffer(absoluteFilePath))
 
   # Private:
   bufferForId: (id) ->
     _.find @buffers, (buffer) -> buffer.id is id
 
   # Private: DEPRECATED
-  buildBufferSync: (absoluteFilePath, initialText) ->
-    buffer = new TextBuffer({project: this, filePath: absoluteFilePath, initialText})
+  buildBufferSync: (absoluteFilePath) ->
+    buffer = new TextBuffer({project: this, filePath: absoluteFilePath})
     buffer.loadSync()
     @addBuffer(buffer)
     buffer
@@ -265,8 +264,8 @@ class Project
   # text - The {String} text to use as a buffer
   #
   # Returns a promise that resolves to the {TextBuffer}.
-  buildBuffer: (absoluteFilePath, initialText) ->
-    buffer = new TextBuffer({project: this, filePath: absoluteFilePath, initialText})
+  buildBuffer: (absoluteFilePath) ->
+    buffer = new TextBuffer({project: this, filePath: absoluteFilePath})
     buffer.load().then (buffer) =>
       @addBuffer(buffer)
       buffer

src/text-buffer.coffee

@@ -1,3 +1,4 @@
+crypto = require 'crypto'
 {Emitter, Subscriber} = require 'emissary'
 guid = require 'guid'
 Q = require 'q'
@@ -38,8 +39,8 @@ class TextBuffer
 
   # Creates a new buffer.
   #
-  # path - A {String} representing the file path
-  # initialText - A {String} setting the starting text
+  # * optionsOrState - An {Object} or a telepath.Document
+  #   + filePath - A {String} representing the file path
   constructor: (optionsOrState={}, params={}) ->
     if optionsOrState instanceof telepath.Document
       {@project} = params
@@ -47,9 +48,9 @@ class TextBuffer
       @id = @state.get('id')
       filePath = @state.get('relativePath')
       @text = @state.get('text')
-      @loadFromDisk = @state.get('isModified') == false
+      @useSerializedText = @state.get('isModified') != false
     else
-      {@project, filePath, initialText} = optionsOrState
+      {@project, filePath} = optionsOrState
       @text = site.createDocument(initialText ? '', shareStrings: true)
       @id = guid.create().toString()
       @state = site.createDocument
@@ -57,7 +58,6 @@ class TextBuffer
         deserializer: @constructor.name
         version: @constructor.version
         text: @text
-      @loadFromDisk = not initialText
 
     @loaded = false
     @subscribe @text, 'changed', @handleTextChange
@@ -68,14 +68,19 @@ class TextBuffer
 
   loadSync: ->
     @updateCachedDiskContentsSync()
-    @reload() if @loadFromDisk
-    @text.clearUndoStack()
+    @finishLoading()
 
   load: ->
-    @updateCachedDiskContents().then =>
-      @reload() if @loadFromDisk
-      @text.clearUndoStack()
-      this
+    @updateCachedDiskContents().then => @finishLoading()
+
+  finishLoading: ->
+    @loaded = true
+    if @useSerializedText and @state.get('diskContentsDigest') == @file?.getDigest()
+      @emitModifiedStatusChanged(true)
+    else
+      @reload()
+    @text.clearUndoStack()
+    this
 
   ### Internal ###
 
@@ -108,6 +113,7 @@ class TextBuffer
   serialize: ->
     state = @state.clone()
     state.set('isModified', @isModified())
+    state.set('diskContentsDigest', @file.getDigest()) if @file
     for marker in state.get('text').getMarkers() when marker.isRemote()
       marker.destroy()
     state
@@ -158,13 +164,11 @@ class TextBuffer
 
   # Private: Rereads the contents of the file, and stores them in the cache.
   updateCachedDiskContentsSync: ->
-    @loaded = true
     @cachedDiskContents = @file?.readSync() ? ""
 
   # Private: Rereads the contents of the file, and stores them in the cache.
   updateCachedDiskContents: ->
     Q(@file?.read() ? "").then (contents) =>
-      @loaded = true
       @cachedDiskContents = contents
 
   # Gets the file's basename--that is, the file without any directory information.