Quick Open enhancements & API cleanup #1565
Conversation
peterflynn
commented
Sep 5, 2012
peterflynn
commented
- Allow search() to return a result object instead of a string; this value is now passed to itemFocus/Select() instead of a raw DOM node. Upshot: no longer need to stash metadata in the list item HTML and parse it back out later.
- Better ranking of search results: prioritize into tiers (favoring exact matches, then matches starting at start of the string); sort identical filenames by path instead of arbitrary order
- Expose a public API to open a particular search mode (making it cleaner to add entirely new search modes with a new prefix character)
- Fix a few minor bugs:
- In HTML id search, attrs like "data-button-id" created many false positives (due to "id" suffix)
- Filenames with no extension were truncated
- JS and CSS search were showing results in reverse-alphabetical order
- Clarify / fix inaccuracies in documentation
- Allow search()/filterFileList() to return a SearchResult object instead of a string, which lets you hang additional metadata off each item - Change API so itemFocus() and itemSelect() receive the object returned by search()/filterFileList() instead of a raw DOM node. Removes need to stash data in HTML text returned by formatter - Better ranking for filename search: ensure that exact matches are at the very top, and matches that start at offset 0 in the string are ranked highly; sort identical filenames by path instead of arbitrary order - Fix bug in HTML Quick Open id search where attrs like "data-button-id" created many false positives - Fix bug in filename extension-stripping - Fix a bunch of docs typos
* origin/master: (670 commits) ... Conflicts: src/search/QuickOpen.js
- Simplify the new match/sort logic and share it between all the built-in Quick Open search modes (file name, JS function, HTML id, CSS selector) - Fix bug where JS and CSS modes sorted results in reverse-alphabetical order - Clarify docs around itemFocus() & itemSelect() - Expose doSearch() as a new public API, making is cleaner to add entirely new search modes (with a new prefix character)
ghost
assigned 
| } else { | ||
| return 0; | ||
| } | ||
| return QuickOpen.stringMatch(itemInfo.selector, query); |
There was a problem hiding this comment.
Not sure if it's worth handling, but CSS Selectors have a mix of case sensitivity (id's and classes) and insensitivity (everything else), and this method ignores case.
|
Done with initial code review. |
| } | ||
|
|
||
| return "<li>" + displayName + "</li>"; | ||
| } | ||
|
|
||
| function _filenameResultsFormatter(item, query) { |
There was a problem hiding this comment.
When displaying URLs, the default way that they wrap is terrible. Basically, lines are only broken on hyphens and spaces. Take a look at the technique in this article to see if this can be improved: http://perishablepress.com/wrapping-content/
Note that we also need to do this in other places in UI (e.g. error dialogs, Find in Files results), so some reusable classes would be nice.
There was a problem hiding this comment.
Hmm. It seems like the ideal thing to do would be either (a) make it break on path separators (but still not break on any/all chars), or (b) some form of smart truncation (some native apps preserve the start & end of the path, but replace the middle with ellipses). I think (a) might be relatively easy if we can just inject "zero-width space" chars or some such in the display string... e.g. http://stackoverflow.com/questions/2046530/whats-the-opposite-of-a-nbsp
In the interest of getting this landed soon though, I'd rather tackle that separately (this patch isn't changing the existing way paths/URLs are displayed... it's just moving the old code up a few lines). Would you mind spinning this off as a bug & assigning to me?
There was a problem hiding this comment.
I split this into #1604. I did not assign it to anyone and wasn't sure of the priority. This may be a good starter bug.
|
@redmunds: Code changes pushed. I think I've fixed or responded to all of the comments above -- please let me know if you'd like any other changes. |
|
Looks good. Merging. |
Quick Open enhancements & API cleanup
