Rewrite the whole damn thing. In ES2015!

[Osmose]

I started rewriting chunks of the code that I wasn't happy with, and after a while I realized it was starting to look like MVC, so I split things up that way and ended up mostly happy with the result. There's a few things I'm still not entirely happy with (like certain parts of the Path class) and there's some improvements that will come later (using async file reading methods instead of synchronous ones) but I think this is ready for testing and releasing.

I tried to stick with CoffeeScript but ended up switching to ES2015 via Babel after getting frustrated too many times with the syntax.

I'm going to start working on specs for this next. I also need to go through and document things better. In the meantime, feedback from anyone who is willing to download this branch and test it out would be greatly appreciated.

CC @artagnon @Traverse @pimentel for feedback if you have time. :D

@mythmon I'll probably be asking you for a review once the specs are done, but any early feedback is appreciated as well.

[artagnon]

Not sure about the pros and cons of moving away from coffeescript -- the API documentation is in coffeescript, for instance.

Just started using the branch and immediately noticed a bug: A scrollToCursor() invocation immediately on launch is missing. I can't see the full path on launch, if it's too big. On the upside, the program seems to be noticeably faster (or is it my imagination?). Good effort -- keep it going :+1:

[Osmose]

Not sure about the pros and cons of moving away from coffeescript -- the API documentation is in coffeescript, for instance.

The docs for Atom, or our docs? I'm actually unsure if the README should show a code sample in CoffeeScript or not, but I'm leaning towards not.

Basically, in order to write effective CoffeeScript, you have to understand JavaScript anyway. Otherwise, you get caught up on error-prone things like passing parameters to a function call without ()s and so on. So I don't see a huge issue in assuming everyone reading Atom's docs or our own will understand basic JavaScript.

I'm sure they exist, but I imagine the group of people that know CoffeeScript and not JavaScript is pretty small. And I'm fine with helping out via code review the people who don't know ES2015 that well enough yet.

Just started using the branch and immediately noticed a bug: A scrollToCursor() invocation immediately on launch is missing. I can't see the full path on launch, if it's too big. On the upside, the program seems to be noticeably faster (or is it my imagination?). Good effort -- keep it going :+1:

Added and pushed with a call to scrollToCursorPosition. Should work better now!

As for speed, I suspected but didn't expect an improvement. I'm glad you noticed one! We're no longer going through space-pen to generate our DOM stuff (just strings passed to jQuery now) and we touch the DOM a lot less in general now that we store and cache data about the current path in a JS object instead of reading it from the DOM.

[Osmose]

Currently about halfway through writing specs, I think. They're tricky business but in the end they'll be huge for ensuring the continued reliability of the package.

[Osmose]

Specs are done, and I have enabled Travis! I wanted to use https://github.com/atom/ci but felt a little weird doing curl | sh in my tests, so I embedded the code since it uses the same license as we do. Then again, the script itself does just download Atom and run it without checking, so maybe it's not worth it. Oh well.

@mythmon I think this is ready for a real review now, if you're willing to give it one. :D

[artagnon]

New bug introduced: when (N > 1) items come up after filtering, C-n is unable to select the last item. When one item comes up, I'm unable to select that item without using the confirm-selected-or-only thing. I suspect an off-by-one error somewhere.

[artagnon]

Also, the plugin seems to have gotten really slow after prolonged usage. Not sure what state it saves to disk, but it seems to recover somewhat on a full Atom restart. Any clues?

I don't think this has anything to do with the rewrite.

[Osmose]

New bug introduced: when (N > 1) items come up after filtering, C-n is unable to select the last item. When one item comes up, I'm unable to select that item without using the confirm--selected-or-only thing. I suspect an off-by-one error somewhere.

I'm unable to replicate this. To be sure I'm doing it right:

1. Open the dialog.
2. Use the C-n shortcut to select the item at the bottom of the list.
3. Hit Enter to open the item at the bottom of the list.

For me, that opens the selected file without fail on the latest code in this PR. For you, it's not opening the file at all? What does it do instead?

Also, the plugin seems to have gotten really slow after prolonged usage. Not sure what state it saves to disk, but it seems to recover somewhat on a full Atom restart. Any clues?

I don't think this has anything to do with the rewrite.

I think I'm getting this as well; a long delay when opening the dialog. The devtools timeline shows two very long style recalculations triggered by:

