Skip to content
This repository was archived by the owner on Dec 15, 2022. It is now read-only.

Commit 7d44d99

Browse files
smashwilsonsmashwilson
committed
Merge pull request #2105 from atom/aw/destroyed-marker-layers
Don't pass destroyed TextEditors to find-and-replace
1 parent 4743c82 commit 7d44d99

File tree

8 files changed

+159
-55
lines changed

8 files changed

+159
-55
lines changed

‎lib/items/changed-file-item.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,9 @@ export default class ChangedFileItem extends React.Component {
4545

4646
this.refEditor = new RefHolder();
4747
this.refEditor.observe(editor => {
48-
this.emitter.emit('did-change-embedded-text-editor', editor);
48+
if (editor.isAlive()) {
49+
this.emitter.emit('did-change-embedded-text-editor', editor);
50+
}
4951
});
5052
}
5153

@@ -94,7 +96,7 @@ export default class ChangedFileItem extends React.Component {
9496
}
9597

9698
observeEmbeddedTextEditor(cb) {
97-
this.refEditor.map(editor => cb(editor));
99+
this.refEditor.map(editor => editor.isAlive() && cb(editor));
98100
return this.emitter.on('did-change-embedded-text-editor', cb);
99101
}
100102

‎lib/items/commit-detail-item.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ export default class CommitDetailItem extends React.Component {
3030

3131
this.refEditor = new RefHolder();
3232
this.refEditor.observe(editor => {
33-
this.emitter.emit('did-change-embedded-text-editor', editor);
33+
if (editor.isAlive()) {
34+
this.emitter.emit('did-change-embedded-text-editor', editor);
35+
}
3436
});
3537
}
3638

@@ -81,7 +83,7 @@ export default class CommitDetailItem extends React.Component {
8183
}
8284

8385
observeEmbeddedTextEditor(cb) {
84-
this.refEditor.map(editor => cb(editor));
86+
this.refEditor.map(editor => editor.isAlive() && cb(editor));
8587
return this.emitter.on('did-change-embedded-text-editor', cb);
8688
}
8789

‎lib/items/commit-preview-item.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ export default class CommitPreviewItem extends React.Component {
3232

3333
this.refEditor = new RefHolder();
3434
this.refEditor.observe(editor => {
35-
this.emitter.emit('did-change-embedded-text-editor', editor);
35+
if (editor.isAlive()) {
36+
this.emitter.emit('did-change-embedded-text-editor', editor);
37+
}
3638
});
3739
}
3840

@@ -83,7 +85,7 @@ export default class CommitPreviewItem extends React.Component {
8385
}
8486

8587
observeEmbeddedTextEditor(cb) {
86-
this.refEditor.map(editor => cb(editor));
88+
this.refEditor.map(editor => editor.isAlive() && cb(editor));
8789
return this.emitter.on('did-change-embedded-text-editor', cb);
8890
}
8991

‎lib/items/issueish-detail-item.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,9 @@ export default class IssueishDetailItem extends Component {
9191

9292
this.refEditor = new RefHolder();
9393
this.refEditor.observe(editor => {
94-
this.emitter.emit('did-change-embedded-text-editor', editor);
94+
if (editor.isAlive()) {
95+
this.emitter.emit('did-change-embedded-text-editor', editor);
96+
}
9597
});
9698
}
9799

@@ -214,7 +216,7 @@ export default class IssueishDetailItem extends Component {
214216
}
215217

216218
observeEmbeddedTextEditor(cb) {
217-
this.refEditor.map(editor => cb(editor));
219+
this.refEditor.map(editor => editor.isAlive() && cb(editor));
218220
return this.emitter.on('did-change-embedded-text-editor', cb);
219221
}
220222

‎test/items/changed-file-item.test.js‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ describe('ChangedFileItem', function() {
5757
);
5858
}
5959

