From d06d8c221be5b82d00da821323fb6d1889e58105 Mon Sep 17 00:00:00 2001 From: Eric Wong Date: Thu, 28 Dec 2017 01:04:01 +0000 Subject: 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/ --- lib/msgthr.rb | 59 +++++++++++++++++++++++++++++++-------------- lib/msgthr/container.rb | 6 ++--- test/test_msgthr.rb | 64 ++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 107 insertions(+), 22 deletions(-) diff --git a/lib/msgthr.rb b/lib/msgthr.rb index ea63731..1517f28 100644 --- a/lib/msgthr.rb +++ b/lib/msgthr.rb @@ -7,8 +7,7 @@ # # * use Msgthr.new to create a new object # * use Msgthr#add! for every message you have -# * use Msgthr#thread! to perform threading operations -# * optionally, use Msgthr#order! to sort messages +# * use Msgthr#thread! to perform threading and (optionally) sort # * use Msgthr#walk_thread to iterate through the threaded tree # # See https://80x24.org/msgthr/README for more info @@ -20,43 +19,39 @@ class Msgthr # calling Msgthr#thread! attr_reader :rootset + # raised when methods are called in an unsupported order + StateError = Class.new(RuntimeError) + # Initialize a Msgthr object def initialize @id_table = {} @rootset = [] + @state = :init # :init => :threaded => :ordered end # Clear internal data structures to save memory and prepare for reuse def clear @rootset.clear @id_table.clear + @state = :init end # Performs threading on the messages and returns the rootset # (set of message containers without parents). # - # Call this after all #add operations are complete. - # This does not sort, use #order! if sorting is necessary. - def thread! - ret = @rootset - @id_table.each_value { |cont| ret << cont if cont.parent.nil? }.clear - ret - end - - # Performs an in-place sort on messages after thread! - # This is optional and intended to be called this only after #thread! + # Call this only after all #add operations are complete. # - # This takes a block which yields an array of Msgthr::Container - # objects for sorting. + # If given an optional block, it will perform an in-place sort + # using the block parameter. # - # To sort by unique +mid+ identifiers for each container: + # To thread and sort by unique +mid+ identifiers for each container: # - # msgthr.order! { |ary| ary.sort_by!(&:mid) } + # msgthr.thread! { |ary| ary.sort_by!(&:mid) } # # If your opaque message pointer contains a +time+ accessor which gives # a Time object: # - # msgthr.order! do |ary| + # msgthr.thread! do |ary| # ary.sort_by! do |cont| # Msgthr::Container # cur = cont.topmost # cur ? cur.msg.time : Time.at(0) @@ -67,16 +62,40 @@ class Msgthr # Msgthr::Container#mid, as any known missing messages (ghosts) # will still have a +mid+. However, Msgthr::Container#topmost is # necessary if accessing Msgthr::Container#msg. + def thread! + raise StateError, "already #@state" if @state != :init + ret = @rootset + @id_table.each_value { |cont| ret << cont if cont.parent.nil? }.clear + @state = :threaded + order! { |ary| yield(ary) } if block_given? + ret + end + + # Calling this method is unnecessary since msgthr 1.1.0. + # In previous releases, the #thread! did not support a block + # parameter for ordering. This method remains for compatibility. def order! + case @state + when :init then raise StateError, "#thread! not called" + when :ordered then raise StateError, "already #@state" + # else @state == :threaded + end + yield @rootset @rootset.each do |cont| # this calls Msgthr::Container#order!, which is non-recursive cont.order! { |children| yield(children) } end + @state = :ordered + @rootset end # non-recursively walk a set of messages after #thread! - # (and optionally, #order!) + # (and optionally, #order!). + # + # If you do not care about ordering, you may call this + # immediately after all #add operations are complete starting + # with msgthr 1.1.0 # # This takes a block and yields 3 elements to it: +|level, container, index|+ # for each message container. @@ -95,6 +114,8 @@ class Msgthr # printf("#{indent} % 3d. %s\n", index, subject) # end def walk_thread + thread! if @state == :init + order! { |_| } if @state == :threaded i = -1 q = @rootset.map { |cont| [ 0, cont, i += 1 ] } while tmp = q.shift @@ -128,6 +149,8 @@ class Msgthr # calling this method to avoid wasting memory on hash keys. Likewise # is true for any String objects in +refs+. def add(mid, refs, msg) + @state == :init or raise StateError, "cannot add when already #@state" + cur = @id_table[mid] ||= Msgthr::Container.new(mid) cur.msg = msg refs or return diff --git a/lib/msgthr/container.rb b/lib/msgthr/container.rb index e51507c..fbff719 100644 --- a/lib/msgthr/container.rb +++ b/lib/msgthr/container.rb @@ -1,9 +1,9 @@ # Copyright (C) 2016 all contributors # License: GPL-2.0+ -# An internal container class, this is exposed for Msgthr#order! -# and Msgthr#walk_thread APIs. They should should not be initialized -# in your own code. +# An internal container class, this is exposed for Msgthr#thread! +# Msgthr#order! and Msgthr#walk_thread APIs through block parameters. +# They should should not be initialized in your own code. # # One container object will exist for every message you call Msgthr#add! on, # so there can potentially be many of these objects for large sets of 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) @@ -30,6 +31,67 @@ class TestMsgthr < Test::Unit::TestCase 0. abc 2. [missing: ] 0. d +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 = <