From 5aaed906b94337bb2136d74672e16679ab457c49 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 10 Jun 2015 14:23:58 +0100
Subject: Rank filtered hints by score.

Thus, better matches are likely to either be first (so just hitting
<Enter> activates them) or just a <Tab> or two away.

Scoring:
   - Requires that every search term be matched.
   - Assigns higher scores to matches at the start of a word, and higher
     scores still for whole-word matches.
---
 content_scripts/link_hints.coffee | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee
index 107a292e..ae9d3f8f 100644
--- a/content_scripts/link_hints.coffee
+++ b/content_scripts/link_hints.coffee
@@ -565,13 +565,41 @@ class FilterHints
   filterLinkHints: (hintMarkers) ->
     idx = 0
     linkSearchString = @linkTextKeystrokeQueue.join("").toLowerCase()
+    do (scoreFunction = @scoreLinkHint linkSearchString) ->
+      linkMarker.score = scoreFunction linkMarker for linkMarker in hintMarkers
+    hintMarkers = hintMarkers[..].sort (a,b) -> b.score - a.score
 
     for linkMarker in hintMarkers
-      continue unless 0 <= linkMarker.linkText.toLowerCase().indexOf linkSearchString
+      continue unless 0 < linkMarker.score
       linkMarker.hintString = @generateHintString idx++
       @renderMarker linkMarker
       linkMarker
 
+  # Assign a score to a filter match (higher is better).  We assign a higher score for matches at the start of
+  # a word, and a considerably higher score still for matches which are whole words.
+  # Note(smblott) if linkSearchString is empty, then every hint get a score of 2.
+  scoreLinkHint: (linkSearchString) ->
+    searchWords = linkSearchString.trim().split /\s+/
+    (linkMarker) ->
+      linkWords = linkMarker.linkWords ?= linkMarker.linkText.trim().toLowerCase().split /\s+/
+
+      searchWordScores =
+        for searchWord in searchWords
+          linkWordScores =
+            for linkWord in linkWords
+              if linkWord == searchWord
+                5
+              else if linkWord.startsWith searchWord
+                2
+              else if 0 <= linkWord.indexOf searchWord
+                1
+              else
+                0
+          Math.max linkWordScores...
+
+      addFunc = (a,b) -> a + b
+      if 0 in searchWordScores then 0 else searchWordScores.reduce addFunc, 0
+
 #
 # Make each hint character a span, so that we can highlight the typed characters as you type them.
 #
-- 
cgit v1.2.3


From ddc747805e08eaad52bf58950b1d2df9bdd2da9c Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 10 Jun 2015 14:43:00 +0100
Subject: Account for words at start of filter text.

We gove these a higher score because it makes it easier for the user to
pick a link if they can just start typing at the beginning.
---
 content_scripts/link_hints.coffee | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee
index ae9d3f8f..3dbb4f23 100644
--- a/content_scripts/link_hints.coffee
+++ b/content_scripts/link_hints.coffee
@@ -586,11 +586,11 @@ class FilterHints
       searchWordScores =
         for searchWord in searchWords
           linkWordScores =
-            for linkWord in linkWords
+            for linkWord, idx in linkWords
               if linkWord == searchWord
-                5
+                if idx == 0 then 8 else 6
               else if linkWord.startsWith searchWord
-                2
+                if idx == 0 then 4 else 2
               else if 0 <= linkWord.indexOf searchWord
                 1
               else
-- 
cgit v1.2.3


From 9b07fa7861620900e0d1da5829b38ddb4e8ff789 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 10 Jun 2015 15:12:45 +0100
Subject: Note changes in README.

---
 README.md                         | 3 ++-
 content_scripts/link_hints.coffee | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/README.md b/README.md
