PaneItem onDidTerminatePendingState is ignored unless onDidDestroy is defined

[smashwilson]

Prerequisites

• Put an X between the brackets on this line if you have done all of the following:
  • Reproduced the problem in Safe Mode: https://flight-manual.atom.io/hacking-atom/sections/debugging/#using-safe-mode
  • Followed all applicable steps in the debugging guide: https://flight-manual.atom.io/hacking-atom/sections/debugging/
  • Checked the FAQs on the message board for common solutions: https://discuss.atom.io/c/faq
  • Checked that your issue isn't already filed: https://github.com/issues?utf8=✓&q=is%3Aissue+user%3Aatom
  • Checked that there is not already an Atom package that provides the described functionality: https://atom.io/packages

Description

When implementing a PaneItem that's intended to be opened as pending, defining onDidTerminatePendingState() is insufficient - you need to implement onDidDestroy() as well.

atom/src/pane.js

Lines 617 to 626 in e2c89b2

	if (typeof item.onDidDestroy === 'function') {
	const itemSubscriptions = new CompositeDisposable()
	itemSubscriptions.add(item.onDidDestroy(() => this.removeItem(item, false)))
	if (typeof item.onDidTerminatePendingState === 'function') {
	itemSubscriptions.add(item.onDidTerminatePendingState(() => {
	if (this.getPendingItem() === item) this.clearPendingItem()
	}))
	}
	this.subscriptionsPerItem.set(item, itemSubscriptions)
	}

Steps to Reproduce

1. Implement a new PaneItem with an opener. Implement onDidTerminatePendingState but not onDidDestroy.
2. Open it as a pending item with a call to workspace.open(..., {pending: true}).
3. Double-click on the tab title.

Expected behavior: The item should terminate its pending state and become a non-pending item.

Actual behavior: The item remains as a pending item.

Reproduces how often: 100%

Versions

$ atom --version
Atom    : 1.30.0-dev-15e769a17
Electron: 2.0.2
Chrome  : 61.0.3163.100
Node    : 8.9.3
$ apm --version
apm  1.19.0
npm  3.10.10
node 6.9.5 x64
atom 1.30.0-dev-15e769a17
python 2.7.15
git 2.17.1

Additional Information

Interestingly, choosing "Keep pending tab" from the context menu on the tab works correctly.

[50Wliu]

Ah! This is probably why I've always been unable to double-click PDF files (pdf-view) to make them permanent.

[jasonrudolph]

@smashwilson: It sounds to me like we want to treat both onDidTerminatePendingState and onDidDestroy as optional functions that a PaneItem can implement:

• If you only implement onDidTerminatePendingState, that's fine: Atom will use it
• If you only implement onDidDestroy, that's fine, too: Atom will use it
• If you implement both, Atom will use both
• If you implement neither, that's fine as well

Does that sound right to you?

[smashwilson]

Yah, that's exactly what I would expect. I didn't see any reason why the two should be bound together as they are. If I missed something, we should at least amend the Workspace docs to note this.

Also: the Workspace "Required Methods" section where these should be documented appears to be broken 😅

[jasonrudolph]

Also: the Workspace "Required Methods" section where these should be documented appears to be broken 😅

@smashwilson: Thanks for pointing this out. I've opened #17746 to track this issue.

[lock]

This issue has been automatically locked since there has not been any recent activity after it was closed. If you can still reproduce this issue in Safe Mode then please open a new issue and fill out the entire issue template to ensure that we have enough information to address your issue. Thanks!