From e427e9efc6adc53a9924b9d5eb224d67cd606ac4 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sun, 31 May 2026 16:24:33 -0700 Subject: [PATCH 1/2] =?UTF-8?q?Add=20the=20ability=20to=20trigger=20?= =?UTF-8?q?=E2=80=9Cresurrection=E2=80=9D=20logic=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …to make it easier for other tools to tell us when one of our `isDeleted()` buffers has been resurrected. --- spec/text-buffer-io-spec.js | 42 ++++++++++++++++++ src/text-buffer.js | 86 +++++++++++++++++++++++++++++++++++-- 2 files changed, 124 insertions(+), 4 deletions(-) diff --git a/spec/text-buffer-io-spec.js b/spec/text-buffer-io-spec.js index 9db5bdc58..72f77e8b5 100644 --- a/spec/text-buffer-io-spec.js +++ b/spec/text-buffer-io-spec.js @@ -680,6 +680,26 @@ describe('TextBuffer IO', () => { await wait(500) expect(buffer.isModified()).toBe(false) expect(buffer.isDeleted()).toBe(true) + + // Simulate an external program recreating the file. + const deletedStatusChanges = [] + let reloadedCount = 0 + let conflictedCount = 0 + buffer.onDidChangeDeleted((status) => deletedStatusChanges.push(status)) + buffer.onDidReload(() => reloadedCount++) + buffer.onDidConflict(() => conflictedCount++) + fs.writeFileSync(filePath, `lorem`) + + // Calling `resurrect` triggers the logic that would take place + // automatically if we were able to detect our own file + // resurrections. + await buffer.resurrect() + + expect(deletedStatusChanges.length).toBe(1) + expect(deletedStatusChanges[0]).toBe(false) + expect(reloadedCount).toBe(1) + expect(conflictedCount).toBe(0) + expect(buffer.isDeleted()).toBe(false) }) it('initially reports the modified status as false, but flips it back to true if the user makes further changes', async () => { @@ -699,6 +719,28 @@ describe('TextBuffer IO', () => { buffer.setText(`lorem ipsum`) expect(buffer.isModified()).toBe(true) expect(buffer.isDeleted()).toBe(true) + + // Now let's add some uncommitted changes in order to complicate the + // resurrection of the file. + buffer.setText('ipsum lorem') + + let reloadedCount = 0 + let conflictedCount = 0 + + // Simulate an external program recreating the file. + buffer.onDidReload(() => reloadedCount++) + buffer.onDidConflict(() => conflictedCount++) + fs.writeFileSync(filePath, `lorem ipsum`) + + // Calling `resurrect` triggers the logic that would take place + // automatically if we were able to detect our own file + // resurrections. + await buffer.resurrect() + + expect(reloadedCount).toBe(1) + expect(conflictedCount).toBe(1) + expect(buffer.isDeleted()).toBe(false) + expect(buffer.isModified()).toBe(true) }) describe('and re-saved', () => { diff --git a/src/text-buffer.js b/src/text-buffer.js index 195270ca3..3dede5aff 100644 --- a/src/text-buffer.js +++ b/src/text-buffer.js @@ -390,6 +390,28 @@ class TextBuffer { return this.emitter.on('did-change-modified', callback) } + // Public: Invoke the given callback when the buffer's "deleted" state is + // changed. + // + // Unlike {::onDidDelete}, this callback will fire when a buffer's "deleted" + // state is removed, not just when it is added. + // + // Returns a {Disposable} on which `.dispose()` can be called to unsubscribe. + onDidChangeDeleted (callback) { + return this.emitter.on('did-change-deleted', callback) + } + + // Public: Invoke the given callback when the buffer's "conflicted" state is + // changed. + // + // Unlike {::onDidConflict}, this callback will fire when a buffer's + // "conflicted" state is removed, not just when it is added. + // + // Returns a {Disposable} on which `.dispose()` can be called to unsubscribe. + onDidChangeConflicted (callback) { + return this.emitter.on('did-change-conflicted', callback) + } + // Public: Invoke the given callback when all marker `::onDidChange` // observers have been notified following a change to the buffer. // @@ -2029,6 +2051,7 @@ class TextBuffer { this.setFile(file) this.fileHasChangedSinceLastLoad = false + this.emitConflictedStatusChanged(false) this.digestWhenLastPersisted = this.buffer.baseTextDigest() this.loaded = true this.emitModifiedStatusChanged(false) @@ -2043,6 +2066,27 @@ class TextBuffer { return this.load({discardChanges: true, internal: true}) } + // Extended: Notify a text buffer that its backing file has been restored and + // trigger a reload. + // + // `TextBuffer` uses a file-watcher library to detect when a file on disk is + // changed, renamed, or deleted so that it can update its internal state + // accordingly. But that library does not detect changes recursively, so + // `TextBuffer` cannot detect on its own when a previously deleted file is + // re-created by an external tool. + // + // Instead, this method exists as an easy way for an external file-watcher to + // signal to `TextBuffer` that this buffer's backing file has been + // resurrected, and that certain events ought to be emitted. + resurrect () { + if (this.isDeleted()) return + this.emitDeletedStatusChanged(false) + // This is like calling `reload`, but less aggressive; it will preserve + // uncommitted buffer contents and emit a `did-conflict` event instead of + // clobbering those changes. + return this.load({discardChanges: false, internal: true}) + } + /* Section: Display Layers */ @@ -2183,15 +2227,24 @@ class TextBuffer { let checkpoint = null let patch try { + let force = options?.discardChanges patch = await this.buffer.load( source, { encoding: this.getEncoding(), - force: options && options.discardChanges, + force, patch: this.loaded } ) + if (this.loaded && !force && !patch) { + // We have attempted to patch the buffer and failed; since we're not + // forcing a reload, this will result in a conflict. Emit the status + // so the user understands why the buffer contents do not match what is + // on disk. + this.emitConflictedStatusChanged(true) + } + // If this is not the most recent load of this file, then we should bow // out and let the newer call to `load` handle the tasks below. if (this.loadCount > loadCount) return @@ -2228,6 +2281,7 @@ class TextBuffer { } this.fileHasChangedSinceLastLoad = false + this.emitConflictedStatusChanged(false) this.digestWhenLastPersisted = this.buffer.baseTextDigest() this.cachedText = null @@ -2341,6 +2395,10 @@ class TextBuffer { // consistent behavior with Mac/Windows. if (!this.file.existsSync()) return if (this.outstandingSaveCount > 0) return + + // This file has changed since we last loaded it from disk, but that + // does not automatically mean there is a conflict. Set the flag, but + // do not emit a `did-conflict` event until we are sure. this.fileHasChangedSinceLastLoad = true if (this.isModified()) { @@ -2348,11 +2406,12 @@ class TextBuffer { 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') + this.emitConflictedStatusChanged(true) } else { // Despite being modified, we're once again in alignment with what // is on disk. This file is not in conflict. this.fileHasChangedSinceLastLoad = false + this.emitConflictedStatusChanged(false) } } else { // This buffer was previously in sync with what was on disk, so we @@ -2360,6 +2419,7 @@ class TextBuffer { // definition, this means there is no conflict, so we'll reset the // appropriate flag. this.fileHasChangedSinceLastLoad = false + this.emitConflictedStatusChanged(false) return this.load({internal: true}) } }, this.fileChangeDelay))) @@ -2375,7 +2435,7 @@ class TextBuffer { // exists on disk. const modified = this.buffer.isModified() this.retainsUnmodifiedTraitAfterDeletion = !modified - this.emitter.emit('did-delete') + this.emitDeletedStatusChanged(true) if (!modified && this.shouldDestroyOnFileDelete()) { return this.destroy() } else { @@ -2514,7 +2574,25 @@ class TextBuffer { emitModifiedStatusChanged (modifiedStatus) { if (modifiedStatus === this.previousModifiedStatus) return this.previousModifiedStatus = modifiedStatus - return this.emitter.emit('did-change-modified', modifiedStatus) + this.emitter.emit('did-change-modified', modifiedStatus) + } + + emitDeletedStatusChanged (deletedStatus) { + if (deletedStatus === this.previousDeletedStatus) return + this.previousDeletedStatus = deletedStatus + this.emitter.emit('did-change-deleted', deletedStatus) + if (deletedStatus) { + this.emitter.emit('did-delete') + } + } + + emitConflictedStatusChanged (conflictedStatus) { + if (conflictedStatus === this.previousConflictedStatus) return + this.previousConflictedStatus = conflictedStatus + this.emitter.emit('did-change-conflicted', conflictedStatus) + if (conflictedStatus) { + this.emitter.emit('did-conflict') + } } logLines (start = 0, end = this.getLastRow()) { From 656db0d6a63c15519a008c47743e8f2387ebb3a1 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Mon, 31 Aug 2026 01:59:05 -0700 Subject: [PATCH 2/2] Fix bugs; add specs --- spec/text-buffer-io-spec.js | 55 ++++++++++++++++++++++++++++++++++- src/text-buffer.js | 57 ++++++++++++++++++++++++++++++------- 2 files changed, 101 insertions(+), 11 deletions(-) diff --git a/spec/text-buffer-io-spec.js b/spec/text-buffer-io-spec.js index 72f77e8b5..ac7b56d9f 100644 --- a/spec/text-buffer-io-spec.js +++ b/spec/text-buffer-io-spec.js @@ -301,6 +301,49 @@ describe('TextBuffer IO', () => { }) }) + describe('.resurrect', () => { + let filePath + + beforeEach(async done => { + filePath = temp.openSync('atom').path + fs.writeFileSync(filePath, 'abcdefg') + buffer = await TextBuffer.load(filePath) + done() + }) + + it('resubscribes to the file', async done => { + fs.unlinkSync(filePath) + await wait(500) + expect(buffer.isDeleted()).toBe(true) + + // A buffer whose file has been deleted no longer has a file watcher, + // because one cannot watch a path that does not exist. Resurrection must + // therefore start watching the file again; detecting the resurrection + // itself is the caller's job, but resuming our own bookkeeping is ours. + spyOn(buffer, 'subscribeToFile').and.callThrough() + + fs.writeFileSync(filePath, 'abcdefg') + await buffer.resurrect() + + expect(buffer.subscribeToFile).toHaveBeenCalled() + expect(buffer.isDeleted()).toBe(false) + done() + }) + + it('does nothing if the file is still missing', async done => { + fs.unlinkSync(filePath) + await wait(500) + expect(buffer.isDeleted()).toBe(true) + + spyOn(buffer, 'subscribeToFile').and.callThrough() + await buffer.resurrect() + + expect(buffer.subscribeToFile).not.toHaveBeenCalled() + expect(buffer.isDeleted()).toBe(true) + done() + }) + }) + describe('.save', () => { let filePath @@ -726,10 +769,12 @@ describe('TextBuffer IO', () => { let reloadedCount = 0 let conflictedCount = 0 + const conflictedStatusChanges = [] // Simulate an external program recreating the file. buffer.onDidReload(() => reloadedCount++) buffer.onDidConflict(() => conflictedCount++) + buffer.onDidChangeConflicted((status) => conflictedStatusChanges.push(status)) fs.writeFileSync(filePath, `lorem ipsum`) // Calling `resurrect` triggers the logic that would take place @@ -737,8 +782,16 @@ describe('TextBuffer IO', () => { // resurrections. await buffer.resurrect() - expect(reloadedCount).toBe(1) + // Nothing was reloaded: the whole point of this scenario is that the + // buffer's uncommitted changes are preserved. + expect(reloadedCount).toBe(0) expect(conflictedCount).toBe(1) + + // The buffer stays conflicted; the status must not flip back on its + // own once the load finishes. + expect(conflictedStatusChanges).toEqual([true]) + expect(buffer.isInConflict()).toBe(true) + expect(buffer.isDeleted()).toBe(false) expect(buffer.isModified()).toBe(true) }) diff --git a/src/text-buffer.js b/src/text-buffer.js index 3dede5aff..72182a433 100644 --- a/src/text-buffer.js +++ b/src/text-buffer.js @@ -106,6 +106,12 @@ class TextBuffer { // now. this.didHaveFileOnDisk = false + // The most recently emitted “deleted” and “conflicted” statuses. They + // start at `false` so that the first status we emit describes an actual + // change rather than the buffer's initial state. + this.previousDeletedStatus = false + this.previousConflictedStatus = 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. @@ -2052,6 +2058,7 @@ class TextBuffer { this.setFile(file) this.fileHasChangedSinceLastLoad = false this.emitConflictedStatusChanged(false) + this.emitDeletedStatusChanged(this.isDeleted()) this.digestWhenLastPersisted = this.buffer.baseTextDigest() this.loaded = true this.emitModifiedStatusChanged(false) @@ -2079,7 +2086,17 @@ class TextBuffer { // signal to `TextBuffer` that this buffer's backing file has been // resurrected, and that certain events ought to be emitted. resurrect () { + // Either the file came back but was gone again by the time we ran… or + // something is calling this method speculatively (perhaps via polling) in + // lieu of a file-watcher. if (this.isDeleted()) return + + // A buffer whose file is deleted loses its file watcher, since one cannot + // watch a path that does not exist. Now that there is a file at this path + // again, we can resume watching it; otherwise this buffer would stay deaf + // to every subsequent change on disk. + this.subscribeToFile() + this.emitDeletedStatusChanged(false) // This is like calling `reload`, but less aggressive; it will preserve // uncommitted buffer contents and emit a `did-conflict` event instead of @@ -2237,11 +2254,14 @@ class TextBuffer { } ) - if (this.loaded && !force && !patch) { - // We have attempted to patch the buffer and failed; since we're not - // forcing a reload, this will result in a conflict. Emit the status - // so the user understands why the buffer contents do not match what is - // on disk. + const bailedOut = Boolean(this.loaded && !force && !patch) + if (bailedOut) { + // A `null` patch means the native layer declined to load. That means + // the buffer has uncommitted changes and we aren't forcing a reload. + // Its contents are now purposefully out of step with what's on disk. + // So instead of a succeeded load, we record it as a file that has + // changed underneath us. + this.fileHasChangedSinceLastLoad = true this.emitConflictedStatusChanged(true) } @@ -2258,7 +2278,7 @@ class TextBuffer { this.emitter.emit('will-reload') } } - this.finishLoading(checkpoint, patch, options) + this.finishLoading(checkpoint, patch, options, bailedOut) } catch (error) { if ((!options || !options.mustExist) && error.code === 'ENOENT') { this.emitter.emit('will-reload') @@ -2272,7 +2292,7 @@ class TextBuffer { return this } - finishLoading (checkpoint, patch, options) { + finishLoading (checkpoint, patch, options, bailedOut = false) { if (this.isDestroyed() || (this.loaded && checkpoint == null && patch != null)) { if (options && options.discardChanges) { this.emitter.emit('did-reload') @@ -2280,8 +2300,13 @@ class TextBuffer { return } - this.fileHasChangedSinceLastLoad = false - this.emitConflictedStatusChanged(false) + if (!bailedOut) this.fileHasChangedSinceLastLoad = false + + // Emit the statuses the buffer actually holds rather than assuming a load + // clears them. Both emitters are no-ops when nothing has changed. + this.emitConflictedStatusChanged(this.isInConflict()) + this.emitDeletedStatusChanged(this.isDeleted()) + this.digestWhenLastPersisted = this.buffer.baseTextDigest() this.cachedText = null @@ -2329,7 +2354,13 @@ class TextBuffer { } this.loaded = true - this.emitter.emit('did-reload') + + // A load that bailed out deliberately left the buffer's contents alone, so + // it did not reload anything; a consumer that hears `did-reload` would + // wrongly assume the buffer now matches what is on disk. Such a load emits + // no `will-reload` either, so staying silent here keeps the pair + // symmetrical. + if (!bailedOut) this.emitter.emit('did-reload') return this } @@ -2396,6 +2427,12 @@ class TextBuffer { if (!this.file.existsSync()) return if (this.outstandingSaveCount > 0) return + // The file exists, so the buffer is no longer deleted, whether or not + // anything ever told it so. A file that is deleted and then recreated + // otherwise leaves the deleted status stuck at `true` forever, which + // would swallow the _next_ deletion. + this.emitDeletedStatusChanged(this.isDeleted()) + // This file has changed since we last loaded it from disk, but that // does not automatically mean there is a conflict. Set the flag, but // do not emit a `did-conflict` event until we are sure.