From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: X-Spam-Status: No, score=-4.0 required=3.0 tests=ALL_TRUSTED,BAYES_00 shortcircuit=no autolearn=ham autolearn_force=no version=3.4.0 Received: from localhost (dcvr.yhbt.net [127.0.0.1]) by dcvr.yhbt.net (Postfix) with ESMTP id D77952079F for ; Wed, 21 Dec 2016 07:36:08 +0000 (UTC) From: Eric Wong To: meta@public-inbox.org Subject: [PATCH 2/2] searchthread: simplify API and remove needless OO Date: Wed, 21 Dec 2016 07:36:07 +0000 Message-Id: <20161221073607.32236-3-e@80x24.org> In-Reply-To: <20161221073607.32236-1-e@80x24.org> References: <20161221073607.32236-1-e@80x24.org> List-Id: This simplifies callers to prevent errors and avoids needless object-orientation in favor of a single procedure call to handle threading and ordering. --- lib/PublicInbox/SearchThread.pm | 37 +++++++++++++------------------------ lib/PublicInbox/SearchView.pm | 25 ++++++++++++------------- lib/PublicInbox/View.pm | 19 ++++++++----------- t/thread-cycle.t | 8 ++++---- 4 files changed, 37 insertions(+), 52 deletions(-) diff --git a/lib/PublicInbox/SearchThread.pm b/lib/PublicInbox/SearchThread.pm index fafe7d7..2cd066d 100644 --- a/lib/PublicInbox/SearchThread.pm +++ b/lib/PublicInbox/SearchThread.pm @@ -21,32 +21,28 @@ package PublicInbox::SearchThread; use strict; use warnings; -sub new { - return bless { - messages => $_[1], - id_table => {}, - rootset => [] - }, $_[0]; -} - sub thread { - my $self = shift; - _add_message($self, $_) foreach @{$self->{messages}}; - my $id_table = delete $self->{id_table}; - $self->{rootset} = [ grep { + my ($messages, $ordersub) = @_; + my $id_table = {}; + _add_message($id_table, $_) foreach @$messages; + my $rootset = [ grep { !delete($_->{parent}) && $_->visible } values %$id_table ]; + $id_table = undef; + $rootset = $ordersub->($rootset); + $_->order_children($ordersub) for @$rootset; + $rootset; } sub _get_cont_for_id ($$) { - my ($self, $mid) = @_; - $self->{id_table}{$mid} ||= PublicInbox::SearchThread::Msg->new($mid); + my ($id_table, $mid) = @_; + $id_table->{$mid} ||= PublicInbox::SearchThread::Msg->new($mid); } sub _add_message ($$) { - my ($self, $smsg) = @_; + my ($id_table, $smsg) = @_; # A. if id_table... - my $this = _get_cont_for_id($self, $smsg->{mid}); + my $this = _get_cont_for_id($id_table, $smsg->{mid}); $this->{smsg} = $smsg; # B. For each element in the message's References field: @@ -59,7 +55,7 @@ sub _add_message ($$) { my $prev; foreach my $ref ($refs =~ m/<([^>]+)>/g) { # Find a Container object for the given Message-ID - my $cont = _get_cont_for_id($self, $ref); + my $cont = _get_cont_for_id($id_table, $ref); # Link the References field's Containers together in # the order implied by the References header @@ -82,13 +78,6 @@ sub _add_message ($$) { $prev->add_child($this) if defined $prev; } -sub order { - my ($self, $ordersub) = @_; - my $rootset = $ordersub->($self->{rootset}); - $self->{rootset} = $rootset; - $_->order_children($ordersub) for @$rootset; -} - package PublicInbox::SearchThread::Msg; use strict; use warnings; diff --git a/lib/PublicInbox/SearchView.pm b/lib/PublicInbox/SearchView.pm index 50a2c01..bd634d8 100644 --- a/lib/PublicInbox/SearchView.pm +++ b/lib/PublicInbox/SearchView.pm @@ -161,6 +161,15 @@ sub search_nav_bot { $rv .= ''; } +sub sort_relevance { + my ($pct) = @_; + sub { + [ sort { (eval { $pct->{$b->topmost->{id}} } || 0) + <=> + (eval { $pct->{$a->topmost->{id}} } || 0) + } @{$_[0]} ] }; +} + sub mset_thread { my ($ctx, $mset, $q) = @_; my %pct; @@ -171,18 +180,8 @@ sub mset_thread { $smsg; } ($mset->items) ]}); - my $th = PublicInbox::SearchThread->new($msgs); - $th->thread; - if ($q->{r}) { # order by relevance - $th->order(sub { - [ sort { (eval { $pct{$b->topmost->{id}} } || 0) - <=> - (eval { $pct{$a->topmost->{id}} } || 0) - } @{$_[0]} ]; - }); - } else { # order by time (default for threaded view) - $th->order(*PublicInbox::View::sort_ts); - } + my $rootset = PublicInbox::SearchThread::thread($msgs, + $q->{r} ? sort_relevance(\%pct) : *PublicInbox::View::sort_ts); my $skel = search_nav_bot($mset, $q). "
";
 	my $inbox = $ctx->{-inbox};
 	$ctx->{-upfx} = '';
@@ -196,7 +195,7 @@ sub mset_thread {
 	$ctx->{seen} = {};
 	$ctx->{s_nr} = scalar(@$msgs).'+ results';
 
-	PublicInbox::View::walk_thread($th, $ctx,
+	PublicInbox::View::walk_thread($rootset, $ctx,
 		*PublicInbox::View::pre_thread);
 
 	my $mime;
diff --git a/lib/PublicInbox/View.pm b/lib/PublicInbox/View.pm
index a50cb64..b779665 100644
--- a/lib/PublicInbox/View.pm
+++ b/lib/PublicInbox/View.pm
@@ -260,8 +260,8 @@ sub _th_index_lite {
 }
 
 sub walk_thread {
-	my ($th, $ctx, $cb) = @_;
-	my @q = map { (0, $_, -1) } @{$th->{rootset}};
+	my ($rootset, $ctx, $cb) = @_;
+	my @q = map { (0, $_, -1) } @$rootset;
 	while (@q) {
 		my ($level, $node, $i) = splice(@q, 0, 3);
 		defined $node or next;
@@ -285,10 +285,10 @@ sub thread_index_entry {
 }
 
 sub stream_thread ($$) {
-	my ($th, $ctx) = @_;
+	my ($rootset, $ctx) = @_;
 	my $inbox = $ctx->{-inbox};
 	my $mime;
-	my @q = map { (0, $_) } @{$th->{rootset}};
+	my @q = map { (0, $_) } @$rootset;
 	my $level;
 	while (@q) {
 		$level = shift @q;
@@ -350,10 +350,10 @@ sub thread_html {
 	$ctx->{mapping} = {};
 	$ctx->{s_nr} = "$nr+ messages in thread";
 
-	my $th = thread_results($msgs);
-	walk_thread($th, $ctx, *pre_thread);
+	my $rootset = thread_results($msgs);
+	walk_thread($rootset, $ctx, *pre_thread);
 	$skel .= '
'; - return stream_thread($th, $ctx) unless $ctx->{flat}; + return stream_thread($rootset, $ctx) unless $ctx->{flat}; # flat display: lazy load the full message from smsg my $inbox = $ctx->{-inbox}; @@ -749,10 +749,7 @@ sub msg_timestamp { sub thread_results { my ($msgs) = @_; require PublicInbox::SearchThread; - my $th = PublicInbox::SearchThread->new($msgs); - $th->thread; - $th->order(*sort_ts); - $th + PublicInbox::SearchThread::thread($msgs, *sort_ts); } sub missing_thread { diff --git a/t/thread-cycle.t b/t/thread-cycle.t index 9dd2aa3..16ceee7 100644 --- a/t/thread-cycle.t +++ b/t/thread-cycle.t @@ -72,11 +72,11 @@ SKIP: { done_testing(); sub thread_to_s { - my $th = PublicInbox::SearchThread->new(shift); - $th->thread; - $th->order(sub { [ sort { $a->{id} cmp $b->{id} } @{$_[0]} ] }); + my ($msgs) = @_; + my $rootset = PublicInbox::SearchThread::thread($msgs, sub { + [ sort { $a->{id} cmp $b->{id} } @{$_[0]} ] }); my $st = ''; - my @q = map { (0, $_) } @{$th->{rootset}}; + my @q = map { (0, $_) } @$rootset; while (@q) { my $level = shift @q; my $node = shift @q or next; -- EW