index a3bf84bb..b2b865f1 100644
--- a/README.md
+++ b/README.md
@@ -163,7 +163,8 @@ Release Notes
 - Added <tt>\`\`</tt> to jump back to the previous position after selected jump-like movements: <br/>
     (`gg`, `G`, `n`, `N`, `/` and local mark movements).
 - Global marks are now persistent (across tab closes and browser sessions) and synced.
-- For filtered link hints (not the default), you can now use `Tab` to select hints.
+- For filtered link hints (not the default), you can now use `Tab` and `Enter`
+  to select hints and hints are ordered by best match.
 - Bug fixes, including:
     - Bookmarklets accessed from the Vomnibar.
     - Global marks on non-Windows platforms.
diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee
index 3dbb4f23..cbb4085e 100644
--- a/content_scripts/link_hints.coffee
+++ b/content_scripts/link_hints.coffee
@@ -577,7 +577,7 @@ class FilterHints
 
   # Assign a score to a filter match (higher is better).  We assign a higher score for matches at the start of
   # a word, and a considerably higher score still for matches which are whole words.
-  # Note(smblott) if linkSearchString is empty, then every hint get a score of 2.
+  # Note(smblott) if linkSearchString is empty, then every hint get a score of 4.
   scoreLinkHint: (linkSearchString) ->
     searchWords = linkSearchString.trim().split /\s+/
     (linkMarker) ->
-- 
cgit v1.2.3


From e516bae3a3374780d2cb1b6c32e5fd1f2c13a408 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 10 Jun 2015 16:14:38 +0100
Subject: Fix incorrect filtering.

---
 content_scripts/link_hints.coffee | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee
index cbb4085e..144400b6 100644
--- a/content_scripts/link_hints.coffee
+++ b/content_scripts/link_hints.coffee
@@ -564,9 +564,15 @@ class FilterHints
   # Filter link hints by search string, renumbering the hints as necessary.
   filterLinkHints: (hintMarkers) ->
     idx = 0
-    linkSearchString = @linkTextKeystrokeQueue.join("").toLowerCase()
+    linkSearchString = @linkTextKeystrokeQueue.join("").trim().toLowerCase()
+    return hintMarkers unless 0 < linkSearchString.length
+
     do (scoreFunction = @scoreLinkHint linkSearchString) ->
       linkMarker.score = scoreFunction linkMarker for linkMarker in hintMarkers
+    # The Javascript sort() method is known not to be stable.  Nevertheless, we require (and assume, here)
+    # that it is deterministic.  So, if the user is typing hint characters, then hints will always end up in
+    # the same order and hence with the same hint strings (because hint-string filtering happens after the
+    # filtering here).
     hintMarkers = hintMarkers[..].sort (a,b) -> b.score - a.score
 
     for linkMarker in hintMarkers
@@ -577,7 +583,6 @@ class FilterHints
 
   # Assign a score to a filter match (higher is better).  We assign a higher score for matches at the start of
   # a word, and a considerably higher score still for matches which are whole words.
-  # Note(smblott) if linkSearchString is empty, then every hint get a score of 4.
   scoreLinkHint: (linkSearchString) ->
     searchWords = linkSearchString.trim().split /\s+/
     (linkMarker) ->
-- 
cgit v1.2.3


From 61764d812a37ca2c29b3b7ddde878f25250abf81 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 10 Jun 2015 16:39:02 +0100
Subject: Fix bug relating to duplicate hint strings.

(Not sure when this crept in.)

We need to ensure that we always generate the same hint strings for the
same filter state.  Here, we do this by always using the same mechanism
(@filterLinkHints) to set the hint strings.
---
 content_scripts/link_hints.coffee | 15 +++++++--------
 tests/dom_tests/dom_tests.coffee  | 27 +++++++++++++++++----------
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee
index 144400b6..15af15c5 100644
--- a/content_scripts/link_hints.coffee
+++ b/content_scripts/link_hints.coffee
@@ -478,7 +478,7 @@ class FilterHints
         @labelMap[forElement] = labelText
 
   generateHintString: (linkHintNumber) ->
-    numberToHintString linkHintNumber + 1, @linkHintNumbers.toUpperCase()
+    numberToHintString linkHintNumber, @linkHintNumbers.toUpperCase()
 
   generateLinkText: (element) ->
     linkText = ""
@@ -512,8 +512,7 @@ class FilterHints
   fillInMarkers: (hintMarkers) ->
     @generateLabelMap()
     DomUtils.textContent.reset()
-    for marker, idx in hintMarkers
-      marker.hintString = @generateHintString(idx)
+    for marker in hintMarkers
       linkTextObject = @generateLinkText(marker.clickableItem)
       marker.linkText = linkTextObject.text
       marker.showLinkText = linkTextObject.show
@@ -522,7 +521,9 @@ class FilterHints
     @activeHintMarker = hintMarkers[0]
     @activeHintMarker?.classList.add "vimiumActiveHintMarker"
 
-    hintMarkers
+    # We use @filterLinkHints() here (although we know that all of the hints will match) to fill in the hint
+    # strings.  This ensures that we always get hint strings in the same order.
+    @filterLinkHints hintMarkers
 
   getMatchingHints: (hintMarkers, tabCount = 0) ->
     delay = 0
@@ -563,10 +564,7 @@ class FilterHints
 
   # Filter link hints by search string, renumbering the hints as necessary.
   filterLinkHints: (hintMarkers) ->
-    idx = 0
     linkSearchString = @linkTextKeystrokeQueue.join("").trim().toLowerCase()
-    return hintMarkers unless 0 < linkSearchString.length
-
     do (scoreFunction = @scoreLinkHint linkSearchString) ->
       linkMarker.score = scoreFunction linkMarker for linkMarker in hintMarkers
     # The Javascript sort() method is known not to be stable.  Nevertheless, we require (and assume, here)
@@ -575,9 +573,10 @@ class FilterHints
     # filtering here).
     hintMarkers = hintMarkers[..].sort (a,b) -> b.score - a.score
 
+    linkHintNumber = 1
     for linkMarker in hintMarkers
       continue unless 0 < linkMarker.score
-      linkMarker.hintString = @generateHintString idx++
+      linkMarker.hintString = @generateHintString linkHintNumber++
       @renderMarker linkMarker
       linkMarker
 
diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee
index dd2f5a5d..a79735ae 100644
--- a/tests/dom_tests/dom_tests.coffee
+++ b/tests/dom_tests/dom_tests.coffee
@@ -212,11 +212,14 @@ context "Filtered link hints",
       @linkHints.deactivateMode()
 
     should "label the images", ->
-      hintMarkers = getHintMarkers()
-      assert.equal "1: alt text", hintMarkers[0].textContent.toLowerCase()
-      assert.equal "2: some title", hintMarkers[1].textContent.toLowerCase()
-      assert.equal "3: alt text", hintMarkers[2].textContent.toLowerCase()
-      assert.equal "4", hintMarkers[3].textContent.toLowerCase()
+      hintMarkers = getHintMarkers().map (marker) -> marker.textContent.toLowerCase()
+      # We don't know the actual hint numbers which will be assigned, so we replace them with "N".
+      hintMarkers = hintMarkers.map (str) -> str.replace /^[1-4]/, "N"
+      assert.equal 4, hintMarkers.length
+      assert.isTrue "N: alt text" in hintMarkers
+      assert.isTrue "N: some title" in hintMarkers
+      assert.isTrue "N: alt text" in hintMarkers
+      assert.isTrue "N" in hintMarkers
 
   context "Input hints",
 
@@ -235,11 +238,15 @@ context "Filtered link hints",
 
     should "label the input elements", ->
       hintMarkers = getHintMarkers()
-      assert.equal "1", hintMarkers[0].textContent.toLowerCase()
-      assert.equal "2", hintMarkers[1].textContent.toLowerCase()
-      assert.equal "3: a label", hintMarkers[2].textContent.toLowerCase()
-      assert.equal "4: a label", hintMarkers[3].textContent.toLowerCase()
-      assert.equal "5", hintMarkers[4].textContent.toLowerCase()
+      hintMarkers = getHintMarkers().map (marker) -> marker.textContent.toLowerCase()
+      # We don't know the actual hint numbers which will be assigned, so we replace them with "N".
+      hintMarkers = hintMarkers.map (str) -> str.replace /^[1-5]/, "N"
+      assert.equal 5, hintMarkers.length
+      assert.isTrue "N" in hintMarkers
+      assert.isTrue "N" in hintMarkers
+      assert.isTrue "N: a label" in hintMarkers
+      assert.isTrue "N: a label" in hintMarkers
+      assert.isTrue "N" in hintMarkers
 
 context "Input focus",
 
-- 
cgit v1.2.3


From 91bbb3122b58e834cc5512768eb09046fac2b448 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 10 Jun 2015 18:23:40 +0100
Subject: Re-work tabMoveLeft/Right.

Note. This does not allow tabs to rotate from the left around to the
right, or vice versa.  Which means "999<<" moves the current tab all the
way to the left (and similarly to the right).

Fixes #1727 (kind of).
---
 background_scripts/main.coffee | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee
index 835b8a9a..6e1226b6 100644
--- a/background_scripts/main.coffee
+++ b/background_scripts/main.coffee
@@ -237,11 +237,11 @@ repeatFunction = (func, totalCount, currentCount, frameId) ->
       -> repeatFunction(func, totalCount, currentCount + 1, frameId),
       frameId)
 
-moveTab = (callback, direction) ->
-  chrome.tabs.getSelected(null, (tab) ->
-    # Use Math.max to prevent -1 as the new index, otherwise the tab of index n will wrap to the far RHS when
-    # moved left by exactly (n+1) places.
-    chrome.tabs.move(tab.id, {index: Math.max(0, tab.index + direction) }, callback))
+moveTab = (count) ->
+  chrome.tabs.getAllInWindow null, (tabs) ->
+    chrome.tabs.getSelected null, (tab) ->
+      chrome.tabs.move tab.id,
+        index: Math.max 0, Math.min tabs.length - 1, tab.index + count
 
 # Start action functions
 
@@ -304,8 +304,8 @@ BackgroundCommands =
     chrome.tabs.getSelected(null, (tab) ->
       chrome.tabs.sendMessage(tab.id,
         { name: "toggleHelpDialog", dialogHtml: helpDialogHtml(), frameId:frameId }))
-  moveTabLeft: (count) -> moveTab(null, -count)
-  moveTabRight: (count) -> moveTab(null, count)
+  moveTabLeft: (count) -> moveTab -count
+  moveTabRight: (count) -> moveTab count
   nextFrame: (count,frameId) ->
     chrome.tabs.getSelected null, (tab) ->
       frameIdsForTab[tab.id] = cycleToFrame frameIdsForTab[tab.id], frameId, count
-- 
cgit v1.2.3


From 30cfcbdcfaecf03de67561ae88574fac837f75e3 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 10 Jun 2015 19:21:41 +0100
Subject: Do not try to move a tab to the left of pinned tabs.

---
 background_scripts/main.coffee | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee
index 6e1226b6..272c3aa6 100644
--- a/background_scripts/main.coffee
+++ b/background_scripts/main.coffee
@@ -239,9 +239,12 @@ repeatFunction = (func, totalCount, currentCount, frameId) ->
 
 moveTab = (count) ->
   chrome.tabs.getAllInWindow null, (tabs) ->
+    pinnedCount = 0
+    for tab in tabs
+      pinnedCount += 1 if tab.pinned
     chrome.tabs.getSelected null, (tab) ->
       chrome.tabs.move tab.id,
-        index: Math.max 0, Math.min tabs.length - 1, tab.index + count
+        index: Math.max pinnedCount, Math.min tabs.length - 1, tab.index + count
 
 # Start action functions
 
-- 
cgit v1.2.3


From abc24123a9bec8f07d019d31c08a8740e695401d Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 10 Jun 2015 20:01:12 +0100
Subject: Refactor moveTab for clarity...

... as suggested by @mrmr1993 in #1728.
---
 background_scripts/main.coffee | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee
index 272c3aa6..d4b14f3c 100644
--- a/background_scripts/main.coffee
+++ b/background_scripts/main.coffee
@@ -239,9 +239,7 @@ repeatFunction = (func, totalCount, currentCount, frameId) ->
 
 moveTab = (count) ->
   chrome.tabs.getAllInWindow null, (tabs) ->
-    pinnedCount = 0
-    for tab in tabs
-      pinnedCount += 1 if tab.pinned
+    pinnedCount = (tabs.filter (tab) -> tab.pinned).length
     chrome.tabs.getSelected null, (tab) ->
       chrome.tabs.move tab.id,
         index: Math.max pinnedCount, Math.min tabs.length - 1, tab.index + count
-- 
cgit v1.2.3


From 420f703794ce978d2d68433aacb0247d708b1c06 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 10 Jun 2015 20:45:43 +0100
Subject: Initialise options-page link-hint mode correctly.

---
 pages/options.coffee | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pages/options.coffee b/pages/options.coffee
index ea4301a9..ba1a7ca2 100644
--- a/pages/options.coffee
+++ b/pages/options.coffee
@@ -229,6 +229,7 @@ initOptionsPage = ->
     element.className = element.className + " example info"
     element.innerHTML = "Leave empty to reset this option."
 
+  $("filterLinkHints").checked = bgSettings.get "filterLinkHints"
   maintainLinkHintsView()
   window.onbeforeunload = -> "You have unsaved changes to options." unless $("saveOptions").disabled
 
-- 
cgit v1.2.3


From 194cf92bd6d8624bd75b7092c5c51ec454919503 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Thu, 11 Jun 2015 05:54:17 +0100
Subject: Initialise options-page link-hint mode correctly (better).

---
 pages/options.coffee | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pages/options.coffee b/pages/options.coffee
index ba1a7ca2..21e81c8f 100644
--- a/pages/options.coffee
+++ b/pages/options.coffee
@@ -229,8 +229,6 @@ initOptionsPage = ->
     element.className = element.className + " example info"
     element.innerHTML = "Leave empty to reset this option."
 
-  $("filterLinkHints").checked = bgSettings.get "filterLinkHints"
-  maintainLinkHintsView()
   window.onbeforeunload = -> "You have unsaved changes to options." unless $("saveOptions").disabled
 
   document.addEventListener "keyup", (event) ->
@@ -260,6 +258,8 @@ initOptionsPage = ->
   for name, type of options
     new type(name,onUpdated)
 
+  maintainLinkHintsView()
+
 initPopupPage = ->
   chrome.tabs.getSelected null, (tab) ->
     exclusions = null
-- 
cgit v1.2.3


From 53d131700e5f33cb9476f00a905c238b0083f3dc Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Thu, 11 Jun 2015 06:25:41 +0100
Subject: Make "a-z" characters work in filter hints mode.

When we read hint characters, we read them lower case.  When we generate
hint markers, we generate them upper case.  So they never match.

(Exactly why anyone would want to use "abcde" for filtered link hints
isn't clear, but at least we should behave correctly.)
---
 content_scripts/link_hints.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee
index 15af15c5..1be762c6 100644
--- a/content_scripts/link_hints.coffee
+++ b/content_scripts/link_hints.coffee
@@ -532,7 +532,7 @@ class FilterHints
     # input. use them to filter the link hints accordingly.
     matchString = @hintKeystrokeQueue.join ""
     linksMatched = @filterLinkHints hintMarkers
-    linksMatched = linksMatched.filter (linkMarker) -> linkMarker.hintString.startsWith matchString
+    linksMatched = linksMatched.filter (linkMarker) -> linkMarker.hintString.toLowerCase().startsWith matchString
 
     if linksMatched.length == 1 && @hintKeystrokeQueue.length == 0 and 0 < @linkTextKeystrokeQueue.length
       # In filter mode, people tend to type out words past the point needed for a unique match. Hence we
-- 
cgit v1.2.3


From a414ed803a33c7af57441fa8527db1dfad79ed41 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Thu, 11 Jun 2015 06:32:04 +0100
Subject: Revert "Make "a-z" characters work in filter hints mode."

This reverts commit 53d131700e5f33cb9476f00a905c238b0083f3dc.

(This needs more thought.)
---
 content_scripts/link_hints.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee
index 1be762c6..15af15c5 100644
--- a/content_scripts/link_hints.coffee
+++ b/content_scripts/link_hints.coffee
@@ -532,7 +532,7 @@ class FilterHints
     # input. use them to filter the link hints accordingly.
     matchString = @hintKeystrokeQueue.join ""
     linksMatched = @filterLinkHints hintMarkers
-    linksMatched = linksMatched.filter (linkMarker) -> linkMarker.hintString.toLowerCase().startsWith matchString
+    linksMatched = linksMatched.filter (linkMarker) -> linkMarker.hintString.startsWith matchString
 
     if linksMatched.length == 1 && @hintKeystrokeQueue.length == 0 and 0 < @linkTextKeystrokeQueue.length
       # In filter mode, people tend to type out words past the point needed for a unique match. Hence we
-- 
cgit v1.2.3


From aa00e29dc2533b6701c65935223599671c5833b1 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sun, 14 Jun 2015 07:40:13 +0100
Subject: Add default value for regexFindMode setting.

Because we haven't had a default value for this setting, we never sync
it, which means -- when its not at its default value -- it isn't picked
up in content scripts by the new settings system.

Fixes #1731.
---
 lib/settings.coffee | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/settings.coffee b/lib/settings.coffee
index ca4e77b0..842f7618 100644
--- a/lib/settings.coffee
+++ b/lib/settings.coffee
@@ -146,6 +146,7 @@ Settings =
       """
     newTabUrl: "chrome://newtab"
     grabBackFocus: false
+    regexFindMode: false
 
     settingsVersion: Utils.getCurrentVersion()
     helpDialog_showAdvancedCommands: false
-- 
cgit v1.2.3


From 8ff1aef751a533c17e683207dae1eb165b210f92 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 17 Jun 2015 05:08:03 +0100
Subject: Fix non-default front-end settings.

(@mrmr1993: This is yet another approach to the Settings problem.)

With the new Settings implemetation, settings which have a non-default
value and which are not in synced storage (that is, they have not been
changed since synced storage was introduced) are not currently
accessible to content scripts.  This commit makes such settings
accessible via chrome.storage.local.

Important:
- There's a change to the established settings data model here.
  Previously, settings with default values were not stored; here, they
  are.  This eliminates a considerable amount logic from Settings, but
  means that migrations will be required if default values are changed
  in future.  (Other than type changes, have we ever changed a default
  value?)
- There's also a change (bug fix?) to the behaviour when an affected
  setting is reset to its default value.  Previously, the change would
  *not* have been synced (whereas all other changes are).  Here, such
  changes *are* synced.  The previous behaviour was inconsistent with
  the syncing behaviour of all other options changes.

Note:
- This isn't particularly well tested.  It's being committed mainly just
  for consideration of the approach, initially.
---
 lib/settings.coffee                       | 68 +++++++++++++++----------------
 tests/unit_tests/settings_test.coffee     | 12 ++----
 tests/unit_tests/test_chrome_stubs.coffee |  6 +--
 3 files changed, 39 insertions(+), 47 deletions(-)

diff --git a/lib/settings.coffee b/lib/settings.coffee
index 842f7618..adbe2bef 100644
--- a/lib/settings.coffee
+++ b/lib/settings.coffee
@@ -6,19 +6,16 @@ Settings =
   onLoadedCallbacks: []
 
   init: ->
-    if Utils.isExtensionPage()
-      # On extension pages, we use localStorage (or a copy of it) as the cache.
-      @cache = if Utils.isBackgroundPage() then localStorage else extend {}, localStorage
-      @onLoaded()
+    chrome.storage.local.get null, (localItems) =>
+      localItems = {} if chrome.runtime.lastError
+      @storage.get null, (syncedItems) =>
+        unless chrome.runtime.lastError
+          @handleUpdateFromChromeStorage key, value for own key, value of extend localItems, syncedItems
 
-    @storage.get null, (items) =>
-      unless chrome.runtime.lastError
-        @handleUpdateFromChromeStorage key, value for own key, value of items
+        chrome.storage.onChanged.addListener (changes, area) =>
+          @propagateChangesFromChromeStorage changes if area == "sync"
 
-      chrome.storage.onChanged.addListener (changes, area) =>
-        @propagateChangesFromChromeStorage changes if area == "sync"
-
-      @onLoaded()
+        @onLoaded()
 
   # Called after @cache has been initialized.  On extension pages, this will be called twice, but that does
   # not matter because it's idempotent.
@@ -37,38 +34,25 @@ Settings =
     # false, 0 or "") are truthy here.  Only null is falsy.
     if @shouldSyncKey key
       unless value and key of @cache and @cache[key] == value
-        defaultValue = @defaults[key]
-        defaultValueJSON = JSON.stringify defaultValue
-
-        if value and value != defaultValueJSON
-          # Key/value has been changed to a non-default value.
-          @cache[key] = value
-          @performPostUpdateHook key, JSON.parse value
-        else
-          # The key has been reset to its default value.
-          delete @cache[key] if key of @cache
-          @performPostUpdateHook key, defaultValue
+        value ?= JSON.stringify @defaults[key]
+        @set key, JSON.parse(value), false
 
   get: (key) ->
     console.log "WARNING: Settings have not loaded yet; using the default value for #{key}." unless @isLoaded
     if key of @cache and @cache[key]? then JSON.parse @cache[key] else @defaults[key]
 
-  set: (key, value) ->
-    # Don't store the value if it is equal to the default, so we can change the defaults in the future.
-    if JSON.stringify(value) == JSON.stringify @defaults[key]
-      @clear key
-    else
-      jsonValue = JSON.stringify value
-      @cache[key] = jsonValue
-      if @shouldSyncKey key
-        setting = {}; setting[key] = jsonValue
+  set: (key, value, shouldSetInSyncedStorage = true) ->
+    @cache[key] = JSON.stringify value
+    if @shouldSyncKey key
+      if shouldSetInSyncedStorage
+        setting = {}; setting[key] = @cache[key]
         @storage.set setting
-      @performPostUpdateHook key, value
+      # Remove settings installed by the "copyNonDefaultsToChromeStorage-20150717" migration; see below.
+      chrome.storage.local.remove key if Utils.isBackgroundPage()
+    @performPostUpdateHook key, value
 
   clear: (key) ->
-    delete @cache[key] if @has key
-    @storage.remove key if @shouldSyncKey key
-    @performPostUpdateHook key, @get key
+    @set key, @defaults[key]
 
   has: (key) -> key of @cache
 
@@ -169,5 +153,19 @@ if Utils.isBackgroundPage()
       rawQuery = Settings.get "findModeRawQuery"
       chrome.storage.local.set findModeRawQueryList: (if rawQuery then [ rawQuery ] else [])
 
+  # Migration (after 1.51, 2015/6/17).
+  # Copy settings with non-default values (and which are not in synced storage) to chrome.storage.local;
+  # thereby making these settings accessible within content scripts.
+  do (migrationKey = "copyNonDefaultsToChromeStorage-20150717") ->
+    unless localStorage[migrationKey]
+      chrome.storage.sync.get null, (items) ->
+        unless chrome.runtime.lastError
+          updates = {}
+          for own key of localStorage
+            if Settings.shouldSyncKey(key) and not items[key]
+              updates[key] = localStorage[key]
+          chrome.storage.local.set updates, ->
+            localStorage[migrationKey] = not chrome.runtime.lastError
+
 root = exports ? window
 root.Settings = Settings
diff --git a/tests/unit_tests/settings_test.coffee b/tests/unit_tests/settings_test.coffee
index 08145190..6270ae3e 100644
--- a/tests/unit_tests/settings_test.coffee
+++ b/tests/unit_tests/settings_test.coffee
@@ -27,12 +27,6 @@ context "settings",
     Settings.set 'scrollStepSize', 20
     assert.equal Settings.get('scrollStepSize'), 20
 
-  should "not store values equal to the default", ->
-    Settings.set 'scrollStepSize', 20
-    assert.isTrue Settings.has 'scrollStepSize'
-    Settings.set 'scrollStepSize', 60
-    assert.isFalse Settings.has 'scrollStepSize'
-
   should "revert to defaults if no key is stored", ->
     Settings.set 'scrollStepSize', 20
     Settings.clear 'scrollStepSize'
@@ -55,7 +49,7 @@ context "synced settings",
     Settings.set 'scrollStepSize', 20
     assert.equal Settings.get('scrollStepSize'), 20
     Settings.propagateChangesFromChromeStorage { scrollStepSize: { newValue: "60" } }
-    assert.isFalse Settings.has 'scrollStepSize'
+    assert.equal Settings.get('scrollStepSize'), 60
 
   should "propagate non-default values from synced storage", ->
     chrome.storage.sync.set { scrollStepSize: JSON.stringify(20) }
@@ -64,12 +58,12 @@ context "synced settings",
   should "propagate default values from synced storage", ->
     Settings.set 'scrollStepSize', 20
     chrome.storage.sync.set { scrollStepSize: JSON.stringify(60) }
-    assert.isFalse Settings.has 'scrollStepSize'
+    assert.equal Settings.get('scrollStepSize'), 60
 
   should "clear a setting from synced storage", ->
     Settings.set 'scrollStepSize', 20
     chrome.storage.sync.remove 'scrollStepSize'
-    assert.isFalse Settings.has 'scrollStepSize'
+    assert.equal Settings.get('scrollStepSize'), 60
 
   should "trigger a postUpdateHook", ->
     message = "Hello World"
diff --git a/tests/unit_tests/test_chrome_stubs.coffee b/tests/unit_tests/test_chrome_stubs.coffee
index fe2fc298..c6a56521 100644
--- a/tests/unit_tests/test_chrome_stubs.coffee
+++ b/tests/unit_tests/test_chrome_stubs.coffee
@@ -57,9 +57,9 @@ exports.chrome =
   storage:
     # chrome.storage.local
     local:
-      get: ->
-      set: ->
-      remove: ->
+      get: (_, callback) -> callback?()
+      set: (_, callback) -> callback?()
+      remove: (_, callback) -> callback?()
 
     # chrome.storage.onChanged
     onChanged:
-- 
cgit v1.2.3


From 84150641a236b30fe5f9c36a0249dbba54dc6166 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Wed, 17 Jun 2015 07:57:49 +0100
Subject: Reinstate unintentionally deleted code.

---
 lib/settings.coffee | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/lib/settings.coffee b/lib/settings.coffee
index adbe2bef..f0cf30f1 100644
--- a/lib/settings.coffee
+++ b/lib/settings.coffee
@@ -6,6 +6,11 @@ Settings =
   onLoadedCallbacks: []
 
   init: ->
+    if Utils.isExtensionPage()
+      # On extension pages, we use localStorage (or a copy of it) as the cache.
+      @cache = if Utils.isBackgroundPage() then localStorage else extend {}, localStorage
+      @onLoaded()
+
     chrome.storage.local.get null, (localItems) =>
       localItems = {} if chrome.runtime.lastError
       @storage.get null, (syncedItems) =>
-- 
cgit v1.2.3


From 46e19df644b4b7e4040f1d8cee33116d9b900f0d Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 20 Jun 2015 07:16:01 +0100
Subject: Document options/settings data model.

---
 lib/settings.coffee | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/lib/settings.coffee b/lib/settings.coffee
index f0cf30f1..ca6cef89 100644
--- a/lib/settings.coffee
+++ b/lib/settings.coffee
@@ -1,4 +1,15 @@
 
+# A "setting" is a stored key/value pair.  An "option" is setting which has a default value and whose value
+# can be changed on the options page.
+#
+# Option values which have never been changed by the user are in Settings.defaults.
+#
+# Settings whose values have been changed are:
+# 1. stored either in chrome.storage.sync or in chrome.storage.local (but never both), and
+# 2. cached in Settings.cache; on extension pages, Settings.cache uses localStorage (so it persists).
+#
+# In all cases except Settings.defaults, values are stored as jsonified strings.
+
 Settings =
   storage: chrome.storage.sync
   cache: {}
@@ -159,7 +170,7 @@ if Utils.isBackgroundPage()
       chrome.storage.local.set findModeRawQueryList: (if rawQuery then [ rawQuery ] else [])
 
   # Migration (after 1.51, 2015/6/17).
-  # Copy settings with non-default values (and which are not in synced storage) to chrome.storage.local;
+  # Copy options with non-default values (and which are not in synced storage) to chrome.storage.local;
   # thereby making these settings accessible within content scripts.
   do (migrationKey = "copyNonDefaultsToChromeStorage-20150717") ->
     unless localStorage[migrationKey]
-- 
cgit v1.2.3


From de770c026a2e023f7be31f2ef1fa5c874578fe29 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 20 Jun 2015 07:27:11 +0100
Subject: Document options/settings data model.

---
 lib/settings.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/settings.coffee b/lib/settings.coffee
index ca6cef89..5955e8ae 100644
--- a/lib/settings.coffee
+++ b/lib/settings.coffee
@@ -1,5 +1,5 @@
 
-# A "setting" is a stored key/value pair.  An "option" is setting which has a default value and whose value
+# A "setting" is a stored key/value pair.  An "option" is a setting which has a default value and whose value
 # can be changed on the options page.
 #
 # Option values which have never been changed by the user are in Settings.defaults.
-- 
cgit v1.2.3


From aa08e16e6613d5a3760761ab89557df41a50f784 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 20 Jun 2015 07:47:09 +0100
Subject: Adding debugging infrastructure for settings.

---
 lib/settings.coffee | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/lib/settings.coffee b/lib/settings.coffee
index 5955e8ae..437e4d45 100644
--- a/lib/settings.coffee
+++ b/lib/settings.coffee
@@ -11,6 +11,7 @@
 # In all cases except Settings.defaults, values are stored as jsonified strings.
 
 Settings =
+  debug: false
   storage: chrome.storage.sync
   cache: {}
   isLoaded: false
@@ -36,6 +37,7 @@ Settings =
   # Called after @cache has been initialized.  On extension pages, this will be called twice, but that does
   # not matter because it's idempotent.
   onLoaded: ->
+    @log "onLoaded: #{@onLoadedCallbacks.length} callback(s)"
     @isLoaded = true
     callback() while callback = @onLoadedCallbacks.pop()
 
@@ -46,6 +48,7 @@ Settings =
     @handleUpdateFromChromeStorage key, change?.newValue for own key, change of changes
 
   handleUpdateFromChromeStorage: (key, value) ->
+    @log "handleUpdateFromChromeStorage: #{key}"
     # Note: value here is either null or a JSONified string.  Therefore, even falsy settings values (like
     # false, 0 or "") are truthy here.  Only null is falsy.
     if @shouldSyncKey key
@@ -59,20 +62,26 @@ Settings =
 
   set: (key, value, shouldSetInSyncedStorage = true) ->
     @cache[key] = JSON.stringify value
+    @log "set: #{key} (length=#{@cache[key].length}, shouldSetInSyncedStorage=#{shouldSetInSyncedStorage})"
     if @shouldSyncKey key
       if shouldSetInSyncedStorage
         setting = {}; setting[key] = @cache[key]
+        @log "   chrome.storage.sync.set(#{key})"
         @storage.set setting
-      # Remove settings installed by the "copyNonDefaultsToChromeStorage-20150717" migration; see below.
-      chrome.storage.local.remove key if Utils.isBackgroundPage()
+      if Utils.isBackgroundPage()
+        # Remove options installed by the "copyNonDefaultsToChromeStorage-20150717" migration; see below.
+        @log "   chrome.storage.local.remove(#{key})"
+        chrome.storage.local.remove key
     @performPostUpdateHook key, value
 
   clear: (key) ->
+    @log "clear: #{key}"
     @set key, @defaults[key]
 
   has: (key) -> key of @cache
 
   use: (key, callback) ->
+    @log "use: #{key} (isLoaded=#{@isLoaded})"
     invokeCallback = => callback @get key
     if @isLoaded then invokeCallback() else @onLoadedCallbacks.push invokeCallback
 
@@ -80,6 +89,10 @@ Settings =
   postUpdateHooks: {}
   performPostUpdateHook: (key, value) -> @postUpdateHooks[key]? value
 
+  # For development only.
+  log: (args...) ->
+    console.log "settings:", args... if @debug
+
   # Default values for all settings.
   defaults:
     scrollStepSize: 60
-- 
cgit v1.2.3