~sircmpwn/aerc

This thread contains a patchset. You're looking at the original emails, but you may wish to use the patch review UI. Review patch
1

[PATCH threading] Add threading support for maildir

Details
Message ID
<20191013160411.46959-1-ben@benburwell.com>
DKIM signature
pass
Download raw message
Patch: +78 -0
---
This is a (WIP) patch to help with implementing threading for the
maildir backend, to be applied on top of Kevin's patchset as discussed
on IRC.

Kevin, feel free to incorporate this into your patchset in any way that
would be helpful.

 worker/maildir/container.go | 46 +++++++++++++++++++++++++++++++++++++
 worker/maildir/message.go   | 17 ++++++++++++++
 worker/maildir/worker.go    | 15 ++++++++++++
 3 files changed, 78 insertions(+)

diff --git a/worker/maildir/container.go b/worker/maildir/container.go
index 85e892a..a7acfb7 100644
--- a/worker/maildir/container.go
+++ b/worker/maildir/container.go
@@ -10,6 +10,7 @@ import (
 	"github.com/emersion/go-maildir"
 
 	"git.sr.ht/~sircmpwn/aerc/lib/uidstore"
+	"git.sr.ht/~sircmpwn/aerc/worker/types"
 )
 
 // A Container is a directory which contains other directories which adhere to
@@ -123,3 +124,48 @@ func (c *Container) copyMessage(
 	_, err := src.Copy(dest, key)
 	return err
 }
