Search completion; refactor searchEngineCompleter.

This revamps how search-engine configuration is handled, and revises
some rather strange legacy code.

5 files changed, 139 insertions, 164 deletions

diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee
index cc334d78..94109b84 100644
--- a/background_scripts/completion.coffee
+++ b/background_scripts/completion.coffee
@@ -355,143 +355,135 @@ class TabCompleter
       tabRecency.recencyScore(suggestion.tabId)
 
 class SearchEngineCompleter
-  searchEngines: {}
-
-  cancel: ->
-    CompletionEngines.cancel()
+  searchEngineConfig: null
 
   refresh: (port) ->
-    @searchEngines = SearchEngineCompleter.getSearchEngines()
-    # Let the vomnibar know the custom search engine keywords.
-    port.postMessage
-      handler: "customSearchEngineKeywords"
-      keywords: key for own key of @searchEngines
+    # Load and parse the search-engine configuration.
+    @searchEngineConfig = new AsyncDataFetcher (callback) ->
+      engines = {}
+      for line in Settings.get("searchEngines").split "\n"
+        line = line.trim()
+        continue if /^[#"]/.test line
+        tokens = line.split /\s+/
+        continue unless 2 <= tokens.length
+        keyword = tokens[0].split(":")[0]
+        description = tokens[2..].join(" ") || "search (#{keyword})"
+        engines[keyword] =
+          keyword: keyword
+          searchUrl: tokens[1]
+          description: description
+
+      # Deliver the resulting engines lookup table.
+      callback engines
+
+      # Let the vomnibar know the custom search engine keywords.
+      port.postMessage
+        handler: "customSearchEngineKeywords"
+        keywords: key for own key of engines
 
   filter: ({ queryTerms, query }, onComplete) ->
     return onComplete [] if queryTerms.length == 0
-    suggestions = []
-
-    { keyword, searchUrl, description } = @getSearchEngineMatches queryTerms, query
-    custom = searchUrl? and keyword?
-    searchUrl ?= Settings.get "searchUrl"
-    haveDescription = description? and 0 < description.length
-    description ||= "search#{if custom then " [#{keyword}]" else ""}"
-
-    queryTerms = queryTerms[1..] if custom
-    query = queryTerms.join " "
-
-    haveCompletionEngine = CompletionEngines.haveCompletionEngine searchUrl
-    # If this is a custom search engine and we have a completer, then exclude results from other completers.
-    # The truthiness of filter? also serves to distinguish two very different vomnibar behaviors, one for
-    # custom search engines with completers (filter? is true), and one for those without (filter? is false).
-    filter =
-      if custom and haveCompletionEngine
-        (suggestion) -> suggestion.type == description
-      else
-        null
 
-    # For custom search engines, we add an auto-selected suggestion.
-    if custom
-      # Note. This suggestion always appears at the top of the suggestion list.  Its settings serves to serve
-      # to configure various vomnibar behaviors.
-      suggestions.push new Suggestion
-        queryTerms: queryTerms
-        type: description
-        url: Utils.createSearchUrl queryTerms, searchUrl
-        title: query
-        relevancy: 1
-        insertText: if filter? then query else null
-        # For all custom search engines, we suppress the leading keyword, for example "w something" becomes
-        # "something" in the vomnibar.
-        suppressLeadingKeyword: true
-        # We complete suggestions only for custom search engines where we have an associated completion
-        # engine.
-        completeSuggestions: filter?
-        # We only use autoSelect and highlight query terms (on custom search engines) when we do not have a
-        # completer.
-        autoSelect: not filter?
-        forceAutoSelect: not filter?
-        highlightTerms: not filter?
-
-    # Post suggestions and bail if there is no prospect of adding further suggestions.
-    if queryTerms.length == 0 or not haveCompletionEngine
-      return onComplete suggestions, { filter }
-
-    onComplete suggestions,
-      filter: filter
-      continuation: (existingSuggestions, onComplete) =>
-        suggestions = []
-        # For custom search-engine queries, this adds suggestions only if we have a completer.  For other queries,
-        # this adds suggestions for the default search engine (if we have a completer for that).
-
-        # 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.
-        #   - 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)
-
-        if 0 < existingSuggestions.length
-          existingSuggestionsMinScore = existingSuggestions[existingSuggestions.length-1].relevancy
-          if relavancy < existingSuggestionsMinScore and MultiCompleter.maxResults <= existingSuggestions.length
-            # No suggestion we propose will have a high enough relavancy to beat the existing suggestions, so bail
-            # immediately.
-            return onComplete []
-
-        CompletionEngines.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
-
-          # 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, MultiCompleter.maxResults - existingSuggestions.length
-          onComplete suggestions[...count]
-
-  getSearchEngineMatches: (queryTerms, query = queryTerms.join " ") ->
-    # To allow users to write queries with leading search-engine keywords, leading whitespace disables custom
-    # search engines; for example, " w" is a regular query.
-    return {} if /^\s/.test query
-    # Trailing whitespace is significant when activating a custom search engine; for example, "w" (just a
-    # regular query) is different from "w " (a custom search engine).
-    length = queryTerms.length + (if /\s$/.test query then 1 else 0)
-    (1 < length and @searchEngines[queryTerms[0]]) or {}
-
-  # Static data and methods for parsing the configured search engines.  We keep a cache of the search-engine
-  # mapping in @searchEnginesMap.
-  @searchEnginesMap: null
-
-  # Parse the custom search engines setting and cache it in SearchEngineCompleter.searchEnginesMap.
-  @parseSearchEngines: (searchEnginesText) ->
-    searchEnginesMap = SearchEngineCompleter.searchEnginesMap = {}
-    for line in searchEnginesText.split /\n/
-      tokens = line.trim().split /\s+/
-      continue if tokens.length < 2 or tokens[0].startsWith('"') or tokens[0].startsWith("#")
-      keywords = tokens[0].split ":"
-      continue unless keywords.length == 2 and not keywords[1] # So, like: [ "w", "" ].
-      searchEnginesMap[keywords[0]] =
-        keyword: keywords[0]
-        searchUrl: tokens[1]
-        description: tokens[2..].join(" ")
-
-  # Fetch the search-engine map, building it if necessary.
-  @getSearchEngines: ->
-    unless SearchEngineCompleter.searchEnginesMap?
-      SearchEngineCompleter.parseSearchEngines Settings.get "searchEngines"
-    SearchEngineCompleter.searchEnginesMap
+    @searchEngineConfig.use (engines) =>
+      keyword = queryTerms[0]
+
+      { custom, searchUrl, description, queryTerms } =
+        if engines[keyword]? and (1 < queryTerms.length or /\s$/.test query)
+          { searchUrl, description } = engines[keyword]
+          custom: true
+          searchUrl: searchUrl
+          description: description
+          queryTerms: queryTerms[1..]
+        else
+          custom: false
+          searchUrl: Settings.get "searchUrl"
+          description: "search"
+          queryTerms: queryTerms
+
+      query = queryTerms.join " "
+      haveCompletionEngine = CompletionEngines.haveCompletionEngine searchUrl
+
+      # 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.
+      version2 = custom and haveCompletionEngine
+
+      # If this is a custom search engine and we have a completer, then we exclude results from other
+      # completers.
+      filter = if version2 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.
+      if custom
+        suggestions.push new Suggestion
+          queryTerms: queryTerms
+          type: description
+          url: Utils.createSearchUrl queryTerms, searchUrl
+          title: query
+          relevancy: 1
+          insertText: if version2 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: version2
+          # Toggles for the legacy behaviour.
+          autoSelect: not version2
+          forceAutoSelect: not version2
+          highlightTerms: not version2
+
+      # Post suggestions and bail if there is no prospect of adding further suggestions.
+      if queryTerms.length == 0 or not haveCompletionEngine
+        return onComplete suggestions, { filter }
+
+      # Post any initial suggestion, and then deliver suggestions from completion engines as a continuation
+      # (so, asynchronously).
+      onComplete suggestions,
+        filter: filter
+        continuation: (existingSuggestions, onComplete) =>
+          suggestions = []
+          # 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.
+          #   - 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)
+
+          if 0 < existingSuggestions.length
+            existingSuggestionsMinScore = existingSuggestions[existingSuggestions.length-1].relevancy
+            if relavancy < existingSuggestionsMinScore and MultiCompleter.maxResults <= existingSuggestions.length
+              # No suggestion we propose will have a high enough relavancy to beat the existing suggestions, so bail
+              # immediately.
+              return onComplete []
+
+          CompletionEngines.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
+
+            # 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, MultiCompleter.maxResults - existingSuggestions.length
+            onComplete suggestions[...count]
+
+  cancel: ->
+    CompletionEngines.cancel()
 
 # 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.
diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee
index 44ed897d..11f492d7 100644
--- a/background_scripts/settings.coffee
+++ b/background_scripts/settings.coffee
@@ -32,9 +32,6 @@ root.Settings = Settings =
       root.Commands.parseCustomKeyMappings value
       root.refreshCompletionKeysAfterMappingSave()
 
-    searchEngines: (value) ->
-      root.SearchEngineCompleter.parseSearchEngines value
-
     exclusionRules: (value) ->
       root.Exclusions.postUpdateHook value
 
diff --git a/lib/utils.coffee b/lib/utils.coffee
index 4c2a7a14..51b16351 100644
--- a/lib/utils.coffee
+++ b/lib/utils.coffee
@@ -260,6 +260,22 @@ class SimpleCache
       delete @previous[key]
     value
 
+# This is a simple class for the common case where we want to use some data value which may be immediately
+# available, or we may have to wait.  It implements the use-immediately-or-wait queue, and calls the function
+# to fetch the data asynchronously.
+class AsyncDataFetcher
+  constructor: (fetch) ->
+    @data = null
+    @queue = []
+    fetch (@data) =>
+      Utils.nextTick =>
+        callback @data for callback in @queue
+        @queue = null
+
+  use: (callback) ->
+    if @data? then callback @data else @queue.push callback
+
 root = exports ? window
 root.Utils = Utils
 root.SimpleCache = SimpleCache
+root.AsyncDataFetcher = AsyncDataFetcher
diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee
index b45c99dd..7711dac4 100644
--- a/tests/unit_tests/completion_test.coffee
+++ b/tests/unit_tests/completion_test.coffee
@@ -236,27 +236,6 @@ context "tab completer",
     assert.arrayEqual ["tab2.com"], results.map (tab) -> tab.url
     assert.arrayEqual [2], results.map (tab) -> tab.tabId
 
-context "search engines",
-  setup ->
-    searchEngines = "foo: bar?q=%s\n# comment\nbaz: qux?q=%s baz description"
-    Settings.set 'searchEngines', searchEngines
-    @completer = new SearchEngineCompleter()
-    # note, I couldn't just call @completer.refresh() here as I couldn't set root.Settings without errors
-    # workaround is below, would be good for someone that understands the testing system better than me to improve
-    @completer.searchEngines = SearchEngineCompleter.getSearchEngines()
-
-  should "return search engine suggestion without description", ->
-    results = filterCompleter(@completer, ["foo", "hello"])
-    assert.arrayEqual ["bar?q=hello"], results.map (result) -> result.url
-    assert.arrayEqual ["hello"], results.map (result) -> result.title
-    assert.arrayEqual ["search [foo]"], results.map (result) -> result.type
-
-  should "return search engine suggestion with description", ->
-    results = filterCompleter(@completer, ["baz", "hello"])
-    # assert.arrayEqual ["qux?q=hello"], results.map (result) -> result.searchUrl
-    # assert.arrayEqual ["hello"], results.map (result) -> result.title
-    # assert.arrayEqual ["baz description"], results.map (result) -> result.type
-
 context "suggestions",
   should "escape html in page titles", ->
     suggestion = new Suggestion
diff --git a/tests/unit_tests/settings_test.coffee b/tests/unit_tests/settings_test.coffee
index 52eec30d..4cd20211 100644
--- a/tests/unit_tests/settings_test.coffee
+++ b/tests/unit_tests/settings_test.coffee
@@ -70,14 +70,5 @@ context "settings",
     chrome.storage.sync.set { scrollStepSize: JSON.stringify(message) }
     assert.equal message, Sync.message
 
-  should "set search engines, retrieve them correctly and check that they have been parsed correctly", ->
-    searchEngines = "foo: bar?q=%s\n# comment\nbaz: qux?q=%s baz description"
-    Settings.set 'searchEngines', searchEngines
-    result = SearchEngineCompleter.getSearchEngines()
-    assert.equal "bar?q=%s", result["foo"].searchUrl
-    assert.isFalse result["foo"].description
-    assert.equal "qux?q=%s", result["baz"].searchUrl
-    assert.equal "baz description", result["baz"].description
-
   should "sync a key which is not a known setting (without crashing)", ->
     chrome.storage.sync.set { notASetting: JSON.stringify("notAUsefullValue") }