Search completion; even more minor tweaks.

3 files changed, 89 insertions, 139 deletions

diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee
index 7d39ccae..4663c091 100644
--- a/background_scripts/completion.coffee
+++ b/background_scripts/completion.coffee
@@ -361,7 +361,7 @@ class SearchEngineCompleter
     CompletionSearch.cancel()
 
   refresh: (port) ->
-    # Load and parse the search-engine configuration.
+    # Parse the search-engine configuration.
     @searchEngines = new AsyncDataFetcher (callback) ->
       engines = {}
       for line in Settings.get("searchEngines").split "\n"
@@ -370,16 +370,17 @@ class SearchEngineCompleter
         tokens = line.split /\s+/
         continue unless 2 <= tokens.length
         keyword = tokens[0].split(":")[0]
+        url = tokens[1]
         description = tokens[2..].join(" ") || "search (#{keyword})"
+        continue unless Utils.hasFullUrlPrefix url
         engines[keyword] =
           keyword: keyword
-          searchUrl: tokens[1]
+          searchUrl: url
           description: description
 
-      # Deliver the resulting engines AsyncDataFetcher table/data.
       callback engines
 
-      # Let the vomnibar in the front end know the custom search engine keywords.
+      # Let the front-end vomnibar know the search-engine keywords.
       port.postMessage
         handler: "keywords"
         keywords: key for own key of engines
@@ -388,6 +389,7 @@ class SearchEngineCompleter
     return onComplete [] if queryTerms.length == 0
 
     @searchEngines.use (engines) =>
+      suggestions = []
       keyword = queryTerms[0]
 
       { custom, searchUrl, description, queryTerms } =
@@ -408,30 +410,27 @@ class SearchEngineCompleter
 
       # Relevancy:
       #   - Relevancy does not depend upon the actual suggestion (so, it does not depend upon word
-      #     relevancy).  We assume that the completion engine has already factored that in.  Also, completion
-      #     engines often handle spelling mistakes, in which case we wouldn't find the query terms in the
-      #     suggestion anyway.
+      #     relevancy, say).  We assume that the completion engine has already factored that in.  Also,
+      #     completion engines often handle spelling mistakes, in which case we wouldn't find the query terms
+      #     in the suggestion anyway.
+      #   - Scores are weighted such that they retain the order provided by the completion engine.
       #   - The relavancy is higher if the query term is longer.  The idea is that search suggestions are more
       #     likely to be relevant if, after typing some number of characters, the user hasn't yet found
       #     a useful suggestion from another completer.
-      #   - Scores are weighted such that they retain the order provided by the completion engine.
+      #
       characterCount = query.length - queryTerms.length + 1
       relavancy = 0.6 * (Math.min(characterCount, 10.0)/10.0)
 
       # This distinguishes two very different kinds of vomnibar baviours, the newer bahviour (true) and the
       # legacy behavior (false).  We retain the latter for the default search engine, and for custom search
-      # engines for which we do not have a completion engine.
+      # engines for which we do not have a completion engine.  By "exclusive vomnibar", we mean suggestions
+      # from other completers are suppressed (so the vomnibar "exclusively" uses suggestions from this search
+      # engine).
       useExclusiveVomnibar = custom and haveCompletionEngine
-
-      # If this is a custom search engine and we have a completer, then we exclude results from other
-      # completers.
       filter = if useExclusiveVomnibar then (suggestion) -> suggestion.type == description else null
 
-      suggestions = []
-
       # For custom search engines, we add a single, top-ranked entry for the unmodified query.  This
-      # suggestion always appears at the top of the suggestion list.  Its setting serve to define various
-      # vomnibar behaviors.
+      # suggestion always appears at the top of the list.
       if custom
         suggestions.push new Suggestion
           queryTerms: queryTerms
@@ -442,15 +441,11 @@ class SearchEngineCompleter
           insertText: if useExclusiveVomnibar then query else null
           # We suppress the leading keyword, for example "w something" becomes "something" in the vomnibar.
           suppressLeadingKeyword: true