+
+func (c *Container) ScanThreads(d maildir.Dir) ([]*types.Thread, error) {
+	uids, err := c.UIDs(d)
+	if err != nil {
+		return nil, err
+	}
+
+	// a map from message-id to the Thread that represents it
+	threads := make(map[string]*types.Thread)
+
+	for _, uid := range uids {
+		m, err := c.Message(d, uid)
+		if err != nil {
+			return nil, err
+		}
+		hs, err := m.Headers()
+		if err != nil {
+			return nil, err
+		}
+		msgID, err := hs.Text("message-id")
+		if err != nil {
+			return nil, err
+		}
+		irt, err := hs.Text("in-reply-to")
+		if err != nil {
+			return nil, err
+		}
+
+		t := &types.Thread{Uid: uid}
+		threads[msgID] = t
+
+		if p, ok := threads[irt]; ok {
+			p.Children = append(p.Children, t)
+			t.Parent = p
+		}
+	}
+
+	var threadSlice []*types.Thread
+	for _, t := range threads {
+		if t.Parent == nil {
+			threadSlice = append(threadSlice, t)
+		}
+	}
+	return threadSlice, nil
+}
diff --git a/worker/maildir/message.go b/worker/maildir/message.go
index dc5646b..283a3cd 100644
--- a/worker/maildir/message.go
+++ b/worker/maildir/message.go
@@ -1,6 +1,7 @@
 package maildir
 
 import (
+	"bufio"
 	"bytes"
 	"fmt"
 	"io"
@@ -8,6 +9,7 @@ import (
 
 	"github.com/emersion/go-maildir"
 	"github.com/emersion/go-message"
+	"github.com/emersion/go-message/textproto"
 
 	"git.sr.ht/~sircmpwn/aerc/models"
 	"git.sr.ht/~sircmpwn/aerc/worker/lib"
@@ -119,3 +121,18 @@ func translateFlags(maildirFlags []maildir.Flag) []models.Flag {
 func (m Message) UID() uint32 {
 	return m.uid
 }
+
+func (m Message) Headers() (*message.Header, error) {
+	f, err := m.dir.Open(m.key)
+	if err != nil {
+		return nil, err
+	}
+	defer f.Close()
+	br := bufio.NewReader(f)
+	h, err := textproto.ReadHeader(br)
+	if err != nil {
+		return nil, err
+	}
+	mh := message.Header{h}
+	return &mh, nil
+}
diff --git a/worker/maildir/worker.go b/worker/maildir/worker.go
index 1df4e09..7bd7cbd 100644
--- a/worker/maildir/worker.go
+++ b/worker/maildir/worker.go
@@ -170,6 +170,8 @@ func (w *Worker) handleMessage(msg types.WorkerMessage) error {
 		return w.handleOpenDirectory(msg)
 	case *types.FetchDirectoryContents:
 		return w.handleFetchDirectoryContents(msg)
+	case *types.FetchDirectoryThreaded:
+		return w.handleDirectoryThreaded(msg)
 	case *types.CreateDirectory:
 		return w.handleCreateDirectory(msg)
 	case *types.FetchMessageHeaders:
@@ -291,6 +293,19 @@ func (w *Worker) handleFetchDirectoryContents(
 	return nil
 }
 
+func (w *Worker) handleDirectoryThreaded(msg *types.FetchDirectoryThreaded) error {
+	threads, err := w.c.ScanThreads(*w.selected)
+	if err != nil {
+		w.worker.Logger.Printf("error scanning threads: %v", err)
+		return err
+	}
+	w.worker.PostMessage(&types.DirectoryThreaded{
+		Message: types.RespondTo(msg),
+		Threads: threads,
+	}, nil)
+	return nil
+}
+
 func (w *Worker) sort(uids []uint32, criteria []*types.SortCriterion) ([]uint32, error) {
 	if len(criteria) == 0 {
 		return uids, nil
-- 
2.23.0
Details
Message ID
<BXORXI6F23HG.3HCM21ZIT01XS@magneto>
In-Reply-To
<20191013160411.46959-1-ben@benburwell.com> (view parent)
DKIM signature
missing
Download raw message
On Sun Oct 13, 2019 at 12:04 PM Ben Burwell wrote:
> ---
> This is a (WIP) patch to help with implementing threading for the
> maildir backend, to be applied on top of Kevin's patchset as discussed
> on IRC.
>
> Kevin, feel free to incorporate this into your patchset in any way that
> would be helpful.
>
>  worker/maildir/container.go | 46 +++++++++++++++++++++++++++++++++++++
>  worker/maildir/message.go   | 17 ++++++++++++++
>  worker/maildir/worker.go    | 15 ++++++++++++
>  3 files changed, 78 insertions(+)
>
> diff --git a/worker/maildir/container.go b/worker/maildir/container.go
> index 85e892a..a7acfb7 100644
> --- a/worker/maildir/container.go
> +++ b/worker/maildir/container.go
> @@ -10,6 +10,7 @@ import (
>  	"github.com/emersion/go-maildir"
>
>  	"git.sr.ht/~sircmpwn/aerc/lib/uidstore"
> +	"git.sr.ht/~sircmpwn/aerc/worker/types"
>  )
>
>  // A Container is a directory which contains other directories which adhere to
> @@ -123,3 +124,48 @@ func (c *Container) copyMessage(
>  	_, err := src.Copy(dest, key)
>  	return err
>  }
> +
> +func (c *Container) ScanThreads(d maildir.Dir) ([]*types.Thread, error) {
> +	uids, err := c.UIDs(d)
> +	if err != nil {
> +		return nil, err
> +	}
> +
> +	// a map from message-id to the Thread that represents it
> +	threads := make(map[string]*types.Thread)
> +
> +	for _, uid := range uids {
> +		m, err := c.Message(d, uid)
> +		if err != nil {
> +			return nil, err
> +		}
> +		hs, err := m.Headers()
> +		if err != nil {
> +			return nil, err
> +		}
> +		msgID, err := hs.Text("message-id")
> +		if err != nil {
> +			return nil, err
> +		}
> +		irt, err := hs.Text("in-reply-to")
> +		if err != nil {
> +			return nil, err
> +		}
> +
> +		t := &types.Thread{Uid: uid}
> +		threads[msgID] = t
> +
> +		if p, ok := threads[irt]; ok {
> +			p.Children = append(p.Children, t)
> +			t.Parent = p
> +		}
> +	}
> +
> +	var threadSlice []*types.Thread
> +	for _, t := range threads {
> +		if t.Parent == nil {
> +			threadSlice = append(threadSlice, t)
> +		}
> +	}
> +	return threadSlice, nil
> +}

So this code is pretty solid, although we would probably want to flesh
out the references algorithm in RFC 5256 a bit more. Right now, this is
only looking at the "In-Reply-To" header, and ignoring the "References"
header.

One thing I am trying to explore right now is moving this code outside
of just your worker. The above algorithm doesn't do anything backend
specific aside from getting the message headers from a file. If we
instead until the message is put into the message store, we can get the
headers in a backend agnostic way using
store.Messages[uid].RFC822Headers.FieldsByKey("In-Reply-To"). I am
looking into adding a new thread when we receive *types.MessageInfo
(we need to wait for the message ID), because adding to the tree is
inexpensive.

These are my thoughts so far. Thanks for sending the patch. This is
still an excellent starting point.

Best,
Kevin