module.exports.ScrollbarComponent.updateHorizontal      @ /Applications/Atom.app/Contents/Resources/app.asar/src/scrollbar-component.js:86
module.exports.ScrollbarComponent.updateSync            @ /Applications/Atom.app/Contents/Resources/app.asar/src/scrollbar-component.js:41
module.exports.TextEditorComponent.updateSync           @ /Applications/Atom.app/Contents/Resources/app.asar/src/text-editor-component.js:232
TextEditorComponent                                     @ /Applications/Atom.app/Contents/Resources/app.asar/src/text-editor-component.js:167
TextEditorElement.mountComponent                        @ /Applications/Atom.app/Contents/Resources/app.asar/src/text-editor-element.js:181
TextEditorElement.attachedCallback                      @ /Applications/Atom.app/Contents/Resources/app.asar/src/text-editor-element.js:84
PanelContainerElement.panelAdded                        @ /Applications/Atom.app/Contents/Resources/app.asar/src/panel-container-element.js:43
module.exports.Emitter.emit                             @ /Applications/Atom.app/Contents/Resources/app.asar/node_modules/event-kit/lib/emitter.js:86
module.exports.PanelContainer.addPanel                  @ /Applications/Atom.app/Contents/Resources/app.asar/src/panel-container.js:69
module.exports.Workspace.addPanel                       @ /Applications/Atom.app/Contents/Resources/app.asar/src/workspace.js:751
module.exports.Workspace.addModalPanel                  @ /Applications/Atom.app/Contents/Resources/app.asar/src/workspace.js:727
createModalPanel                                        @ view.js:82
attach                                                  @ controller.js:243
toggle                                                  @ controller.js:164
module.exports.CommandRegistry.handleCommandEvent       @ /Applications/Atom.app/Contents/Resources/app.asar/src/command-registry.js:243
(anonymous function)                                    @ /Applications/Atom.app/Contents/Resources/app.asar/src/command-registry.js:3
module.exports.KeymapManager.dispatchCommandEvent       @ /Applications/Atom.app/Contents/Resources/app.asar/node_modules/atom-keymap/lib/keymap-manager.js:528
module.exports.KeymapManager.handleKeyboardEvent        @ /Applications/Atom.app/Contents/Resources/app.asar/node_modules/atom-keymap/lib/keymap-manager.js:351
module.exports.WindowEventHandler.handleDocumentKeydown @ /Applications/Atom.app/Contents/Resources/app.asar/src/window-event-handler.js:97

I suspect something can be done about avoiding this expensive recalculation. Or maybe we just need to re-create the DOM for each modal panel instead of preserving it? I'll look into this more.

[artagnon]

I'm hitting C-n repeatedly there, but I can only select 3p-tmw-osx by typing out atleast 3p-tmw-. A filter + select is needed. Maybe you're missing the filter part?

Also, the prolonged usage effects aren't just limited to startup time -- even switching directories takes forever.

[Osmose]

I'm hitting C-n repeatedly there, but I can only select 3p-tmw-osx by typing out atleast 3p-tmw-. A filter + select is needed. Maybe you're missing the filter part?

Found it! You're right, I was testing without filtering first. Turns out I was determining the index of the item to highlight by filtering the list of paths to only those that were visible, but then actually applying the highlight using the list of paths including ones that aren't visible. I've pushed a fix, nice catch!

[Osmose]

Also, the prolonged usage effects aren't just limited to startup time -- even switching directories takes forever.

I was eventually able to replicate this as well. Instead of bothering tracking down the real issue, I decided to just see what would happen if I removed jQuery completely and used vanilla JS DOM functions and, what do you know, it appears to have solved the issue! I suspect that there might have been some issues around jQuery storing the paths in it's jQuery specific $.data storage vs the DOM data attributes, but I have nothing solid to back up that suspicion.

In any case, the plugin seems snappier than before and I can't replicate the slowness the way I was able to before. jQuery is now just a dev dependency since it's used in the tests, but I imagine we can remove that pretty easily someday.

Does the latest version still have performance issues for you, @artagnon?

[artagnon]

Thanks for fixing the selection issue. I'll let you know about perf after a few days of usage.

In the meantime, you should consider merging the thing and developing incremental bugfixes (I can't make out what your change is without going through some trouble).

[mythmon]

I'm surprised how well it works to just return strings full of HTML like that. It feels weird, but it seems to work really well, so stick with it I guess. If you care about malicious content, it might be worth making dom a quasiliteral parser that escapes it's output (and then use it like dom

${foo}

``.

I think that's all I have do say for now.

[artagnon]

A couple of days of heavy usage, and I must report that I'm extremely happy with it. It's a big improvement over the previous version.

Thanks again, for your hard work.