From d310297fe7a0836691f10c8fdc4064fa939e51a8 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 25 Oct 2012 13:07:38 -0600 Subject: [PATCH] Radically simplify the definition of "modified" for buffers Now, we maintain md5 signatures for the on-disk and in-memory contents of the buffer. Whenever either contents change, we recompute the signature and store it on the buffer. We can tell if the buffer is modified by comparing these signatures. When the disk contents change, we compare the memory and disk signatures *before* recomputing the disk signature to determine whether to update the buffer or mark it as a conflict. --- spec/app/buffer-spec.coffee | 21 +++++++------- spec/app/editor-spec.coffee | 1 + src/app/buffer.coffee | 56 ++++++++++++++++--------------------- src/app/status-bar.coffee | 2 +- 4 files changed, 36 insertions(+), 44 deletions(-) diff --git a/spec/app/buffer-spec.coffee b/spec/app/buffer-spec.coffee index 4fbdab261..ec9de0b84 100644 --- a/spec/app/buffer-spec.coffee +++ b/spec/app/buffer-spec.coffee @@ -77,7 +77,7 @@ describe 'Buffer', -> waitsFor "buffer path change", -> eventHandler.callCount > 0 - describe "when the buffer's file is modified (via another process)", -> + describe "when the buffer's on-disk contents change (via another process writing to its file)", -> path = null beforeEach -> path = "/tmp/tmp.txt" @@ -98,8 +98,8 @@ describe 'Buffer', -> runs -> expect(changeHandler).not.toHaveBeenCalled() - describe "when the buffer is unmodified", -> - it "triggers 'change' event and buffer remains unmodified", -> + describe "when the buffer's memory contents are the same as the *previous* disk contents", -> + it "changes the memory contents of the buffer to match the new disk contents and triggers a 'change' event", -> changeHandler = jasmine.createSpy('changeHandler') buffer.on 'change', changeHandler fs.write(path, "second") @@ -116,8 +116,8 @@ describe 'Buffer', -> expect(event.newText).toBe "second" expect(buffer.isModified()).toBeFalsy() - describe "when the buffer is modified", -> - it "sets modifiedOnDisk to be true", -> + describe "when the buffer's memory contents differ from the *previous* disk contents", -> + it "leaves the buffer in a modified state (does not update its memory contents)", -> fileChangeHandler = jasmine.createSpy('fileChange') buffer.file.on 'contents-change', fileChangeHandler @@ -129,7 +129,7 @@ describe 'Buffer', -> fileChangeHandler.callCount > 0 runs -> - expect(buffer.isModifiedOnDisk()).toBeTruthy() + expect(buffer.isModified()).toBeTruthy() describe "when the buffer's file is deleted (via another process)", -> it "no longer has a path", -> @@ -342,14 +342,13 @@ describe 'Buffer', -> expect(-> buffer.save()).toThrow() describe "reload()", -> - it "loads text from disk are sets @modified and @modifiedOnDisk to false", -> - buffer.modified = true - buffer.modifiedOnDisk = true + it "reloads current text from disk and clears any conflicts", -> buffer.setText("abc") + buffer.conflict = true buffer.reload() - expect(buffer.modifed).toBeFalsy() - expect(buffer.modifiedOnDisk).toBeFalsy() + expect(buffer.isModified()).toBeFalsy() + expect(buffer.isInConflict()).toBeFalsy() expect(buffer.getText()).toBe(fileContents) describe ".saveAs(path)", -> diff --git a/spec/app/editor-spec.coffee b/spec/app/editor-spec.coffee index c544a5b01..7caf1fb69 100644 --- a/spec/app/editor-spec.coffee +++ b/spec/app/editor-spec.coffee @@ -155,6 +155,7 @@ describe "Editor", -> it "closes active edit session and loads next edit session", -> editor.edit(rootView.project.buildEditSessionForPath()) editSession = editor.activeEditSession + spyOn(editSession.buffer, 'isModified').andReturn false spyOn(editSession, 'destroy').andCallThrough() spyOn(editor, "remove").andCallThrough() editor.trigger "core:close" diff --git a/src/app/buffer.coffee b/src/app/buffer.coffee index 8aed86145..fb51399e4 100644 --- a/src/app/buffer.coffee +++ b/src/app/buffer.coffee @@ -8,14 +8,15 @@ UndoManager = require 'undo-manager' BufferChangeOperation = require 'buffer-change-operation' Anchor = require 'anchor' AnchorRange = require 'anchor-range' +{hex_md5} = require 'md5' module.exports = class Buffer @idCounter = 1 undoManager: null - modified: null - contentOnDisk: null - modifiedOnDisk: null + diskContentSignature: null + memoryContentSignature: null + conflict: false lines: null file: null anchors: null @@ -31,13 +32,19 @@ class Buffer if path throw "Path '#{path}' does not exist" unless fs.exists(path) @setPath(path) - @contentOnDisk = fs.read(@getPath()) - @setText(@contentOnDisk) + @setText(fs.read(@getPath())) + @computeDiskContentSignature() else @setText('') + @computeMemoryContentSignature() @undoManager = new UndoManager(this) - @modified = false + + computeDiskContentSignature: -> + @diskContentSignature = fs.md5ForPath(@getPath()) + + computeMemoryContentSignature: -> + @memoryContentSignature = hex_md5(@getText()) destroy: -> throw new Error("Destroying buffer twice with path '#{@getPath()}'") if @destroyed @@ -57,10 +64,12 @@ class Buffer subscribeToFile: -> @file.on "contents-change", => if @isModified() - @modifiedOnDisk = true + @conflict = true + @computeDiskContentSignature() + @trigger "contents-change-on-disk" else @setText(fs.read(@file.getPath())) - @modified = false + @computeDiskContentSignature() @file.on "remove", => @file = null @@ -70,10 +79,7 @@ class Buffer @trigger "path-change", this reload: -> - @contentOnDisk = fs.read(@getPath()) - @setText(@contentOnDisk) - @modified = false - @modifiedOnDisk = false + @setText(fs.read(@getPath())) getBaseName: -> @file?.getBaseName() @@ -87,13 +93,7 @@ class Buffer @file?.off() @file = new File(path) @subscribeToFile() - @file.on "contents-change", => - if @isModified() - @modifiedOnDisk = true - @trigger "contents-change-on-disk" - else - @setText(fs.read(@file.getPath())) - @modified = false + @trigger "path-change", this getExtension: -> @@ -209,7 +209,8 @@ class Buffer replaceLines: (startRow, endRow, newLines) -> @lines[startRow..endRow] = newLines - @modified = true + @computeMemoryContentSignature() + @conflict = false unless @isModified() pushOperation: (operation, editSession) -> if @undoManager @@ -235,23 +236,14 @@ class Buffer @trigger 'before-save' fs.write path, @getText() @file?.updateMd5() - @modified = false - @modifiedOnDisk = false @setPath(path) - @contentOnDisk = @getText() + @computeDiskContentSignature() @trigger 'after-save' - isInConflict: -> - @isModified() and @isModifiedOnDisk() - - isModifiedOnDisk: -> - @modifiedOnDisk - isModified: -> - @modified + @memoryContentSignature != @diskContentSignature - contentDifferentOnDisk: -> - @contentOnDisk != @getText() + isInConflict: -> @conflict getAnchors: -> new Array(@anchors...) diff --git a/src/app/status-bar.coffee b/src/app/status-bar.coffee index 658ca7f0c..ed667a179 100644 --- a/src/app/status-bar.coffee +++ b/src/app/status-bar.coffee @@ -41,7 +41,7 @@ class StatusBar extends View @updateBufferModifiedText() updateBufferModifiedText: -> - if @buffer.isModified() and @buffer.contentDifferentOnDisk() + if @buffer.isModified() @bufferModified.text('*') else @bufferModified.text('')