Merge branch 'master' into non-breaking-space

Conflicts:
	test/task_list/filter_test.rb

Author: mtodd

app/assets/javascripts/task_lists.coffee

@@ -5,7 +5,6 @@
 #= provides tasklist:change
 #= provides tasklist:changed
 #
-#= require crema/events/fire
 #= require crema/events/pageupdate
 #
 # Enables Task List update behavior.
@@ -181,10 +180,13 @@ updateTaskList = ($item) ->
   index      = 1 + $container.find('.task-list-item-checkbox').index($item)
   checked    = $item.prop 'checked'
 
-  $field.fire 'tasklist:change', [index, checked], ->
+  event = $.Event 'tasklist:change'
+  $field.trigger event, [index, checked]
+
+  unless event.isDefaultPrevented()
     $field.val updateTaskListItem($field.val(), index, checked)
     $field.trigger 'change'
-    $field.fire 'tasklist:changed', [index, checked]
+    $field.trigger 'tasklist:changed', [index, checked]
 
 # When the task list item checkbox is updated, submit the change
 $(document).on 'change', '.task-list-item-checkbox', ->

lib/task_list/filter.rb

@@ -50,20 +50,16 @@ class Filter < HTML::Pipeline::Filter
       (?=\s)                  # followed by whitespace
     /x
 
-    ListSelector = [
-      # select UL/OL
-      ".//li[starts-with(text(),'[ ]')]/..",
-      ".//li[starts-with(text(),'[x]')]/..",
-      # and those wrapped in Ps
-      ".//li/p[1][starts-with(text(),'[ ]')]/../..",
-      ".//li/p[1][starts-with(text(),'[x]')]/../.."
-    ].join(' | ').freeze
-
-    # Selects all LIs from a TaskList UL/OL
-    ItemSelector = ".//li".freeze
+    ListItemSelector = ".//li[task_list_item(.)]".freeze
+
+    class XPathSelectorFunction
+      def self.task_list_item(nodes)
+        nodes if nodes.text =~ ItemPattern
+      end
+    end
 
     # Selects first P tag of an LI, if present
-    ItemParaSelector = ".//p[1]".freeze
+    ItemParaSelector = "./p[1]".freeze
 
     # List of `TaskList::Item` objects that were recognized in the document.
     # This is available in the result hash as `:task_list_items`.
@@ -103,20 +99,22 @@ def render_task_list_item(item)
     #
     # Returns an Array of Nokogiri::XML::Element objects for ordered and
     # unordered lists.
-    def task_lists
-      doc.xpath(ListSelector)
+    def list_items
+      doc.xpath(ListItemSelector, XPathSelectorFunction)
     end
 
-    # Public: filters a Nokogiri::XML::Element ordered/unordered list, marking
-    # up the list items in order to add behavior and include metadata.
+    # Filters the source for task list items.
     #
-    # Modifies the provided node.
+    # Each item is wrapped in HTML to identify, style, and layer
+    # useful behavior on top of.
+    #
+    # Modifications apply to the parsed document directly.
     #
     # Returns nothing.
-    def filter_list(node)
-      add_css_class(node, 'task-list')
+    def filter!
+      list_items.reverse.each do |li|
+        add_css_class(li.parent, 'task-list')
 
-      node.xpath(ItemSelector).each do |li|
         outer, inner =
           if p = li.xpath(ItemParaSelector)[0]
             [p, p.inner_html]
@@ -125,28 +123,15 @@ def filter_list(node)
           end
         if match = (inner.chomp =~ ItemPattern && $1)
           item = TaskList::Item.new(match, inner)
-          task_list_items << item
+          # prepend because we're iterating in reverse
+          task_list_items.unshift item
 
           add_css_class(li, 'task-list-item')
           outer.inner_html = render_task_list_item(item)
         end
       end
     end
 
-    # Filters the source for task list items.
-    #
-    # Each item is wrapped in HTML to identify, style, and layer
-    # useful behavior on top of.
-    #
-    # Modifications apply to the parsed document directly.
-    #
-    # Returns nothing.
-    def filter!
-      task_lists.each do |node|
-        filter_list node
-      end
-    end
-
     def call
       filter!
       doc
@@ -156,6 +141,7 @@ def call
     # names.
     def add_css_class(node, *new_class_names)
       class_names = (node['class'] || '').split(' ')
+      return if new_class_names.all? { |klass| class_names.include?(klass) }
       class_names.concat(new_class_names)
       node['class'] = class_names.uniq.join(' ')
     end

lib/task_list/version.rb

@@ -1,3 +1,3 @@
 class TaskList
-  VERSION = [0, 2, 0].join('.')
+  VERSION = [0, 3, 0].join('.')
 end

task-lists.gemspec

@@ -24,4 +24,5 @@ Gem::Specification.new do |gem|
   gem.add_development_dependency "json"
   gem.add_development_dependency "rack"
   gem.add_development_dependency "sprockets"
+  gem.add_development_dependency "minitest", "~> 5.3.2"
 end

test/functional/helpers/remote.coffee

@@ -1,3 +1,3 @@
 #= require jquery
-#= require rails/remote
+#= require rails-behaviors/remote
 #= require task_lists

test/task_list/filter_test.rb

@@ -2,7 +2,7 @@
 require File.expand_path('../../test_helper', __FILE__)
 require 'task_list/filter'
 
-class TaskList::FilterTest < Test::Unit::TestCase
+class TaskList::FilterTest < Minitest::Test
   def setup
     @pipeline = HTML::Pipeline.new [
       HTML::Pipeline::MarkdownFilter,
@@ -78,6 +78,33 @@ def test_handles_encoding_correctly
     assert_equal unicode, item.text.strip
   end
 
+  def test_handles_nested_items
+    text = <<-md
+- [ ] one
+  - [ ] one.one
+    md
+    assert item = filter(text)[:output].css('.task-list-item .task-list-item').pop
+  end
+
+  def test_handles_complicated_nested_items
+    text = <<-md
+- [ ] one
+  - [ ] one.one
+  - [x] one.two
+    - [ ] one.two.one
+    - [ ] one.two.two
+  - [ ] one.three
+  - [ ] one.four
+- [ ] two
+  - [x] two.one
+  - [ ] two.two
+- [ ] three
+    md
+
+    assert_equal 6 + 2, filter(text)[:output].css('.task-list-item .task-list-item').size
+    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

test/task_list/summary_test.rb

@@ -2,7 +2,7 @@
 require File.expand_path('../../test_helper', __FILE__)
 require 'task_list/summary'
 
-class TaskList::SummaryTest < Test::Unit::TestCase
+class TaskList::SummaryTest < Minitest::Test
   def setup
     @complete   = make_item "[x]", "complete"
     @incomplete = make_item "[ ]", "incomplete"

test/task_list_test.rb

@@ -3,7 +3,7 @@
 require 'task_list'
 require 'task_list/filter'
 
-class TaskListTest < Test::Unit::TestCase
+class TaskListTest < Minitest::Test
   class Record < Struct.new(:body)
     def task_list_items
       []

test/test_helper.rb

@@ -1,3 +1,3 @@
 $:.unshift "lib"
-require 'test/unit'
+require 'minitest/autorun'
 require 'task_list'