Merge pull request #14 from github/non-breaking-space

Support incomplete items with non-breaking spaces

Author: mtodd

app/assets/javascripts/task_lists.coffee

@@ -101,15 +101,28 @@
 incomplete = "[ ]"
 complete   = "[x]"
 
+# Escapes the String for regular expression matching.
+escapePattern = (str) ->
+  str.
+    replace(/([\[\]])/g, "\\$1"). # escape square brackets
+    replace(/\s/, "\\s")          # match all white space
+
+incompletePattern = ///
+  #{escapePattern(incomplete)}
+///
+completePattern = ///
+  #{escapePattern(complete)}
+///
+
 # Pattern used to identify all task list items.
 # Useful when you need iterate over all items.
 itemPattern = ///
   ^
   (?:\s*[-+*]|(?:\d+\.))? # optional list prefix
   \s*                     # optional whitespace prefix
   (                       # checkbox
-    #{complete.  replace(/([\[\]])/g, "\\$1")}|
-    #{incomplete.replace(/([\[\]])/g, "\\$1")}
+    #{escapePattern(complete)}|
+    #{escapePattern(incomplete)}
   )
   (?=\s)                  # followed by whitespace
 ///
@@ -131,8 +144,8 @@ codeFencesPattern = ///
 itemsInParasPattern = ///
   ^
   (
-    #{complete.  replace(/[\[\]]/g, "\\$1")}|
-    #{incomplete.replace(/[\[\]]/g, "\\$1")}
+    #{escapePattern(complete)}|
+    #{escapePattern(incomplete)}
   )
   .+
   $
@@ -152,9 +165,9 @@ updateTaskListItem = (source, itemIndex, checked) ->
       if index == itemIndex
         line =
           if checked
-            line.replace(incomplete, complete)
+            line.replace(incompletePattern, complete)
           else
-            line.replace(complete, incomplete)
+            line.replace(completePattern, incomplete)
     line
   result.join("\n")
 

lib/task_list/filter.rb

@@ -31,8 +31,11 @@ def self.filter(*args)
   #   :task_list_items - An array of TaskList::Item objects.
   class Filter < HTML::Pipeline::Filter
 
-    Incomplete  = "[ ]".freeze
-    Complete    = "[x]".freeze
+    Incomplete = "[ ]".freeze
+    Complete   = "[x]".freeze
+
+    IncompletePattern = /\[[[:space:]]\]/.freeze # matches all whitespace
+    CompletePattern   = Regexp.escape(Complete).freeze
 
     # Pattern used to identify all task list items.
     # Useful when you need iterate over all items.
@@ -41,8 +44,8 @@ class Filter < HTML::Pipeline::Filter
       (?:\s*[-+*]|(?:\d+\.))? # optional list prefix
       \s*                     # optional whitespace prefix
       (                       # checkbox
-        #{Regexp.escape(Complete)}|
-        #{Regexp.escape(Incomplete)}
+        #{CompletePattern}|
+        #{IncompletePattern}
       )
       (?=\s)                  # followed by whitespace
     /x

test/task_list/filter_test.rb

@@ -105,6 +105,26 @@ def test_handles_complicated_nested_items
     assert_equal 2, filter(text)[:output].css('.task-list-item .task-list-item .task-list-item').size
   end
 
+  # NOTE: This is an edge case experienced regularly by users using a Swiss
+  # German keyboard.
+  # See: https://github.com/github/github/pull/18362
+  def test_non_breaking_space_between_brackets
+    text = "- [\xC2\xA0] ok"
+    assert item = filter(text)[:output].css('.task-list-item').pop, "item expected"
+    assert_equal 'ok', item.text.strip
+  end
+
+  # See: https://github.com/github/github/pull/18362
+  def test_non_breaking_space_between_brackets_in_paras
+    text = <<-md
+- [\xC2\xA0] one
+- [\xC2\xA0] this one will be wrapped in a para
+
+- [\xC2\xA0] this one too, wtf
+    md
+    assert_equal 3, filter(text)[:output].css(@item_selector).size
+  end
+
   protected
 
   def filter(input, context = @context, result = nil)

test/unit/test_updates.coffee

@@ -20,19 +20,36 @@ module "TaskList updates",
       disabled: true
       checked: false
 
+    # non-breaking space. See: https://github.com/github/task-lists/pull/14
+    @nbsp = String.fromCharCode(160)
+    @incompleteNBSPItem = $ '<li>', class: 'task-list-item'
+    @incompleteNBSPCheckbox = $ '<input>',
+      type: 'checkbox'
+      class: 'task-list-item-checkbox'
+      disabled: true
+      checked: false
+
     @field = $ '<textarea>', class: 'js-task-list-field', text: """
       - [x] complete
       - [ ] incomplete
+      - [#{@nbsp}] incompleteNBSP
     """
 
     @changes =
       toComplete: """
       - [ ] complete
       - [ ] incomplete
+      - [#{@nbsp}] incompleteNBSP
       """
       toIncomplete: """
       - [x] complete
       - [x] incomplete
+      - [#{@nbsp}] incompleteNBSP
+      """
+      toIncompleteNBSP: """
+      - [x] complete
+      - [ ] incomplete
+      - [x] incompleteNBSP
       """
 
     @completeItem.append @completeCheckbox
@@ -43,6 +60,10 @@ module "TaskList updates",
     @list.append @incompleteItem
     @incompleteItem.expectedIndex = 2
 
+    @incompleteNBSPItem.append @incompleteNBSPCheckbox
+    @list.append @incompleteNBSPItem
+    @incompleteNBSPItem.expectedIndex = 3
+
     @container.append @list
     @container.append @field
 
@@ -79,3 +100,18 @@ asyncTest "updates the source, marking the complete item as incomplete", ->
   , 20
 
   @completeCheckbox.click()
+
+# See: https://github.com/github/task-lists/pull/14
+asyncTest "updates the source for items with non-breaking spaces", ->
+  expect 3
+
+  @field.on 'tasklist:changed', (event, index, checked) =>
+    ok checked
+    equal index, @incompleteNBSPItem.expectedIndex
+    equal @field.val(), @changes.toIncompleteNBSP
+
+  setTimeout ->
+    start()
+  , 20
+
+  @incompleteNBSPCheckbox.click()