60-
function open(wrapper, options = {}) {
60+
function open(options = {}) {
6161
const opts = {
6262
relPath: 'a.txt',
6363
workingDirectory: repository.getWorkingDirectoryPath(),
@@ -70,37 +70,37 @@ describe('ChangedFileItem', function() {
7070

7171
it('locates the repository from the context pool', async function() {
7272
const wrapper = mount(buildPaneApp());
73-
await open(wrapper);
73+
await open();
7474

7575
assert.strictEqual(wrapper.update().find('ChangedFileContainer').prop('repository'), repository);
7676
});
7777

7878
it('passes an absent repository if the working directory is unrecognized', async function() {
7979
const wrapper = mount(buildPaneApp());
80-
await open(wrapper, {workingDirectory: '/nope'});
80+
await open({workingDirectory: '/nope'});
8181

8282
assert.isTrue(wrapper.update().find('ChangedFileContainer').prop('repository').isAbsent());
8383
});
8484

8585
it('passes other props to the container', async function() {
8686
const other = Symbol('other');
8787
const wrapper = mount(buildPaneApp({other}));
88-
await open(wrapper);
88+
await open();
8989

9090
assert.strictEqual(wrapper.update().find('ChangedFileContainer').prop('other'), other);
9191
});
9292

9393
describe('getTitle()', function() {
9494
it('renders an unstaged title', async function() {
95-
const wrapper = mount(buildPaneApp());
96-
const item = await open(wrapper, {stagingStatus: 'unstaged'});
95+
mount(buildPaneApp());
96+
const item = await open({stagingStatus: 'unstaged'});
9797

9898
assert.strictEqual(item.getTitle(), 'Unstaged Changes: a.txt');
9999
});
100100

101101
it('renders a staged title', async function() {
102-
const wrapper = mount(buildPaneApp());
103-
const item = await open(wrapper, {stagingStatus: 'staged'});
102+
mount(buildPaneApp());
103+
const item = await open({stagingStatus: 'staged'});
104104

105105
assert.strictEqual(item.getTitle(), 'Staged Changes: a.txt');
106106
});
@@ -150,23 +150,23 @@ describe('ChangedFileItem', function() {
150150
});
151151

152152
it('serializes itself as a FilePatchControllerStub', async function() {
153-
const wrapper = mount(buildPaneApp());
154-
const item0 = await open(wrapper, {relPath: 'a.txt', workingDirectory: '/dir0', stagingStatus: 'unstaged'});
153+
mount(buildPaneApp());
154+
const item0 = await open({relPath: 'a.txt', workingDirectory: '/dir0', stagingStatus: 'unstaged'});
155155
assert.deepEqual(item0.serialize(), {
156156
deserializer: 'FilePatchControllerStub',
157157
uri: 'atom-github://file-patch/a.txt?workdir=%2Fdir0&stagingStatus=unstaged',
158158
});
159159

160-
const item1 = await open(wrapper, {relPath: 'b.txt', workingDirectory: '/dir1', stagingStatus: 'staged'});
160+
const item1 = await open({relPath: 'b.txt', workingDirectory: '/dir1', stagingStatus: 'staged'});
161161
assert.deepEqual(item1.serialize(), {
162162
deserializer: 'FilePatchControllerStub',
163163
uri: 'atom-github://file-patch/b.txt?workdir=%2Fdir1&stagingStatus=staged',
164164
});
165165
});
166166

167167
it('has some item-level accessors', async function() {
168-
const wrapper = mount(buildPaneApp());
169-
const item = await open(wrapper, {relPath: 'a.txt', workingDirectory: '/dir', stagingStatus: 'unstaged'});
168+
mount(buildPaneApp());
169+
const item = await open({relPath: 'a.txt', workingDirectory: '/dir', stagingStatus: 'unstaged'});
170170

171171
assert.strictEqual(item.getStagingStatus(), 'unstaged');
172172
assert.strictEqual(item.getFilePath(), 'a.txt');
@@ -178,16 +178,18 @@ describe('ChangedFileItem', function() {
178178
let sub, editor;
179179

180180
beforeEach(function() {
181-
editor = Symbol('editor');
181+
editor = {
182+
isAlive() { return true; },
183+
};
182184
});
183185

184186
afterEach(function() {
185187
sub && sub.dispose();
186188
});
187189

188-
it('calls its callback immediately if an editor is present', async function() {
190+
it('calls its callback immediately if an editor is present and alive', async function() {
189191
const wrapper = mount(buildPaneApp());
190-
const item = await open(wrapper);
192+
const item = await open();
191193

192194
wrapper.update().find('ChangedFileContainer').prop('refEditor').setter(editor);
193195

@@ -196,15 +198,37 @@ describe('ChangedFileItem', function() {
196198
assert.isTrue(cb.calledWith(editor));
197199
});
198200

201+
it('does not call its callback if an editor is present but destroyed', async function() {
202+
const wrapper = mount(buildPaneApp());
203+
const item = await open();
204+
205+
wrapper.update().find('ChangedFileContainer').prop('refEditor').setter({isAlive() { return false; }});
206+
207+
const cb = sinon.spy();
208+
sub = item.observeEmbeddedTextEditor(cb);
209+
assert.isFalse(cb.called);
210+
});
211+
199212
it('calls its callback later if the editor changes', async function() {
200213
const wrapper = mount(buildPaneApp());
201-
const item = await open(wrapper);
214+
const item = await open();
202215

203216
const cb = sinon.spy();
204217
sub = item.observeEmbeddedTextEditor(cb);
205218

206219
wrapper.update().find('ChangedFileContainer').prop('refEditor').setter(editor);
207220
assert.isTrue(cb.calledWith(editor));
208221
});
222+
223+
it('does not call its callback after its editor is destroyed', async function() {
224+
const wrapper = mount(buildPaneApp());
225+
const item = await open();
226+
227+
const cb = sinon.spy();
228+
sub = item.observeEmbeddedTextEditor(cb);
229+
230+
wrapper.update().find('ChangedFileContainer').prop('refEditor').setter({isAlive() { return false; }});
231+
assert.isFalse(cb.called);
232+
});
209233
});
210234
});

‎test/items/commit-detail-item.test.js‎

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -192,16 +192,18 @@ describe('CommitDetailItem', function() {
192192
let sub, editor;
193193

194194
beforeEach(function() {
195-
editor = Symbol('editor');
195+
editor = {
196+
isAlive() { return true; },
197+
};
196198
});
197199

198200
afterEach(function() {
199201
sub && sub.dispose();
200202
});
201203

202-
it('calls its callback immediately if an editor is present', async function() {
204+
it('calls its callback immediately if an editor is present and alive', async function() {
203205
const wrapper = mount(buildPaneApp());
204-
const item = await open(wrapper);
206+
const item = await open();
205207

206208
wrapper.update().find('CommitDetailContainer').prop('refEditor').setter(editor);
207209

@@ -210,15 +212,37 @@ describe('CommitDetailItem', function() {
210212
assert.isTrue(cb.calledWith(editor));
211213
});
212214

215+
it('does not call its callback if an editor is present but destroyed', async function() {
216+
const wrapper = mount(buildPaneApp());
217+
const item = await open();
218+
219+
wrapper.update().find('CommitDetailContainer').prop('refEditor').setter({isAlive() { return false; }});
220+
221+
const cb = sinon.spy();
222+
sub = item.observeEmbeddedTextEditor(cb);
223+
assert.isFalse(cb.called);
224+
});
225+
213226
it('calls its callback later if the editor changes', async function() {
214227
const wrapper = mount(buildPaneApp());
215-
const item = await open(wrapper);
228+
const item = await open();
216229

217230
const cb = sinon.spy();
218231
sub = item.observeEmbeddedTextEditor(cb);
219232

220233
wrapper.update().find('CommitDetailContainer').prop('refEditor').setter(editor);
221234
assert.isTrue(cb.calledWith(editor));
222235
});
236+
237+
it('does not call its callback after its editor is destroyed', async function() {
238+
const wrapper = mount(buildPaneApp());
239+
const item = await open();
240+
241+
const cb = sinon.spy();
242+
sub = item.observeEmbeddedTextEditor(cb);
243+
244+
wrapper.update().find('CommitDetailContainer').prop('refEditor').setter({isAlive() { return false; }});
245+
assert.isFalse(cb.called);
246+
});
223247
});
224248
});

0 commit comments

Comments
 (0)