[CLOSED] Fix Quick Open async-related bugs & performance problems

[core-ai-bot]

Issue by peterflynn
Tuesday Jan 15, 2013 at 01:36 GMT
Originally opened as adobe/brackets#2548

---

Fixes these issues:

• [CLOSED] Add "Open Recent" functionality. #1855 (Typing quickly in Go to Definition goes to wrong location on Enter) -- moved Enter/Esc handling to keyup and added async delay to parallel how letter keys are handled; register keyup handler earlier so it can block Enter from being processed by Smart Autocomplete (which we did before by killing the UI before the keyup could arrive)
• Remaining bit of [CLOSED] Live File Preview Chrome not found #1627 (Quick Open doesn't scroll based on prepopulated text if opened via menu) -- moved code from keyup to new _handleShowResults() method
• Side issue mentioned in [CLOSED] Add "Open Recent" functionality. #1855 (auto-scroll is always one keystroke behind as you type a query) -- same fix, moving code out of keyup
• Slow performance when typing quickly in big files like codemirror.js -- added staleness check in _filterCallback()

Also, some code cleanup:

• Move code that didn't need to be in keyup into _filterCallback()
• Remove keydown handler now that it's unneeded
• Add more docs on event ordering, etc.

I contemplated ditching Smart Autocomplete altogether, since we have been fighting with it so much. But ultimately, it didn't seem worth it for this fix because it wouldn't avoid most of the thorny issues:

• Processing letter keys async after the key event is a must if we want to avoid performance problems from multiple key events queueing up (the staleness check in _filterCallback() won't work otherwise). So the async complexity there wouldn't be avoided.
• Earlier I griped about Smart Autocomplete not noticing programmatic text field changes, which we'd had to work around in an earlier bug. But it turns out that's basically impossible to fix since programmatic changes don't dispatch any events.

Down the road we still may want to get rid of it: it would still make key handling a tad cleaner (e.g. simplifying the Enter key fix here), let us improve perf slightly more (by skipping list re-render when stale instead of only skipping re-filtering as here), let us fix memory leak #1550, and fix the odd bug where hitting right-arrow is treated as hitting Enter. But it didn't feel worth it to make that big migration yet.

---

peterflynn included the following code: https://github.com/adobe/brackets/pull/2548/commits

[core-ai-bot]

Comment by peterflynn
Tuesday Jan 15, 2013 at 01:37 GMT

---

Also, miraculously I think this and #2462 manage not to collide -- the merge within QuickOpen.js should be pretty trivial.

[core-ai-bot]

Comment by peterflynn
Tuesday Jan 15, 2013 at 01:50 GMT

---

Note: for more context on the various async issues with Quick Open, see my comment thread in #1855. Additional background is in #1627 and #1384.

[core-ai-bot]

Comment by dangoor
Tuesday Jan 15, 2013 at 15:48 GMT

---

The fix looks great (all of the comments are super helpful) and works well in my testing. I didn't spot anything I would change.