Merge with sublime-tabs

[ddavison]

Hey,

Considering your package's simplicity, we should at least think about merging preview-tabs with sublime-tabs. How that happens, i'm not sure. For "marketing" techniques, i'd advise we leave the name at sublime-tabs, and since I have that package name reserved, i could give you write access, etc.

We could rewrite the source of sublime-tabs to what you have in preview-tabs.

What do you think?

[ddavison]

I also have travis.ci integrated with sublime-tabs running tests against the package, as well as the treeview/tabs packages just to make sure it's not breaking anything.

[ahuth]

I'm down with doing this. My only concern is that since preview-tabs is so new, weird edge cases will turn up for a while.

That shouldn't stop the merger from happening, though. If anything, getting it out to the user base of sublime-tabs will be a quick way to battle-test everything.

One possible way forward is:

1. Resolve issues #4 and #5 of preview-tabs and push a 0.1.4 release.
2. Put in deprecation warnings for preview-tabs, link to sublime-tabs from the readme here.
3. Make a branch/pull-request off of sublime-tabs where the code transfers over.
4. See if anyone wants to add feedback to the PR and make sure there are no changes of functionality (unless we want something to change).
5. Merge and be happy.

[ddavison]

:+1: I like it. I'll keep reading your sauce to make sure i understand whats happening, to think about potential pitfalls.

[ahuth]

Update: This is now out of date. See below.

High-level overview:

• PreviewTabsView
  • Exists for each pane
  • Can have 0 or 1 'previews'
  • Makes a new 'preview' when paneItems are added (link)
  • Tells its 'preview' (if any) to keep (or persist) upon double click of it in the tree view (link)
• PreviewTabsPreview
  • Is the aforementioned 'preview'
  • 'Persists' itself upon save/modify/tab double-click (link)
  • When it 'persists' itself, triggers a callback from its PreviewTabsView (link)
• PreivewTabsEventHandler
  • Wraps jQuery-style events in an api that matches Atom Event-Kit's. Lets us use subscription.dispose() to clean up event subscriptions, no matter what kind of events they are:

[ahuth]

The above is now out of date, as I've re-organized the code. Now:

• PreviewTabsPanesManager - Adds and removes pane controllers as panes are added and removed
• PreviewTabsPaneController - Keeps track of a pane's "preview"
• PreviewTabsPreview - No change. Closes or makes permanent a previewed tab/editor
• PreviewTabsEventHandler - No change. Gives jQuery-style events the same api as Event-Kit events

[ddavison]

@brkattk was nice enough to compile this list of features of Sublime Tabs, and I've revised it. This list contains all of the features that Sublime Tabs has, and if I deprecate Sublime Tabs in favor of Preview Tabs, i'd like to make sure that the users have all of this functionality:

• [x] Opening a new tab makes tab temporary
• [x] Editing a temporary tab will make it permanent
• [x] Opening a new tab when there is an temp tab already will replace an existing temporary tab
• [x] Double clicking any tab makes it permanent
• [ ] Opening views that do not contain an editor creates a permanent tab (Settings view, Images, etc)
• [ ] Opening views that do not contain an editor replaces an existing temp tab
• [ ] Opening images should create a temporary tab
• [x] Saving a file should make a tab permanent
• [x] Double clicking a file in the tree view should open a permanent tab in the editor, unless it's not an editable file type
• [x] Single clicking a file in the tree view should open a temporary tab in the editor

@ahuth , please check all that apply right now, and we can use that as a "to-do" list of what to accomplish to merge. :)

[ahuth]

Hey, thank you for putting together this list.

Unfortunately, I've actually pulled this package off of atom.io. Atom changed a month or so back, and this stopped working correctly. Since then, I haven't had the motivation to go through and figure out how to make it work right, again.

This architecture, of listening and responding to events, is still a great one, and probably ideal for the long run.

If I ever get motivated and jump back into this to get it working, I'll definitely let you know.

Sorry!

[hatzopoulos]

@ahuth ,

Sorry to hear that. That's too bad for me because I was looking forward to this package. And I don't know about you but it has been working great for me. I've installed it manually:

cd ~/.atom/packages
git clone https://github.com/ahuth/preview-tabs
cd preview-tabs/
npm install

Works great! Before this package I was having issues with the sublime-tabs package in combination with some other packages (I believe it was tree-view-open-files but it could have been more).

[oktayacikalin]

@ahuth I just also installed it like @hatzopoulos wrote above. Works fine apart from one small issue where the first generated tab is being created after the current tab and then switching to another file moves it one position to the right. After that it stays put on switching to other files. Please keep pushing this functionality - it's really something useful!

[ahuth]

Thanks for checking this out and installing it.

I agree, this functionality is really nice, and the event-oriented architecture seems like a good way to go. However, I was running into a bunch of issues after Atom reached 1.0 for its api. Maybe you guys haven't ran into some of the issues, yet?

In any case, I think this idea could work, but needs to be re-made for the new (and stable) Atom api. Just not sure I have the motivation to work on it right now. Maybe someday, though!

[oktayacikalin]

Then at least add a label to this issue that you would accept help from others :) For example asking @ddavison would be worth a try..

[wmertens]

A fun edge case is that when you use it together with open-last-project, it will open all the files that were open last time, only it will open them one by one in the same tab :grinning:.

Yet another reason why this should be in core... Hoping it will land there soon!

[ahuth]

Yikes, that is interesting.

Kind of makes sense, too, since that package is probably not opening all those files by double-clicking them.

Unfortunately, I won't be able to look into fixing it. Even though the event-based architecture used here is sound, I don't have any plans to keep maintaining this package right now. Sorry!

[wmertens]

@ahuth I completely understand, however I wonder if you might have quick comments on this. The open-last-project package uses this code to restore tabs:

this.data.files.forEach(function(file) {
    atom.workspace.open(file);
});

Does that mean this is the wrong way to open files, or that preview-tabs should be adapted to differentiate between code opening tabs and users?

[ahuth]

Preview tabs should definitely be able to tell when a user has opened a tab, and react to that.

Not sure of the best way to do that, but it would be great if it would only respond to user events.