-          # Should we highlight (via the selection) the longest continuation of the current query which is
-          # contained in all completions?
-          completeSuggestions: useExclusiveVomnibar
+          selectCommonMatches: false
           # Toggles for the legacy behaviour.
           autoSelect: not useExclusiveVomnibar
           forceAutoSelect: not useExclusiveVomnibar
           highlightTerms: not useExclusiveVomnibar
-          # Do not use this entry for vomnibar completion (highlighting the common text of the suggestions).
-          highlightCommonMatches: false
 
       mkSuggestion = do ->
         (suggestion) ->
@@ -460,14 +455,12 @@ class SearchEngineCompleter
             url: Utils.createSearchUrl suggestion, searchUrl
             title: suggestion
             relevancy: relavancy *= 0.9
-            highlightTerms: false
             insertText: suggestion
-            # Do use this entry for vomnibar completion.
-            highlightCommonMatches: true
+            highlightTerms: false
+            selectCommonMatches: true
 
-      # If we have cached suggestions, then we can bundle them immediately (otherwise we'll have to do an HTTP
-      # request, which we do asynchronously).  This is a synchronous call (for cached suggestions only)
-      # because no callback is provided.
+      # If we have cached suggestions, then we can bundle them immediately (otherwise we'll have to fetch them
+      # asynchronously).
       cachedSuggestions = CompletionSearch.complete searchUrl, queryTerms
 
       # Post suggestions and bail if we already have all of the suggestions, or if there is no prospect of
@@ -476,42 +469,15 @@ class SearchEngineCompleter
         if cachedSuggestions?
           console.log "using cached suggestions:", query
           suggestions.push cachedSuggestions.map(mkSuggestion)...
-        return onComplete suggestions, { filter }
+        return onComplete suggestions, { filter, continuation: null }
 
-      # Post any initial suggestion, and then deliver suggestions from completion engines as a continuation
-      # (so, asynchronously).
+      # Post any initial suggestion, and then deliver the rest of the suggestions as a continuation (so,
+      # asynchronously).
       onComplete suggestions,
         filter: filter
-        continuation: (existingSuggestions, onComplete) =>
-          suggestions = []
-
-          if 0 < existingSuggestions.length
-            existingSuggestionsMinScore = existingSuggestions[existingSuggestions.length-1].relevancy
-            if relavancy < existingSuggestionsMinScore and maxResults <= existingSuggestions.length
-              # No suggestion we propose will have a high enough relavancy to beat the existing suggestions, so bail
-              # immediately.
-              console.log "skip: cannot add completions" if @debug
-              return onComplete []
-
-          CompletionSearch.complete searchUrl, queryTerms, (completionSuggestions = []) =>
-            for suggestion in completionSuggestions
-              suggestions.push new Suggestion
-                queryTerms: queryTerms
-                type: description
-                url: Utils.createSearchUrl suggestion, searchUrl
-                title: suggestion
-                relevancy: relavancy *= 0.9
-                highlightTerms: false
-                insertText: suggestion
-                # Do use this entry for vomnibar completion.
-                highlightCommonMatches: true
-
-            # We keep at least three suggestions (if possible) and at most six.  We keep more than three only if
-            # there are enough slots.  The idea is that these suggestions shouldn't wholly displace suggestions
-            # from other completers.  That would potentially be a problem because there is no relationship
-            # between the relevancy scores produced here and those produced by other completers.
-            count = Math.min 6, Math.max 3, maxResults - existingSuggestions.length
-            onComplete suggestions[...count]
+        continuation: (onComplete) =>
+          CompletionSearch.complete searchUrl, queryTerms, (suggestions = []) =>
+            onComplete suggestions.map mkSuggestion
 
 # A completer which calls filter() on many completers, aggregates the results, ranks them, and returns the top
 # 10. Queries from the vomnibar frontend script come through a multi completer.
@@ -520,11 +486,8 @@ class MultiCompleter
 
   constructor: (@completers) ->
 
-  refresh: (port) ->
-    completer.refresh? port for completer in @completers
-
-  cancel: (port) ->
-    completer.cancel? port for completer in @completers
+  refresh: (port) -> completer.refresh? port for completer in @completers
+  cancel: (port) -> completer.cancel? port for completer in @completers
 
   filter: (request, onComplete) ->
     @debug = true
@@ -533,14 +496,9 @@ class MultiCompleter
 
     RegexpCache.clear()
     { queryTerms } = request
