simplify API to avoid (and raise on) user errors

This fixes our API to match the documentation in making
Msgthr#order! optional.  Furthermore, the block previously
passed to Msgthr#order! may now be passed to Msgthr#thread!
instead.

We accomplish this by tracking internal state explicitly, so a
Msgthr::StateError exception will be raised when methods are
called in an unsupported order.  This internal state is reset
with Msgthr#clear.

For users who truly do not care about ordering, Msgthr#walk_thread
may be called immediately after the last call to Msgthr#add.

Thanks to Dimid Duchovny for the feedback which led to this:
https://80x24.org/msgthr-public/CANKvuDc2mkxLuh+3+WXWfMXzxK2bShNesrD5xLocGOD1RybbwQ@mail.gmail.com/

1 files changed, 63 insertions, 1 deletions

diff --git a/test/test_msgthr.rb b/test/test_msgthr.rb
index 19cec75..b14e135 100644
--- a/test/test_msgthr.rb
+++ b/test/test_msgthr.rb
@@ -11,8 +11,9 @@ class TestMsgthr < Test::Unit::TestCase
     thr.add('c', nil, 'c')
     thr.add('D', nil, 'D')
     thr.add('d', %w(missing), 'd')
-    thr.thread!
+    rset = thr.thread!
     rootset = thr.order! { |c| c.sort_by!(&:mid) }
+    assert_same rset, rootset
     assert_equal %w(D c missing), rootset.map(&:mid)
     assert_equal 'D', rootset[0].msg
     assert_equal %w(b), rootset[1].children.map(&:mid)
@@ -33,4 +34,65 @@ class TestMsgthr < Test::Unit::TestCase
 EOF
     assert_equal exp, out
   end
+
+  def test_order_in_thread
+    thr = Msgthr.new
+    thr.add(1, nil, 'a')
+    thr.add(2, [1], 'b')
+    thr.thread! do |ary|
+      ary.sort_by! do |cont|
+        cur = cont.topmost
+        cur ? cur : 0
+      end
+    end
+    out = ''
+    thr.walk_thread do |level, container, index|
+      msg = container.msg
+      out << "#{level} [#{index}] #{msg}\n"
+    end
+    exp = <<EOF.b
+0 [0] a
+1 [0] b
+EOF
+    assert_equal exp, out
+  end
+
+  def test_out_of_order
+    thr = Msgthr.new
+    thr.thread!
+    assert_raise(Msgthr::StateError) { thr.add(1, nil, 'a') }
+    thr.clear # make things good again, following should not raise:
+    thr.add(1, nil, 'a')
+    thr.thread!
+    assert_raise(Msgthr::StateError) { thr.thread! }
+
+    out = []
+    thr.walk_thread do |level, container, index|
+      msg = container.msg
+      out << "#{level} [#{index}] #{msg}"
+    end
+    assert_equal [ '0 [0] a' ], out
+    assert_raise(Msgthr::StateError) { thr.thread! { raise "DO NOT CALL" } }
+    assert_raise(Msgthr::StateError) { thr.order! { |_| raise "DO NOT CALL" } }
+
+    # this is legal, even if non-sensical
+    thr.clear
+    thr.walk_thread { |level, container, index| raise "DO NOT CALL" }
+  end
+
+  def test_short_add_to_walk
+    thr = Msgthr.new
+    thr.add(1, nil, 'a')
+    thr.add(2, [1], 'b')
+    out = ''
+    thr.walk_thread do |level, container, index|
+      msg = container.msg
+      out << "#{level} [#{index}] #{msg}\n"
+    end
+    exp = <<EOF.b
+0 [0] a
+1 [0] b
+EOF
+    assert_equal exp, out
+  end
 end