Refactor (and fix) key-parsing regexp.

Fixes #2299.

Also, separate this regexp out into its constituent parts, because it is
becoming too different to read.

2 files changed, 27 insertions, 13 deletions

diff --git a/background_scripts/commands.coffee b/background_scripts/commands.coffee
index 531d491f..674a1e22 100644
--- a/background_scripts/commands.coffee
+++ b/background_scripts/commands.coffee
@@ -41,18 +41,23 @@ Commands =
   # them you have to press "shift" as well.
   # We sort modifiers here to match the order used in keyboard_utils.coffee.
   # The return value is a sequence of keys: e.g. "<Space><c-A>b" -> ["<space>", "<c-A>", "b"].
-  parseKeySequence: (key) ->
-    if key.length == 0
-      []
-    # Parse "<c-a>bcd" as "<c-a>" and "bcd".
-    else if 0 == key.search /^<((?:[acm]-)*(?:.|[a-zA-Z0-9]{2,}))>(.*)/i
-      [modifiers..., keyChar] = RegExp.$1.split "-"
-      keyChar = keyChar.toLowerCase() unless keyChar.length == 1
-      modifiers = (modifier.toLowerCase() for modifier in modifiers)
-      modifiers.sort()
-      ["<#{[modifiers..., keyChar].join '-'}>", @parseKeySequence(RegExp.$2)...]
-    else
-      [key[0], @parseKeySequence(key[1..])...]
+  parseKeySequence: do ->
+    modifier = "(?:[acm]-)"                             # E.g. "a-", "c-", "m-".
+    namedKey = "(?:[a-z][a-z0-9]+)"                     # E.g. "left" or "f12" (always two characters or more).
+    modifiedKey = "(?:#{modifier}+(?:.|#{namedKey}))"   # E.g. "c-*" or "c-left".
+    specialKeyRegexp = new RegExp "^<(#{namedKey}|#{modifiedKey})>(.*)", "i"
+    (key) ->
+      if key.length == 0
+        []
+      # Parse "<c-a>bcd" as "<c-a>" and "bcd".
+      else if 0 == key.search specialKeyRegexp
+        [modifiers..., keyChar] = RegExp.$1.split "-"
+        keyChar = keyChar.toLowerCase() unless keyChar.length == 1
+        modifiers = (modifier.toLowerCase() for modifier in modifiers)
+        modifiers.sort()
+        ["<#{[modifiers..., keyChar].join '-'}>", @parseKeySequence(RegExp.$2)...]
+      else
+        [key[0], @parseKeySequence(key[1..])...]
 
   parseCustomKeyMappings: (customKeyMappings) ->
     for line in customKeyMappings.split "\n"
diff --git a/tests/unit_tests/commands_test.coffee b/tests/unit_tests/commands_test.coffee
index 508e298b..2c2e9542 100644
--- a/tests/unit_tests/commands_test.coffee
+++ b/tests/unit_tests/commands_test.coffee
@@ -15,12 +15,19 @@ context "Key mappings",
     @testKeySequence "A", "A", 1
     @testKeySequence "ab", "a/b", 2
 
+  should "recognise non-alphabetic keys", ->
+    @testKeySequence "#", "#", 1
+    @testKeySequence ".", ".", 1
+    @testKeySequence "##", "#/#", 2
+    @testKeySequence "..", "./.", 2
+
   should "parse keys with modifiers", ->
     @testKeySequence "<c-a>", "<c-a>", 1
     @testKeySequence "<c-A>", "<c-A>", 1
     @testKeySequence "<C-A>", "<c-A>", 1
     @testKeySequence "<c-a><a-b>", "<c-a>/<a-b>", 2
     @testKeySequence "<m-a>", "<m-a>", 1
+    @testKeySequence "z<m-a>", "z/<m-a>", 2
 
   should "normalize with modifiers", ->
     # Modifiers should be in alphabetical order.
@@ -33,7 +40,7 @@ context "Key mappings",
     @testKeySequence "<f12>", "<f12>", 1
     @testKeySequence "<F12>", "<f12>", 1
 
-  should "handle angle brackets", ->
+  should "handle angle brackets which are part of not modifiers", ->
     @testKeySequence "<", "<", 1
     @testKeySequence ">", ">", 1
 
@@ -46,6 +53,8 @@ context "Key mappings",
     @testKeySequence "<<space>", "</<space>", 2
     @testKeySequence "<C->>", "<c->>", 1
 
+    @testKeySequence "<a>", "</a/>", 3
+
   should "negative tests", ->
     # These should not be parsed as modifiers.
     @testKeySequence "<b-a>", "</b/-/a/>", 5