-    request.maxResults = @maxResults
-
-    @mostRecentQuery = null
-    @filterInProgress = true
 
-    suggestions = []
-    continuations = []
-    filters = []
+    [ @mostRecentQuery, @filterInProgress ] = [ null, true ]
+    [ suggestions, continuations, filters ] = [ [], [], [] ]
 
     # Run each of the completers (asynchronously).
     jobs = new JobRunner @completers.map (completer) ->
@@ -554,38 +512,37 @@ class MultiCompleter
     # Once all completers have finished, process and post the results, and run any continuations or pending
     # queries.
     jobs.onReady =>
-      # Apply filters.
       suggestions = suggestions.filter filter for filter in filters
-
-      # Should we run continuations?
       shouldRunContinuations = 0 < continuations.length and not @mostRecentQuery?
 
-      # Post results, unless there are none AND we will be running a continuation.  This avoids
+      # Post results, unless there are none and we will be running a continuation.  This avoids
       # collapsing the vomnibar briefly before expanding it again, which looks ugly.
       unless suggestions.length == 0 and shouldRunContinuations
+        suggestions = @prepareSuggestions queryTerms, suggestions
         onComplete
-          results: @prepareSuggestions queryTerms, suggestions
+          results: suggestions
           mayCacheResults: continuations.length == 0
 
       # Run any continuations (asynchronously).
       if shouldRunContinuations
-        continuationJobs = new JobRunner continuations.map (continuation) ->
+        jobs = new JobRunner continuations.map (continuation) ->
           (callback) ->
-            continuation suggestions, (newSuggestions) ->
+            continuation (newSuggestions) ->
               suggestions.push newSuggestions...
               callback()
 
-        continuationJobs.onReady =>
+        jobs.onReady =>
+          suggestions = @prepareSuggestions queryTerms, suggestions
           # We post these results even if a new query has started.  The vomnibar will not display the
-          # completions, but will cache the results.
+          # completions (they're arriving too late), but it will cache them.
           onComplete
-            results: @prepareSuggestions queryTerms, suggestions
+            results: suggestions
             mayCacheResults: true
 
       # Admit subsequent queries, and launch any pending query.
       @filterInProgress = false
       if @mostRecentQuery
-        console.log "running pending query:", @mostRecentQuery[0] if @debug
+        console.log "running pending query:", @mostRecentQuery[0].query if @debug
         @filter @mostRecentQuery...
 
   prepareSuggestions: (queryTerms, suggestions) ->
diff --git a/background_scripts/completion_search.coffee b/background_scripts/completion_search.coffee
index eb27c076..46533833 100644
--- a/background_scripts/completion_search.coffee
+++ b/background_scripts/completion_search.coffee
@@ -1,9 +1,12 @@
 
 CompletionSearch =
   debug: true
+  inTransit: {}
+  completionCache: new SimpleCache 2 * 60 * 60 * 1000, 5000 # Two hour, 5000 entries.
+  engineCache:new SimpleCache 1000 * 60 * 60 * 1000 # 1000 hours.
 
-  # The amount of time to wait for new requests before launching the HTTP request.  The intention is to cut
-  # down on the number of HTTP requests we issue.
+  # The amount of time to wait for new requests before launching the current request (for example, if the user
+  # is still typing).
   delay: 100
 
   get: (searchUrl, url, callback) ->
@@ -15,13 +18,11 @@ CompletionSearch =
 
     xhr.onreadystatechange = ->
       if xhr.readyState == 4
-        callback(if xhr.status == 200 then xhr else null)
+        callback if xhr.status == 200 then xhr else null
 
-  # Look up the completion engine for this searchUrl.  Because of DummyCompletionEngine, above, we know there
-  # will always be a match.  Imagining that there may be many completion engines, and knowing that this is
-  # called for every query, we cache the result.
+  # Look up the completion engine for this searchUrl.  Because of DummyCompletionEngine, we know there will
+  # always be a match.
   lookupEngine: (searchUrl) ->
-    @engineCache ?= new SimpleCache 30 * 60 * 60 * 1000 # 30 hours (these are small, we can keep them longer).
     if @engineCache.has searchUrl
       @engineCache.get searchUrl
     else
