diff --git a/spec/text-buffer-io-spec.js b/spec/text-buffer-io-spec.js index 3f2cf8a9a..1254b0d07 100644 --- a/spec/text-buffer-io-spec.js +++ b/spec/text-buffer-io-spec.js @@ -14,7 +14,7 @@ const winattr = require('winattr') process.on('unhandledRejection', console.error) async function wait (ms) { - return new Promise(r => setTimeout(r, ms)); + return new Promise(r => setTimeout(r, ms)) } describe('TextBuffer IO', () => { @@ -34,7 +34,7 @@ describe('TextBuffer IO', () => { // `await` briefly to allow the file watcher to clean up. This is a // `pathwatcher` requirement that we can fix by updating its API — but // that's a can of worms we don't want to open yet. - await wait(10); + await wait(10) }) describe('.load', () => { @@ -408,6 +408,13 @@ describe('TextBuffer IO', () => { expect(buffer.isInConflict()).toBe(true) await buffer.save() expect(buffer.isInConflict()).toBe(false) + // Ensure we don't get flipped into conflicted status after the + // `onDidChange` handler comes through… + await wait(1000) + expect(buffer.isInConflict()).toBe(false) + buffer.setText('q') + // …and the buffer is modified again. + expect(buffer.isInConflict()).toBe(false) done() }) }) @@ -574,8 +581,9 @@ describe('TextBuffer IO', () => { }) describe('.isModified', () => { + let filePath beforeEach(async done => { - const filePath = temp.openSync('atom').path + filePath = temp.openSync('atom').path fs.writeFileSync(filePath, '') buffer = await TextBuffer.load(filePath) done() @@ -600,10 +608,23 @@ describe('TextBuffer IO', () => { buffer.undo() buffer.undo() expect(buffer.isModified()).toBe(false) + expect(buffer.isDeleted()).toBe(false) await stopChangingPromise() expect(modifiedStatusChanges).toEqual([true, false]) done() }) + + describe('and the file is deleted', () => { + it('reports the modified status as true', async () => { + buffer.setText(`lorem ipsum`) + await buffer.save() + buffer.setText(`lorem ipsum dolor`) + fs.unlinkSync(filePath) + await wait(500) + expect(buffer.isModified()).toBe(true) + expect(buffer.isDeleted()).toBe(true) + }) + }) }) describe('when the buffer is saved', () => { @@ -620,6 +641,54 @@ describe('TextBuffer IO', () => { expect(modifiedStatusChanges).toEqual([false]) done() }) + + describe('and the file is deleted', () => { + it('reports the modified status as false', async () => { + buffer.setText(`lorem ipsum`) + await buffer.save() + fs.unlinkSync(filePath) + await wait(500) + expect(buffer.isModified()).toBe(false) + expect(buffer.isDeleted()).toBe(true) + }) + + it('initially reports the modified status as false, but flips it back to true if the user makes further changes', async () => { + buffer.setText(`lorem ipsum`) + await buffer.save() + fs.unlinkSync(filePath) + await wait(500) + expect(buffer.isModified()).toBe(false) + expect(buffer.isDeleted()).toBe(true) + + buffer.insert([0, 0], '! ') + expect(buffer.isModified()).toBe(true) + expect(buffer.isDeleted()).toBe(true) + + // `isModified` should return `true` even if we revert the buffer's + // contents to what they were at the time of deletion. + buffer.setText(`lorem ipsum`) + expect(buffer.isModified()).toBe(true) + expect(buffer.isDeleted()).toBe(true) + }) + + describe('and re-saved', () => { + it('results in isModified and isDeleted no longer returning true', async () => { + buffer.setText(`lorem ipsum`) + await buffer.save() + fs.unlinkSync(filePath) + await wait(500) + buffer.insert([0, 0], '! ') + expect(buffer.isModified()).toBe(true) + expect(buffer.isDeleted()).toBe(true) + + await buffer.saveAs(filePath) + + expect(buffer.isModified()).toBe(false) + expect(buffer.isDeleted()).toBe(false) + }) + }) + }) + }) describe('when the buffer is reloaded', () => { @@ -1080,7 +1149,18 @@ describe('TextBuffer IO', () => { fs.removeSync(filePath) buffer.file.onDidDelete(() => { expect(buffer.getPath()).toBe(filePath) - expect(buffer.isModified()).toBeTruthy() + // `buffer.isModified` used to report `true` automatically whenever + // a buffer does not have a backing file. Now it depends on whether + // the file ever existed – and, if so, whether the buffer was in a + // modified state when the file was deleted. + // + // The narrow exception we're carving out is one where the file + // contents were in sync with the buffer contents at the moment of + // file deletion. If so, its `isModified=false` status will persist + // until even one edit is made, at which point it will flip back to + // `isModified=true` until the buffer is destroyed or once again + // saved to disk. + expect(buffer.isModified()).toBeFalsy() done() }) }) diff --git a/src/text-buffer.js b/src/text-buffer.js index 19a1bcbb2..14db2c280 100644 --- a/src/text-buffer.js +++ b/src/text-buffer.js @@ -102,6 +102,15 @@ class TextBuffer { this.cachedHasAstral = null this._emittedWillChangeEvent = false + // Whether a buffer has ever had a backing file, whether or not it exists + // now. + this.didHaveFileOnDisk = false + + // When a buffer's backing file is deleted while the file is unmodified, + // this trait flips to `true`… and then flips back to `false` if any + // further edits are made. + this.retainsUnmodifiedTraitAfterDeletion = false + this.setEncoding(params.encoding) this.setPreferredLineEnding(params.preferredLineEnding) @@ -524,6 +533,11 @@ class TextBuffer { // // Returns a {Boolean}. isModified () { + if (this.isDeleted()) { + // We typically consider a deleted file to be modified… unless it was + // unmodified at the time of deletion and has not been modified since. + return !this.retainsUnmodifiedTraitAfterDeletion + } if (this.file) { return !this.file.existsSync() || this.buffer.isModified() } else { @@ -531,8 +545,20 @@ class TextBuffer { } } - // Public: Determine if the in-memory contents of the buffer conflict with the - // on-disk contents of its associated file. + // Public: Determine if the buffer is in a deleted state — meaning that it + // was previously backed by a file on disk, but is no longer. + isDeleted () { + let hasNoFile = !this.file || !this.file.existsSync() + return hasNoFile && this.didHaveFileOnDisk + } + + // Public: Determine if the in-memory contents of the buffer conflict with + // the on-disk contents of its associated file. + // + // This happens if the contents of a buffer’s backing file change while the + // editor has uncommitted changes. Those uncommitted changes build upon a + // state that is now stale; if those changes were committed to disk, it could + // clobber the changes made by the external program. // // Returns a {Boolean}. isInConflict () { @@ -845,6 +871,7 @@ class TextBuffer { if (undo != null) { Grim.deprecate('The `undo` option is deprecated. Call groupLastChanges() on the TextBuffer afterward instead.') } + this.retainsUnmodifiedTraitAfterDeletion = false if (this.transactCallDepth === 0) { const newRange = this.transact(() => this.setTextInRange(range, newText, {normalizeLineEndings})) @@ -1929,6 +1956,7 @@ class TextBuffer { try { await this.buffer.save(destination, this.getEncoding()) + this.didHaveFileOnDisk = true } catch (error) { if (error.code !== 'EACCES' || destination !== filePath) throw error @@ -2101,6 +2129,8 @@ class TextBuffer { Grim.deprecate('The .load instance method is deprecated. Create a loaded buffer using TextBuffer.load(filePath) instead.') } + this.didHaveFileOnDisk = true + const source = this.file instanceof File ? this.file.getPath() : this.file.createReadStream() @@ -2273,9 +2303,20 @@ class TextBuffer { if (this.isModified()) { const source = this.file.getPath() if (!(await this.buffer.baseTextMatchesFile(source, this.getEncoding()))) { + // Emit `did-conflict` and take no other action. We will keep the + // current buffer contents so that the user's changes are not lost. this.emitter.emit('did-conflict') + } else { + // Despite being modified, we're once again in alignment with what + // is on disk. This file is not in conflict. + this.fileHasChangedSinceLastLoad = false } } else { + // This buffer was previously in sync with what was on disk, so we + // can update its contents to match the new contents on disk. By + // definition, this means there is no conflict, so we'll reset the + // appropriate flag. + this.fileHasChangedSinceLastLoad = false return this.load({internal: true}) } }, this.fileChangeDelay))) @@ -2284,6 +2325,7 @@ class TextBuffer { if (this.file.onDidDelete) { this.fileSubscriptions.add(this.file.onDidDelete(() => { const modified = this.buffer.isModified() + this.retainsUnmodifiedTraitAfterDeletion = !modified this.emitter.emit('did-delete') if (!modified && this.shouldDestroyOnFileDelete()) { return this.destroy()