* Re: library usage
2017-12-28 1:43 7% ` Eric Wong
@ 2017-12-31 13:06 0% ` Dimid Duchovny
0 siblings, 0 replies; 2+ results
From: Dimid Duchovny @ 2017-12-31 13:06 UTC (permalink / raw)
To: Eric Wong; +Cc: msgthr-public
2017-12-28 3:43 GMT+02:00 Eric Wong <e@80x24.org>:
> Dimid Duchovny <dimidd@gmail.com> wrote:
>> Thank you for the quick reply. IMHO, the best option would be indeed
>> to add an optional proc to thread! . If nil, it would be equivalent to
>> the noop order!.
>
> Thanks for the feedback, I've pushed the following out and will
> likely release 1.1.0 in a day or two (in case you notice any
> other things wrong).
>
> I was planning on writing something else using this, but it's on
> the backburner for now; so you might be the only user at the
> moment :)
>
Thank you very much Eric!
As a matter of fact, I did find an issue, which I'm not sure is a bug;
consider this line from the `add` method:
cur = @id_table[mid] ||= Msgthr::Container.new(mid)
If `mid` is the empty string or a string which only contains
whitespace, it's considered a valid key for `id_table`.
However, I'd expect an empty string and (arguably) whitespace-only
strings to be equivalent to nil.
Happy new year!
> https://80x24.org/msgthr.git/patch?id=d06d8c221be5b82d00da821323fb6d1889e58105
> -------------8<------------
> 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 <msgthr-public@80x24.org>
> # License: GPL-2.0+ <https://www.gnu.org/licenses/gpl-2.0.txt>
>
> -# 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)
> @@ -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
> --
> EW
^ permalink raw reply [relevance 0%]
* Re: library usage
@ 2017-12-28 1:43 7% ` Eric Wong
2017-12-31 13:06 0% ` Dimid Duchovny
0 siblings, 1 reply; 2+ results
From: Eric Wong @ 2017-12-28 1:43 UTC (permalink / raw)
To: Dimid Duchovny; +Cc: msgthr-public
Dimid Duchovny <dimidd@gmail.com> wrote:
> Thank you for the quick reply. IMHO, the best option would be indeed
> to add an optional proc to thread! . If nil, it would be equivalent to
> the noop order!.
Thanks for the feedback, I've pushed the following out and will
likely release 1.1.0 in a day or two (in case you notice any
other things wrong).
I was planning on writing something else using this, but it's on
the backburner for now; so you might be the only user at the
moment :)
https://80x24.org/msgthr.git/patch?id=d06d8c221be5b82d00da821323fb6d1889e58105
-------------8<------------
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 <msgthr-public@80x24.org>
# License: GPL-2.0+ <https://www.gnu.org/licenses/gpl-2.0.txt>
-# 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)
@@ -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
--
EW
^ permalink raw reply related [relevance 7%]
Results 1-2 of 2 | reverse | options above
-- pct% links below jump to the message on this page, permalinks otherwise --
2017-12-27 12:22 library usage Dimid Duchovny
2017-12-27 18:01 ` Eric Wong
2017-12-27 19:06 ` Dimid Duchovny
2017-12-28 1:43 7% ` Eric Wong
2017-12-31 13:06 0% ` Dimid Duchovny
Code repositories for project(s) associated with this public inbox
https://80x24.org/msgthr.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).