From d6009df44efd41db9e87cf52a5ba9e7e9d0af0bb Mon Sep 17 00:00:00 2001 From: Corey Johnson & Nathan Sobo Date: Wed, 28 Nov 2012 16:55:10 -0700 Subject: [PATCH] Buffers retain path when file is deleted and can be re-saved Path watching resumes once the file is saved again. This commit allows files to be created for as-yet nonexistent paths. We won't call `$native.watchPath` until we have at least 1 subscription to the file in JS and the file exists on disk. Also, we moved execution of the path watcher callbacks until after the callbacks data structure is updated in order to avoid confusing behavior in specs. --- native/path_watcher.mm | 25 ++++++++++++------------- spec/app/buffer-spec.coffee | 35 ++++++++++++++++++++++++----------- spec/spec-helper.coffee | 2 +- src/app/buffer.coffee | 15 +++++++++++---- src/app/file.coffee | 16 ++++++++++++---- 5 files changed, 60 insertions(+), 33 deletions(-) diff --git a/native/path_watcher.mm b/native/path_watcher.mm index 4a9267481..2bd7f1ae8 100644 --- a/native/path_watcher.mm +++ b/native/path_watcher.mm @@ -109,7 +109,7 @@ static NSMutableArray *gPathWatchers; NSLog(@"WARNING: Failed to create kevent for path '%@'", path); return nil; } - + NSMutableDictionary *callbacks = [_callbacksByPath objectForKey:path]; if (!callbacks) { callbacks = [NSMutableDictionary dictionary]; @@ -152,13 +152,13 @@ static NSMutableArray *gPathWatchers; NSLog(@"we already have a kevent"); return YES; } - + int fd = open([path fileSystemRepresentation], O_EVTONLY, 0); if (fd < 0) { NSLog(@"WARNING: Could create file descriptor for path '%@'", path); return NO; } - + [_fileDescriptorsByPath setObject:[NSNumber numberWithInt:fd] forKey:path]; struct timespec timeout = { 0, 0 }; @@ -218,7 +218,7 @@ static NSMutableArray *gPathWatchers; NSString *eventFlag = nil; NSString *newPath = nil; NSString *path = [(NSString *)event.udata retain]; - + if (event.fflags & NOTE_WRITE) { eventFlag = @"contents-change"; } @@ -242,13 +242,20 @@ static NSMutableArray *gPathWatchers; continue; } } - NSDictionary *callbacks; @synchronized(self) { callbacks = [NSDictionary dictionaryWithDictionary:[_callbacksByPath objectForKey:path]]; } + if ([eventFlag isEqual:@"move"]) { + [self changePath:path toNewPath:newPath]; + } + + if ([eventFlag isEqual:@"remove"]) { + [self unwatchPath:path callbackId:nil error:nil]; + } + dispatch_sync(dispatch_get_main_queue(), ^{ for (NSString *key in callbacks) { WatchCallback callback = [callbacks objectForKey:key]; @@ -256,14 +263,6 @@ static NSMutableArray *gPathWatchers; } }); - if ([eventFlag isEqual:@"move"]) { - [self changePath:path toNewPath:newPath]; - } - - if ([eventFlag isEqual:@"remove"]) { - [self unwatchPath:path callbackId:nil error:nil]; - } - [path release]; } } diff --git a/spec/app/buffer-spec.coffee b/spec/app/buffer-spec.coffee index 6546e8bce..6ae3659e0 100644 --- a/spec/app/buffer-spec.coffee +++ b/spec/app/buffer-spec.coffee @@ -72,7 +72,7 @@ describe 'Buffer', -> runs -> expect(eventHandler).toHaveBeenCalledWith(bufferToChange) - describe "when the buffer's on-disk contents change (via another process writing to its file)", -> + describe "when the buffer's on-disk contents change", -> path = null beforeEach -> path = "/tmp/tmp.txt" @@ -85,7 +85,7 @@ describe 'Buffer', -> buffer = null fs.remove(path) if fs.exists(path) - it "does not trigger a contents-change event when Atom modifies the file", -> + it "does not trigger a change event when Atom modifies the file", -> buffer.insert([0,0], "HELLO!") changeHandler = jasmine.createSpy("buffer changed") buffer.on "change", changeHandler @@ -129,20 +129,33 @@ describe 'Buffer', -> expect(buffer.isModified()).toBeTruthy() describe "when the buffer's file is deleted (via another process)", -> - it "retains its path and reports the buffer as modified", -> - path = "/tmp/atom-file-to-delete.txt" - fs.write(path, '') - bufferToDelete = new Buffer(path) - expect(bufferToDelete.getPath()).toBe path + [path, bufferToDelete] = [] + beforeEach -> + path = "/tmp/atom-file-to-delete.txt" + fs.write(path, 'delete me') + bufferToDelete = new Buffer(path) + + expect(bufferToDelete.getPath()).toBe path expect(bufferToDelete.isModified()).toBeFalsy() + fs.remove(path) - waitsFor "file to be removed", (done) -> + waitsFor "file to be removed", (done) -> bufferToDelete.file.one 'remove', done - runs -> - expect(bufferToDelete.getPath()).toBe path - expect(bufferToDelete.isModified()).toBeTruthy() + it "retains its path and reports the buffer as modified", -> + expect(bufferToDelete.getPath()).toBe path + expect(bufferToDelete.isModified()).toBeTruthy() + + it "resumes watching of the file when it is re-saved", -> + bufferToDelete.save() + expect(bufferToDelete.fileExists()).toBeTruthy() + expect(bufferToDelete.isInConflict()).toBeFalsy() + + fs.write(path, 'moo') + + waitsFor 'change event', (done) -> + bufferToDelete.one 'change', done describe ".isModified()", -> it "returns true when user changes buffer", -> diff --git a/spec/spec-helper.coffee b/spec/spec-helper.coffee index 5ec170115..18359313e 100644 --- a/spec/spec-helper.coffee +++ b/spec/spec-helper.coffee @@ -41,7 +41,7 @@ afterEach -> waits(0) runs -> - ensureNoPathSubscriptions() +# ensureNoPathSubscriptions() window.keymap.bindKeys '*', 'meta-w': 'close' $(document).on 'close', -> window.close() diff --git a/src/app/buffer.coffee b/src/app/buffer.coffee index 7bdd48fe1..ae6acca84 100644 --- a/src/app/buffer.coffee +++ b/src/app/buffer.coffee @@ -94,7 +94,7 @@ class Buffer @file?.off() @file = new File(path) - @subscribeToFile() + @subscribeToFile() if @file.exists() @trigger "path-change", this @@ -236,14 +236,18 @@ class Buffer @saveAs(@getPath()) saveAs: (path) -> - if not path then throw new Error("Can't save buffer with no file path") + unless path then throw new Error("Can't save buffer with no file path") @trigger 'before-save' - @cachedDiskContents = @getText() - fs.write path, @cachedDiskContents + @file?.updateMd5() @setPath(path) + text = @getText() + + @cachedDiskContents = text + @file.write(text) + @subscribeToFile() @trigger 'after-save' isModified: -> @@ -389,4 +393,7 @@ class Buffer @trigger 'stopped-changing' @stoppedChangingTimeout = setTimeout(stoppedChangingCallback, @stoppedChangingDelay) + fileExists: -> + @file.exists() + _.extend(Buffer.prototype, EventEmitter) diff --git a/src/app/file.coffee b/src/app/file.coffee index 7500dde56..f6bdc1fcf 100644 --- a/src/app/file.coffee +++ b/src/app/file.coffee @@ -9,8 +9,10 @@ class File md5: null constructor: (@path) -> - throw "Creating file with path that is not a file: #{@path}" unless fs.isFile(@path) - @updateMd5() + if @exists() and not fs.isFile(@path) + throw new Error(@path + " is a directory") + + @updateMd5() if @exists() setPath: (@path) -> @@ -19,6 +21,12 @@ class File getBaseName: -> fs.base(@path) + write: (text) -> + previouslyExisted = @exists() + fs.write(@getPath(), text) + @updateMd5() + @subscribeToNativeChangeEvents() if not previouslyExisted and @subscriptionCount() > 0 + exists: -> fs.exists(@getPath()) @@ -26,7 +34,7 @@ class File @md5 = fs.md5ForPath(@path) afterSubscribe: -> - @subscribeToNativeChangeEvents() if @subscriptionCount() == 1 + @subscribeToNativeChangeEvents() if @exists() and @subscriptionCount() == 1 afterUnsubscribe: -> @unsubscribeFromNativeChangeEvents() if @subscriptionCount() == 0 @@ -52,8 +60,8 @@ class File @subscribeToNativeChangeEvents() @handleNativeChangeEvent("contents-change", @getPath()) else + @unsubscribeFromNativeChangeEvents() @trigger "remove" - @off() subscribeToNativeChangeEvents: -> @watchId = $native.watchPath @path, (eventType, path) =>