@@ -29,7 +30,7 @@ CompletionSearch =
         engine = new engine()
         return @engineCache.set searchUrl, engine if engine.match searchUrl
 
-  # True if we have a completion engine for this search URL, undefined otherwise.
+  # True if we have a completion engine for this search URL, false otherwise.
   haveCompletionEngine: (searchUrl) ->
     not @lookupEngine(searchUrl).dummy
 
@@ -39,17 +40,19 @@ CompletionSearch =
   #  - queryTerms are the query terms.
   #  - callback will be applied to a list of suggestion strings (which may be an empty list, if anything goes
   #    wrong).
+  #
+  # If no callback is provided, then we're to provide suggestions only if we can do so synchronously (ie.
+  # from a cache).  In this case we just return the results.  Returns null if we cannot service the request
+  # synchronously.
+  #
   complete: (searchUrl, queryTerms, callback = null) ->
-    query = queryTerms.join ""
+    query = queryTerms.join(" ").toLowerCase()
 
-    # If no callback is provided, then we're to provide suggestions only if we can do so synchronously (ie.
-    # from a cache).  In this case we just return the results.  Return null if we cannot service the request
-    # synchronously.
     returnResultsOnlyFromCache = not callback?
     callback ?= (suggestions) -> suggestions
 
-    # We don't complete single characters: the results are usually useless.
-    return callback [] unless 1 < query.length
+    # We don't complete queries which are too short: the results are usually useless.
+    return callback [] unless 3 < query.length
 
     # We don't complete regular URLs or Javascript URLs.
     return callback [] if 1 == queryTerms.length and Utils.isUrl query
@@ -57,26 +60,18 @@ CompletionSearch =
 
     # Cache completions.  However, completions depend upon both the searchUrl and the query terms.  So we need
     # to generate a key.  We mix in some junk generated by pwgen. A key clash might be possible, but
-    # vanishingly unlikely.
+    # is vanishingly unlikely.
     junk = "//Zi?ei5;o//"
     completionCacheKey = searchUrl + junk + queryTerms.map((s) -> s.toLowerCase()).join junk
-    @completionCache ?= new SimpleCache 60 * 60 * 1000, 2000 # One hour, 2000 entries.
+
     if @completionCache.has completionCacheKey
-      if returnResultsOnlyFromCache
-        return callback @completionCache.get completionCacheKey
-      else
-        # We add a short delay, even for a cache hit.  This avoids an ugly flicker when the additional
-        # suggestions are posted.
-        Utils.setTimeout 50, =>
-          console.log "hit", completionCacheKey if @debug
-          callback @completionCache.get completionCacheKey
-        return
+      console.log "hit", completionCacheKey if @debug
+      return callback @completionCache.get completionCacheKey
 
     # If the user appears to be typing a continuation of the characters of the most recent query, then we can
     # re-use the previous suggestions.
     if @mostRecentQuery? and @mostRecentSuggestions?
-      reusePreviousSuggestions = do (query) =>
-        query = queryTerms.join(" ").toLowerCase()
+      reusePreviousSuggestions = do =>
         # Verify that the previous query is a prefix of the current query.
         return false unless 0 == query.indexOf @mostRecentQuery.toLowerCase()
         # Ensure that every previous suggestion contains the text of the new query.
@@ -93,44 +88,42 @@ CompletionSearch =
     # That's all of the caches we can try.  Bail if the caller is looking for synchronous results.
     return callback null if returnResultsOnlyFromCache
 
-    fetchSuggestions = (engine, callback) =>
-      url = engine.getUrl queryTerms
-      query = queryTerms.join(" ").toLowerCase()
-      @get searchUrl, url, (xhr = null) =>
-        # Parsing the response may fail if we receive an unexpected or an unexpectedly-formatted response.  In
-        # all cases, we fall back to the catch clause, below.  Therefore, we "fail safe" in the case of
-        # incorrect or out-of-date completion engines.
-        try
-          suggestions = engine.parse xhr
-          # Filter out the query itself. It's not adding anything.
-          suggestions = (suggestion for suggestion in suggestions when suggestion.toLowerCase() != query)
-          console.log "GET", url if @debug
-        catch
-          suggestions = []
-          # We cache failures too, but remove them after just thirty minutes.
-          Utils.setTimeout 30 * 60 * 1000, => @completionCache.set completionCacheKey, null
-          console.log "fail", url if @debug
-
-        callback suggestions
-
     # We pause in case the user is still typing.
     Utils.setTimeout @delay, handler = @mostRecentHandler = =>
       if handler == @mostRecentHandler
         @mostRecentHandler = null
 
