Allow workspace item objects to be passed to Workspace.open

[maxbrunsfeld]

Currently, in order to add a given item to the workspace, there is some indirection involved. You must add an opener function that returns that item based on some URI, and then to call Workspace.open with that URI:

const MY_URI = 'my-package://something'

atom.workspace.addOpener((uri) => {
  if (uri === MY_URI) return new MyItem()
})

atom.workspace.open(MY_URI)

The URIs are involved because the workspace uses them as a serializable identifier for the items. But often the indirection of the opener concept is unnecessary. This PR adds the ability to add a given item directly to the workspace:

atom.workspace.open(myItem)

The item can still have a getURI method, so the workspace will still be able to identifiy it for purposes like remembering which Dock you last placed it in.

/cc @nathansobo @matthewwithanm

[matthewwithanm]

I really like getting rid of the indirection requirement! My only question is howtoggle() would work. URIs serve as an (indirect) lazy factory, and a matcher. Constructors fulfill a similar role in JS and while they're not serializable, maybe that doesn't matter in this case (since the deserialized object would use this session's constructor). So maybe toggle(MyConstructor).

[nathansobo]

What am I missing about toggle(myItem). If your item is not yet in the workspace, we add it. If it is in the workspace and hidden, we show it. If it's in the workspace and showing, we hide it.

[matthewwithanm]

Sorry you're right. Not sure what I was thinking. 🤔

[nathansobo]

Adjusted this a bit... Passing in the item directly was making the later part of the method think the item already existed in the workspace, which wasn't necessarily true. I extracted an explicit boolean variable, itemExistsInWorkspace, to represent that.

I also added support for passing items to toggle and hide.

[matthewwithanm]

Oh now I know what I was thinking! toggle() actually hides all matches. So passing the item would have to have different semantics (or change the existing behavior).

[nathansobo]

@matthewwithanm Ah, because there could be multiple items with the same URI. I feel like it could be okay to change the definition of "match" when passing a single item to just be instance equality, as I did in my latest commit. Whatayathink?

[matthewwithanm]

it is a bit of a bummer that you won't be able to get the existing behavior without adding an opener.

also, i feel like this might introduce some weirdness. like, package authors will forget that the item can be cloned (via splitting). the user will split it and there'll be two in the workspace. if they're just storing the item instance, and calling toggle(item), it'll only toggle one of them. and then if that one's destroyed…? when you're not storing the items, it actually ends up being a lot cleaner i think. but this encourages everybody to store the items because adding an opener is a hassle.