-        # Share duplicate requests. First fetch the suggestions...
-        @inTransit ?= {}
+        # Elide duplicate requests. First fetch the suggestions...
         @inTransit[completionCacheKey] ?= new AsyncDataFetcher (callback) =>
-          fetchSuggestions @lookupEngine(searchUrl), callback
+          engine = @lookupEngine searchUrl
+          url = engine.getUrl queryTerms
+
+          @get searchUrl, url, (xhr = null) =>
+            # Parsing the response may fail if we receive an unexpected or an unexpectedly-formatted response.
+            # In all cases, we fall back to the catch clause, below.  Therefore, we "fail safe" in the case of
+            # incorrect or out-of-date completion engines.
+            try
+              suggestions = engine.parse xhr
+              # Filter out the query itself. It's not adding anything.
+              suggestions = (suggestion for suggestion in suggestions when suggestion.toLowerCase() != query)
+              console.log "GET", url if @debug
+            catch
+              suggestions = []
+              # We cache failures too, but remove them after just thirty minutes.
+              Utils.setTimeout 30 * 60 * 1000, => @completionCache.set completionCacheKey, null
+              console.log "fail", url if @debug
+
+            callback suggestions
 
         # ... then use the suggestions.
         @inTransit[completionCacheKey].use (suggestions) =>
-          @mostRecentQuery = queryTerms.join " "
+          @mostRecentQuery = query
           @mostRecentSuggestions = suggestions
           callback @completionCache.set completionCacheKey, suggestions
           delete @inTransit[completionCacheKey]
 
-  # Cancel any pending (ie. blocked on @delay) queries.  Does not cancel in-flight queries.
+  # Cancel any pending (ie. blocked on @delay) queries.  Does not cancel in-flight queries.  This is called
+  # whenever the user is typing.
   cancel: ->
     if @mostRecentHandler?
       @mostRecentHandler = null
diff --git a/pages/vomnibar.coffee b/pages/vomnibar.coffee
index df67d2eb..74963bfe 100644
--- a/pages/vomnibar.coffee
+++ b/pages/vomnibar.coffee
@@ -112,7 +112,7 @@ class VomnibarUI
     for i in [0...@completionList.children.length]
       @completionList.children[i].className = (if i == @selection then "vomnibarSelected" else "")
 
-  highlightCommonMatches: (response) ->
+  selectCommonMatches: (response) ->
     # For custom search engines, add characters to the input which are:
     #   - not in the query/input
     #   - in all completions
@@ -134,7 +134,7 @@ class VomnibarUI
 
     # Get the completions from which we can select text to highlight.
     completions = @completions.filter (completion) ->
-      completion.highlightCommonMatches? and completion.highlightCommonMatches
+      completion.selectCommonMatches? and completion.selectCommonMatches
 
     # Fetch the query and the suggestion texts.
     query = @input.value.ltrim().split(/\s+/).join(" ").toLowerCase()
@@ -246,7 +246,7 @@ class VomnibarUI
         return unless 0 < query.length
         if @suppressedLeadingKeyword?
           # This is a custom search engine completion.  Because of the way we add and highlight the text
-          # common to all completions in the input (highlightCommonMatches), the text in the input might not
+          # common to all completions in the input (selectCommonMatches), the text in the input might not
           # correspond to any of the completions.  So we fire the query off to the background page and use the
           # completion at the top of the list (which will be the right one).
           @update true, =>
@@ -317,7 +317,7 @@ class VomnibarUI
       @selection = Math.min @completions.length - 1, Math.max @initialSelectionValue, @selection
       @previousAutoSelect = null if @completions[0]?.autoSelect and @completions[0]?.forceAutoSelect
       @updateSelection()
-      @highlightCommonMatches response
+      @selectCommonMatches response
       callback?()
 
   updateOnInput: =>