~sircmpwn/aerc

notmuch updates v1 PROPOSED

This patch set refactors the notmuch backend in the hopes that the segfaults
due to the notmuch C library disappear.

It also adds the implementation of the modify-labels command.

*Note* I've also had to include the commit that implements the modify-labels
command, else it would not compile.

It is however only for reference, the actual PR was send separately.
Once (and if) merged the modifi-lables command commit can be dropped from this
patchset

Reto Brunner (4):
  notmuch: extract all notmuch db operations.
  notmuch: sync maildir flags
  Add modify-labels command
  notmuch: implement ModifyLabels

 commands/msg/modify-labels.go  |  69 ++++++++++++
 doc/aerc.1.scd                 |   8 ++
 go.mod                         |   2 +-
 go.sum                         |   2 +
 lib/msgstore.go                |   9 ++
 worker/notmuch/lib/database.go | 181 ++++++++++++++++++++++++++++++
 worker/notmuch/message.go      | 134 +++++++++-------------
 worker/notmuch/worker.go       | 195 ++++++++++++++-------------------
 worker/types/messages.go       |   7 ++
 9 files changed, 407 insertions(+), 200 deletions(-)
 create mode 100644 commands/msg/modify-labels.go
 create mode 100644 worker/notmuch/lib/database.go

-- 
2.23.0
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~sircmpwn/aerc/%3C20190911190704.57667-1-reto%40labrat.space%3E/mbox | git am -3
Learn more about email & git

[PATCH 1/4] notmuch: extract all notmuch db operations. Export this patch

For some reason the current code frequently segfaults due to an
invalid C memory address. This commit mediates that by never keeping an object
alive longer than absolutely necessary.
---
I can now read my emails wihtout it segfaulting every 5min, so it seems to work
much better than before.

 worker/notmuch/lib/database.go | 178 +++++++++++++++++++++++++++++++++
 worker/notmuch/message.go      | 134 +++++++++----------------
 worker/notmuch/worker.go       | 112 ++++-----------------
 3 files changed, 249 insertions(+), 175 deletions(-)
 create mode 100644 worker/notmuch/lib/database.go

diff --git a/worker/notmuch/lib/database.go b/worker/notmuch/lib/database.go
new file mode 100644
index 0000000..3a106f3
--- /dev/null
+++ b/worker/notmuch/lib/database.go
@@ -0,0 +1,178 @@
+//+build notmuch
+
+package lib
+
+import (
+	"fmt"
+	"log"
+
+	notmuch "github.com/zenhack/go.notmuch"
+)
+
+type DB struct {
+	path         string
+	excludedTags []string
+	ro           *notmuch.DB
+	logger       *log.Logger
+}
+
+func NewDB(path string, excludedTags []string,
+	logger *log.Logger) *DB {
+	db := &DB{
+		path:         path,
+		excludedTags: excludedTags,
+		logger:       logger,
+	}
+	return db
+}
+
+func (db *DB) Connect() error {
+	return db.connectRO()
+}
+
+// connectRW returns a writable notmuch DB, which needs to be closed to commit
+// the changes and to release the DB lock
+func (db *DB) connectRW() (*notmuch.DB, error) {
+	rw, err := notmuch.Open(db.path, notmuch.DBReadWrite)
+	if err != nil {
+		return nil, fmt.Errorf("could not connect to notmuch db: %v", err)
+	}
+	return rw, err
+}
+
+// connectRO connects a RO db to the worker
+func (db *DB) connectRO() error {
+	if db.ro != nil {
+		if err := db.ro.Close(); err != nil {
+			db.logger.Printf("connectRO: could not close the old db: %v", err)
+		}
+	}
+	var err error
+	db.ro, err = notmuch.Open(db.path, notmuch.DBReadOnly)
+	if err != nil {
+		return fmt.Errorf("could not connect to notmuch db: %v", err)
+	}
+	return nil
+}
+
+//getQuery returns a query based on the provided query string.
+//It also configures the query as specified on the worker
+func (db *DB) newQuery(query string) (*notmuch.Query, error) {
+	if db.ro == nil {
+		return nil, fmt.Errorf("not connected to the notmuch db")
+	}
+	q := db.ro.NewQuery(query)
+	q.SetExcludeScheme(notmuch.EXCLUDE_TRUE)
+	q.SetSortScheme(notmuch.SORT_OLDEST_FIRST)
+	for _, t := range db.excludedTags {
+		err := q.AddTagExclude(t)
+		if err != nil && err != notmuch.ErrIgnored {
+			return nil, err
+		}
+	}
+	return q, nil
+}
+
+func (db *DB) MsgIDsFromQuery(q string) ([]string, error) {
+	if db.ro == nil {
+		return nil, fmt.Errorf("not connected to the notmuch db")
+	}
+	// query, err := db.cachedOrCreateQuery(q)
+	query, err := db.newQuery(q)
+	if err != nil {
+		return nil, err
+	}
+	msgs, err := query.Messages()
+	if err != nil {
+		return nil, err
+	}
+	var msg *notmuch.Message
+	var msgIDs []string
+	for msgs.Next(&msg) {
+		msgIDs = append(msgIDs, msg.ID())
+	}
+	return msgIDs, nil
+}
+
+type MessageCount struct {
+	Exists int
+	Unread int
+}
+
+func (db *DB) QueryCountMessages(q string) (MessageCount, error) {
+	// query, err := db.cachedOrCreateQuery(q)
+	query, err := db.newQuery(q)
+	if err != nil {
+		return MessageCount{}, err
+	}
+	exists := query.CountMessages()
+	uq, err := db.newQuery(fmt.Sprintf("(%v) and (tag:unread)"))
+	defer uq.Close()
+	if err != nil {
+		return MessageCount{}, err
+	}
+	unread := uq.CountMessages()
+	return MessageCount{
+		Exists: exists,
+		Unread: unread,
+	}, nil
+}
+
+func (db *DB) MsgFilename(key string) (string, error) {
+	msg, err := db.ro.FindMessage(key)
+	if err != nil {
+		return "", err
+	}
+	return msg.Filename(), nil
+}
+
+func (db *DB) MsgTags(key string) ([]string, error) {
+	msg, err := db.ro.FindMessage(key)
+	if err != nil {
+		return nil, err
+	}
+	ts := msg.Tags()
+	var tags []string
+	var tag *notmuch.Tag
+	for ts.Next(&tag) {
+		tags = append(tags, tag.Value)
+	}
+	return tags, nil
+}
+
+func (db *DB) msgModify(key string,
+	cb func(*notmuch.Message) error) error {
+	defer db.connectRO()
+	db.ro.Close()
+
+	rw, err := db.connectRW()
+	if err != nil {
+		return err
+	}
+	defer rw.Close()
+
+	msg, err := rw.FindMessage(key)
+	if err != nil {
+		return err
+	}
+	defer msg.Close()
+
+	cb(msg)
+	return nil
+}
+
+func (db *DB) MsgModifyTags(key string, add, remove []string) error {
+	err := db.msgModify(key, func(msg *notmuch.Message) error {
+		ierr := msg.Atomic(func(msg *notmuch.Message) {
+			for _, t := range add {
+				msg.AddTag(t)
+			}
+			for _, t := range remove {
+				msg.RemoveTag(t)
+			}
+		})
+		return ierr
+	})
+	return err
+}
+
diff --git a/worker/notmuch/message.go b/worker/notmuch/message.go
index aa16cee..c51e2e9 100644
--- a/worker/notmuch/message.go
+++ b/worker/notmuch/message.go
@@ -11,22 +11,24 @@ import (
 
 	"git.sr.ht/~sircmpwn/aerc/models"
 	"git.sr.ht/~sircmpwn/aerc/worker/lib"
+	notmuch "git.sr.ht/~sircmpwn/aerc/worker/notmuch/lib"
 	"github.com/emersion/go-message"
 	_ "github.com/emersion/go-message/charset"
-	notmuch "github.com/zenhack/go.notmuch"
 )
 
 type Message struct {
-	uid     uint32
-	key     string
-	msg     *notmuch.Message
-	rwDB    func() (*notmuch.DB, error) // used to open a db for writing
-	refresh func(*Message) error        // called after msg modification
+	uid uint32
+	key string
+	db  *notmuch.DB
 }
 
 // NewReader reads a message into memory and returns an io.Reader for it.
 func (m *Message) NewReader() (io.Reader, error) {
-	f, err := os.Open(m.msg.Filename())
+	name, err := m.Filename()
+	if err != nil {
+		return nil, err
+	}
+	f, err := os.Open(name)
 	if err != nil {
 		return nil, err
 	}
@@ -46,7 +48,11 @@ func (m *Message) MessageInfo() (*models.MessageInfo, error) {
 // NewBodyPartReader creates a new io.Reader for the requested body part(s) of
 // the message.
 func (m *Message) NewBodyPartReader(requestedParts []int) (io.Reader, error) {
-	f, err := os.Open(m.msg.Filename())
+	name, err := m.Filename()
+	if err != nil {
+		return nil, err
+	}
+	f, err := os.Open(name)
 	if err != nil {
 		return nil, err
 	}
@@ -61,7 +67,11 @@ func (m *Message) NewBodyPartReader(requestedParts []int) (io.Reader, error) {
 // MarkRead either adds or removes the maildir.FlagSeen flag from the message.
 func (m *Message) MarkRead(seen bool) error {
 	haveUnread := false
-	for _, t := range m.tags() {
+	tags, err := m.Tags()
+	if err != nil {
+		return err
+	}
+	for _, t := range tags {
 		if t == "unread" {
 			haveUnread = true
 			break
@@ -80,7 +90,7 @@ func (m *Message) MarkRead(seen bool) error {
 		return nil
 	}
 
-	err := m.AddTag("unread")
+	err = m.AddTag("unread")
 	if err != nil {
 		return err
 	}
@@ -88,86 +98,18 @@ func (m *Message) MarkRead(seen bool) error {
 }
 
 // tags returns the notmuch tags of a message
-func (m *Message) tags() []string {
-	ts := m.msg.Tags()
-	var tags []string
-	var tag *notmuch.Tag
-	for ts.Next(&tag) {
-		tags = append(tags, tag.Value)
-	}
-	return tags
-}
-
-func (m *Message) modify(cb func(*notmuch.Message) error) error {
-	db, err := m.rwDB()
-	if err != nil {
-		return err
-	}
-	defer db.Close()
-	msg, err := db.FindMessage(m.key)
-	if err != nil {
-		return err
-	}
-	err = cb(msg)
-	if err != nil {
-		return err
-	}
-	// we need to explicitly close here, else we don't commit
-	dcerr := db.Close()
-	if dcerr != nil && err == nil {
-		err = dcerr
-	}
-	// next we need to refresh the notmuch msg, else we serve stale tags
-	rerr := m.refresh(m)
-	if rerr != nil && err == nil {
-		err = rerr
-	}
-	return err
-}
-
-func (m *Message) AddTag(tag string) error {
-	err := m.modify(func(msg *notmuch.Message) error {
-		return msg.AddTag(tag)
-	})
-	return err
-}
-
-func (m *Message) AddTags(tags []string) error {
-	err := m.modify(func(msg *notmuch.Message) error {
-		ierr := msg.Atomic(func(msg *notmuch.Message) {
-			for _, t := range tags {
-				msg.AddTag(t)
-			}
-		})
-		return ierr
-	})
-	return err
-}
-
-func (m *Message) RemoveTag(tag string) error {
-	err := m.modify(func(msg *notmuch.Message) error {
-		return msg.RemoveTag(tag)
-	})
-	return err
-}
-
-func (m *Message) RemoveTags(tags []string) error {
-	err := m.modify(func(msg *notmuch.Message) error {
-		ierr := msg.Atomic(func(msg *notmuch.Message) {
-			for _, t := range tags {
-				msg.RemoveTag(t)
-			}
-		})
-		return ierr
-	})
-	return err
+func (m *Message) Tags() ([]string, error) {
+	return m.db.MsgTags(m.key)
 }
 
 func (m *Message) ModelFlags() ([]models.Flag, error) {
 	var flags []models.Flag
 	seen := true
-
-	for _, tag := range m.tags() {
+	tags, err := m.Tags()
+	if err != nil {
+		return nil, err
+	}
+	for _, tag := range tags {
 		switch tag {
 		case "replied":
 			flags = append(flags, models.AnsweredFlag)
@@ -188,3 +130,25 @@ func (m *Message) ModelFlags() ([]models.Flag, error) {
 func (m *Message) UID() uint32 {
 	return m.uid
 }
+
+func (m *Message) Filename() (string, error) {
+	return m.db.MsgFilename(m.key)
+}
+
+//AddTag adds a single tag.
+//Consider using *Message.ModifyTags for multiple additions / removals
+//instead of looping over a tag array
+func (m *Message) AddTag(tag string) error {
+	return m.ModifyTags([]string{tag}, nil)
+}
+
+//RemoveTag removes a single tag.
+//Consider using *Message.ModifyTags for multiple additions / removals
+//instead of looping over a tag array
+func (m *Message) RemoveTag(tag string) error {
+	return m.ModifyTags(nil, []string{tag})
+}
+
+func (m *Message) ModifyTags(add, remove []string) error {
+	return m.db.MsgModifyTags(m.key, add, remove)
+}
diff --git a/worker/notmuch/worker.go b/worker/notmuch/worker.go
index 58a63ec..59624b3 100644
--- a/worker/notmuch/worker.go
+++ b/worker/notmuch/worker.go
@@ -14,9 +14,9 @@ import (
 	"git.sr.ht/~sircmpwn/aerc/lib/uidstore"
 	"git.sr.ht/~sircmpwn/aerc/models"
 	"git.sr.ht/~sircmpwn/aerc/worker/handlers"
+	notmuch "git.sr.ht/~sircmpwn/aerc/worker/notmuch/lib"
 	"git.sr.ht/~sircmpwn/aerc/worker/types"
 	"github.com/mitchellh/go-homedir"
-	notmuch "github.com/zenhack/go.notmuch"
 )
 
 func init() {
@@ -27,12 +27,10 @@ var errUnsupported = fmt.Errorf("unsupported command")
 
 type worker struct {
 	w            *types.Worker
-	pathToDB     string
-	db           *notmuch.DB
 	query        string
 	uidStore     *uidstore.Store
-	excludedTags []string
 	nameQueryMap map[string]string
+	db           *notmuch.DB
 }
 
 // NewWorker creates a new maildir worker with the provided worker.
@@ -116,46 +114,18 @@ func (w *worker) handleConfigure(msg *types.Configure) error {
 	if err != nil {
 		return fmt.Errorf("could not resolve home directory: %v", err)
 	}
-	w.pathToDB = filepath.Join(home, u.Path)
+	pathToDB := filepath.Join(home, u.Path)
 	w.uidStore = uidstore.NewStore()
-
 	if err = w.loadQueryMap(msg.Config); err != nil {
 		return fmt.Errorf("could not load query map: %v", err)
 	}
-	if err = w.loadExcludeTags(msg.Config); err != nil {
-		return fmt.Errorf("could not load excluded tags: %v", err)
-	}
-	w.w.Logger.Printf("configured db directory: %s", w.pathToDB)
-	return nil
-}
-
-// connectRW returns a writable notmuch DB, which needs to be closed to commit
-// the changes and to release the DB lock
-func (w *worker) connectRW() (*notmuch.DB, error) {
-	db, err := notmuch.Open(w.pathToDB, notmuch.DBReadWrite)
-	if err != nil {
-		return nil, fmt.Errorf("could not connect to notmuch db: %v", err)
-	}
-	return db, err
-}
-
-// connectRO connects a RO db to the worker
-func (w *worker) connectRO() error {
-	if w.db != nil {
-		if err := w.db.Close(); err != nil {
-			w.w.Logger.Printf("connectRO: could not close the old db: %v", err)
-		}
-	}
-	var err error
-	w.db, err = notmuch.Open(w.pathToDB, notmuch.DBReadOnly)
-	if err != nil {
-		return fmt.Errorf("could not connect to notmuch db: %v", err)
-	}
+	excludedTags := w.loadExcludeTags(msg.Config)
+	w.db = notmuch.NewDB(pathToDB, excludedTags, w.w.Logger)
 	return nil
 }
 
 func (w *worker) handleConnect(msg *types.Connect) error {
-	err := w.connectRO()
+	err := w.db.Connect()
 	if err != nil {
 		return err
 	}
@@ -177,21 +147,6 @@ func (w *worker) handleListDirectories(msg *types.ListDirectories) error {
 	return nil
 }
 
-//getQuery returns a query based on the provided query string.
-//It also configures the query as specified on the worker
-func (w *worker) getQuery(query string) (*notmuch.Query, error) {
-	q := w.db.NewQuery(query)
-	q.SetExcludeScheme(notmuch.EXCLUDE_TRUE)
-	q.SetSortScheme(notmuch.SORT_OLDEST_FIRST)
-	for _, t := range w.excludedTags {
-		err := q.AddTagExclude(t)
-		if err != nil && err != notmuch.ErrIgnored {
-			return nil, err
-		}
-	}
-	return q, nil
-}
-
 func (w *worker) handleOpenDirectory(msg *types.OpenDirectory) error {
 	w.w.Logger.Printf("opening %s", msg.Directory)
 	// try the friendly name first, if that fails assume it's a query
@@ -200,7 +155,7 @@ func (w *worker) handleOpenDirectory(msg *types.OpenDirectory) error {
 		q = msg.Directory
 	}
 	w.query = q
-	query, err := w.getQuery(w.query)
+	count, err := w.db.QueryCountMessages(w.query)
 	if err != nil {
 		return err
 	}
@@ -211,11 +166,11 @@ func (w *worker) handleOpenDirectory(msg *types.OpenDirectory) error {
 			Flags:    []string{},
 			ReadOnly: false,
 			// total messages
-			Exists: query.CountMessages(),
+			Exists: count.Exists,
 			// new messages since mailbox was last opened
 			Recent: 0,
 			// total unread
-			Unseen: 0,
+			Unseen: count.Unread,
 		},
 	}
 	w.w.PostMessage(info, nil)
@@ -226,11 +181,7 @@ func (w *worker) handleOpenDirectory(msg *types.OpenDirectory) error {
 
 func (w *worker) handleFetchDirectoryContents(
 	msg *types.FetchDirectoryContents) error {
-	q, err := w.getQuery(w.query)
-	if err != nil {
-		return err
-	}
-	uids, err := w.uidsFromQuery(q)
+	uids, err := w.uidsFromQuery(w.query)
 	if err != nil {
 		w.w.Logger.Printf("error scanning uids: %v", err)
 		return err
@@ -267,15 +218,14 @@ func (w *worker) handleFetchMessageHeaders(
 	return nil
 }
 
-func (w *worker) uidsFromQuery(query *notmuch.Query) ([]uint32, error) {
-	msgs, err := query.Messages()
+func (w *worker) uidsFromQuery(query string) ([]uint32, error) {
+	msgIDs, err := w.db.MsgIDsFromQuery(query)
 	if err != nil {
 		return nil, err
 	}
-	var msg *notmuch.Message
 	var uids []uint32
-	for msgs.Next(&msg) {
-		uid := w.uidStore.GetOrInsert(msg.ID())
+	for _, id := range msgIDs {
+		uid := w.uidStore.GetOrInsert(id)
 		uids = append(uids, uid)
 
 	}
@@ -287,25 +237,10 @@ func (w *worker) msgFromUid(uid uint32) (*Message, error) {
 	if !ok {
 		return nil, fmt.Errorf("Invalid uid: %v", uid)
 	}
-	nm, err := w.db.FindMessage(key)
-	if err != nil {
-		return nil, fmt.Errorf("Could not fetch message for key %q: %v", key, err)
-	}
 	msg := &Message{
-		key:  key,
-		uid:  uid,
-		msg:  nm,
-		rwDB: w.connectRW,
-		refresh: func(m *Message) error {
-			//close the old message manually, else we segfault during gc
-			m.msg.Close()
-			err := w.connectRO()
-			if err != nil {
-				return err
-			}
-			m.msg, err = w.db.FindMessage(m.key)
-			return err
-		},
+		key: key,
+		uid: uid,
+		db:  w.db,
 	}
 	return msg, nil
 }
@@ -409,11 +344,7 @@ func (w *worker) handleSearchDirectory(msg *types.SearchDirectory) error {
 	s := strings.Join(msg.Argv[1:], " ")
 	// we only want to search in the current query, so merge the two together
 	search := fmt.Sprintf("(%v) and (%v)", w.query, s)
-	query, err := w.getQuery(search)
-	if err != nil {
-		return err
-	}
-	uids, err := w.uidsFromQuery(query)
+	uids, err := w.uidsFromQuery(search)
 	if err != nil {
 		return err
 	}
@@ -452,12 +383,13 @@ func (w *worker) loadQueryMap(acctConfig *config.AccountConfig) error {
 	return nil
 }
 
-func (w *worker) loadExcludeTags(acctConfig *config.AccountConfig) error {
+func (w *worker) loadExcludeTags(
+	acctConfig *config.AccountConfig) []string {
 	raw, ok := acctConfig.Params["exclude-tags"]
 	if !ok {
 		// nothing to do
 		return nil
 	}
-	w.excludedTags = strings.Split(raw, ",")
-	return nil
+	excludedTags := strings.Split(raw, ",")
+	return excludedTags
 }
-- 
2.23.0

[PATCH 2/4] notmuch: sync maildir flags Export this patch

Sync notmuch special tags like "unread" back to maildir flags

---
 go.mod                         | 2 +-
 go.sum                         | 2 ++
 worker/notmuch/lib/database.go | 5 ++++-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/go.mod b/go.mod
index 1e30e3d..aeb7f8c 100644
--- a/go.mod
+++ b/go.mod
@@ -29,7 +29,7 @@ require (
 	github.com/smartystreets/assertions v1.0.1 // indirect
 	github.com/smartystreets/goconvey v0.0.0-20190710185942-9d28bd7c0945 // indirect
 	github.com/stretchr/testify v1.3.0
-	github.com/zenhack/go.notmuch v0.0.0-20190726231123-3d59f87d986e
+	github.com/zenhack/go.notmuch v0.0.0-20190821052706-5a1961965cfb
 	golang.org/x/net v0.0.0-20190813141303-74dc4d7220e7 // indirect
 	golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
 	golang.org/x/sys v0.0.0-20190813064441-fde4db37ae7a // indirect
diff --git a/go.sum b/go.sum
index 20d560f..2749ac5 100644
--- a/go.sum
+++ b/go.sum
@@ -85,6 +85,8 @@ github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0
 github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
 github.com/zenhack/go.notmuch v0.0.0-20190726231123-3d59f87d986e h1:IeB1sn/RMq/o0PP7HNMNpTUHvvOZln9smuJXz1S7qJY=
 github.com/zenhack/go.notmuch v0.0.0-20190726231123-3d59f87d986e/go.mod h1:zJtFvR3NinVdmBiLyB4MyXKmqyVfZEb2cK97ISfTgV8=
+github.com/zenhack/go.notmuch v0.0.0-20190821052706-5a1961965cfb h1:eZBIw4TilXSAEYcWKf51bERhwH431YwntDYus0Bgxh0=
+github.com/zenhack/go.notmuch v0.0.0-20190821052706-5a1961965cfb/go.mod h1:zJtFvR3NinVdmBiLyB4MyXKmqyVfZEb2cK97ISfTgV8=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
 golang.org/x/crypto v0.0.0-20190605123033-f99c8df09eb5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
 golang.org/x/image v0.0.0-20190523035834-f03afa92d3ff/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
diff --git a/worker/notmuch/lib/database.go b/worker/notmuch/lib/database.go
index 3a106f3..b58b27f 100644
--- a/worker/notmuch/lib/database.go
+++ b/worker/notmuch/lib/database.go
@@ -158,6 +158,10 @@ func (db *DB) msgModify(key string,
 	defer msg.Close()
 
 	cb(msg)
+	err = msg.TagsToMaildirFlags()
+	if err != nil {
+		db.logger.Printf("could not sync maildir flags: %v", err)
+	}
 	return nil
 }
 
@@ -175,4 +179,3 @@ func (db *DB) MsgModifyTags(key string, add, remove []string) error {
 	})
 	return err
 }
-
-- 
2.23.0

[PATCH 3/4] Add modify-labels command Export this patch

This adds the event type as well as the command implementation, but no backend
support it yet.
---

Note this commit is only for reference so that this patch series actually compiles.
It can be dropped after the "read" PR is merged (if you are happy with it)

 commands/msg/modify-labels.go | 69 +++++++++++++++++++++++++++++++++++
 doc/aerc.1.scd                |  8 ++++
 lib/msgstore.go               |  9 +++++
 worker/types/messages.go      |  7 ++++
 4 files changed, 93 insertions(+)
 create mode 100644 commands/msg/modify-labels.go

diff --git a/commands/msg/modify-labels.go b/commands/msg/modify-labels.go
new file mode 100644
index 0000000..f33a9ca
--- /dev/null
+++ b/commands/msg/modify-labels.go
@@ -0,0 +1,69 @@
+package msg
+
+import (
+	"errors"
+	"time"
+
+	"git.sr.ht/~sircmpwn/aerc/widgets"
+	"git.sr.ht/~sircmpwn/aerc/worker/types"
+	"github.com/gdamore/tcell"
+)
+
+type ModifyLabels struct{}
+
+func init() {
+	register(ModifyLabels{})
+}
+
+func (ModifyLabels) Aliases() []string {
+	return []string{"modify-labels"}
+}
+
+func (ModifyLabels) Complete(aerc *widgets.Aerc, args []string) []string {
+	return nil
+}
+
+func (ModifyLabels) Execute(aerc *widgets.Aerc, args []string) error {
+	changes := args[1:]
+	if len(changes) == 0 {
+		return errors.New("Usage: modify-labels <[+-]label> ...")
+	}
+
+	widget := aerc.SelectedTab().(widgets.ProvidesMessage)
+	acct := widget.SelectedAccount()
+	if acct == nil {
+		return errors.New("No account selected")
+	}
+	store := widget.Store()
+	if store == nil {
+		return errors.New("Cannot perform action. Messages still loading")
+	}
+	msg, err := widget.SelectedMessage()
+	if err != nil {
+		return err
+	}
+	var add, remove []string
+	for _, l := range changes {
+		switch l[0] {
+		case '+':
+			add = append(add, l[1:])
+		case '-':
+			remove = append(remove, l[1:])
+		default:
+			// if no operand is given assume add
+			add = append(add, l)
+		}
+	}
+	store.ModifyLabels([]uint32{msg.Uid}, add, remove, func(
+		msg types.WorkerMessage) {
+
+		switch msg := msg.(type) {
+		case *types.Done:
+			aerc.PushStatus("labels updated", 10*time.Second)
+		case *types.Error:
+			aerc.PushStatus(" "+msg.Error.Error(), 10*time.Second).
+				Color(tcell.ColorDefault, tcell.ColorRed)
+		}
+	})
+	return nil
+}
diff --git a/doc/aerc.1.scd b/doc/aerc.1.scd
index c3be01b..8650d9a 100644
--- a/doc/aerc.1.scd
+++ b/doc/aerc.1.scd
@@ -133,6 +133,14 @@ message list, the message in the message viewer, etc).
 
 	*-t*: Toggle the selected message between read and unread.
 
+*modify-labels* <[+-]label>...
+	Modify message labels (e.g. notmuch tags).++
+Labels prefixed with a '+' are added, those prefixed with a '-' removed.++
+As a convenience, labels without either operand add the specified label.
+
+	Example: `modify-labels +inbox -spam unread` adds the labels inbox and unread
+	and removes spam
+
 *unsubscribe*
 	Attempt to automatically unsubscribe the user from the mailing list through
 	use of the List-Unsubscribe header. If supported, aerc may open a compose
diff --git a/lib/msgstore.go b/lib/msgstore.go
index 2733288..73c79e7 100644
--- a/lib/msgstore.go
+++ b/lib/msgstore.go
@@ -425,3 +425,12 @@ func (store *MessageStore) NextResult() {
 func (store *MessageStore) PrevResult() {
 	store.nextPrevResult(-1)
 }
+
+func (store *MessageStore) ModifyLabels(uids []uint32, add, remove []string,
+	cb func(msg types.WorkerMessage)) {
+	store.worker.PostAction(&types.ModifyLabels{
+		Uids:   uids,
+		Add:    add,
+		Remove: remove,
+	}, cb)
+}
diff --git a/worker/types/messages.go b/worker/types/messages.go
index 7ab94e0..9f40b8f 100644
--- a/worker/types/messages.go
+++ b/worker/types/messages.go
@@ -175,3 +175,10 @@ type MessagesDeleted struct {
 	Message
 	Uids []uint32
 }
+
+type ModifyLabels struct {
+	Message
+	Uids   []uint32
+	Add    []string
+	Remove []string
+}
-- 
2.23.0

[PATCH 4/4] notmuch: implement ModifyLabels Export this patch

---
 worker/notmuch/worker.go | 87 ++++++++++++++++++++++++++++------------
 1 file changed, 61 insertions(+), 26 deletions(-)

diff --git a/worker/notmuch/worker.go b/worker/notmuch/worker.go
index 59624b3..3d48d8c 100644
--- a/worker/notmuch/worker.go
+++ b/worker/notmuch/worker.go
@@ -90,6 +90,8 @@ func (w *worker) handleMessage(msg types.WorkerMessage) error {
 		return w.handleReadMessages(msg)
 	case *types.SearchDirectory:
 		return w.handleSearchDirectory(msg)
+	case *types.ModifyLabels:
+		return w.handleModifyLabels(msg)
 
 		// not implemented, they are generally not used
 		// in a notmuch based workflow
@@ -159,7 +161,6 @@ func (w *worker) handleOpenDirectory(msg *types.OpenDirectory) error {
 	if err != nil {
 		return err
 	}
-	//TODO: why does this need to be sent twice??
 	info := &types.DirectoryInfo{
 		Info: &models.DirectoryInfo{
 			Name:     msg.Directory,
@@ -173,6 +174,7 @@ func (w *worker) handleOpenDirectory(msg *types.OpenDirectory) error {
 			Unseen: count.Unread,
 		},
 	}
+	//TODO: why does this need to be sent twice??
 	w.w.PostMessage(info, nil)
 	w.w.PostMessage(info, nil)
 	w.done(msg)
@@ -181,15 +183,10 @@ func (w *worker) handleOpenDirectory(msg *types.OpenDirectory) error {
 
 func (w *worker) handleFetchDirectoryContents(
 	msg *types.FetchDirectoryContents) error {
-	uids, err := w.uidsFromQuery(w.query)
+	err := w.emitDirectoryContents(msg)
 	if err != nil {
-		w.w.Logger.Printf("error scanning uids: %v", err)
 		return err
 	}
-	w.w.PostMessage(&types.DirectoryContents{
-		Message: types.RespondTo(msg),
-		Uids:    uids,
-	}, nil)
 	w.done(msg)
 	return nil
 }
@@ -203,16 +200,12 @@ func (w *worker) handleFetchMessageHeaders(
 			w.err(msg, err)
 			continue
 		}
-		info, err := m.MessageInfo()
+		err = w.emitMessageInfo(m, msg)
 		if err != nil {
-			w.w.Logger.Printf("could not get message info: %v", err)
+			w.w.Logger.Printf(err.Error())
 			w.err(msg, err)
 			continue
 		}
-		w.w.PostMessage(&types.MessageInfo{
-			Message: types.RespondTo(msg),
-			Info:    info,
-		}, nil)
 	}
 	w.done(msg)
 	return nil
@@ -274,15 +267,11 @@ func (w *worker) handleFetchMessageBodyPart(
 	}
 
 	// send updated flags to ui
-	info, err := m.MessageInfo()
+	err = w.emitMessageInfo(m, msg)
 	if err != nil {
-		w.w.Logger.Printf("could not fetch message info: %v", err)
-		return err
+		w.w.Logger.Printf(err.Error())
+		w.err(msg, err)
 	}
-	w.w.PostMessage(&types.MessageInfo{
-		Message: types.RespondTo(msg),
-		Info:    info,
-	}, nil)
 	w.done(msg)
 	return nil
 }
@@ -324,16 +313,12 @@ func (w *worker) handleReadMessages(msg *types.ReadMessages) error {
 			w.err(msg, err)
 			continue
 		}
-		info, err := m.MessageInfo()
+		err = w.emitMessageInfo(m, msg)
 		if err != nil {
-			w.w.Logger.Printf("could not get message info: %v", err)
+			w.w.Logger.Printf(err.Error())
 			w.err(msg, err)
 			continue
 		}
-		w.w.PostMessage(&types.MessageInfo{
-			Message: types.RespondTo(msg),
-			Info:    info,
-		}, nil)
 	}
 	w.done(msg)
 	return nil
@@ -355,6 +340,31 @@ func (w *worker) handleSearchDirectory(msg *types.SearchDirectory) error {
 	return nil
 }
 
+func (w *worker) handleModifyLabels(msg *types.ModifyLabels) error {
+	for _, uid := range msg.Uids {
+		m, err := w.msgFromUid(uid)
+		if err != nil {
+			return fmt.Errorf("could not get message from uid %v: %v", uid, err)
+		}
+		err = m.ModifyTags(msg.Add, msg.Remove)
+		if err != nil {
+			return fmt.Errorf("could not modify message tags: %v", err)
+		}
+		err = w.emitMessageInfo(m, msg)
+		if err != nil {
+			return err
+		}
+	}
+	// tags changed, most probably some messages shifted to other folders
+	// so we need to re-enumerate the query content
+	err := w.emitDirectoryContents(msg)
+	if err != nil {
+		return err
+	}
+	w.done(msg)
+	return nil
+}
+
 func (w *worker) loadQueryMap(acctConfig *config.AccountConfig) error {
 	raw, ok := acctConfig.Params["query-map"]
 	if !ok {
@@ -393,3 +403,28 @@ func (w *worker) loadExcludeTags(
 	excludedTags := strings.Split(raw, ",")
 	return excludedTags
 }
+
+func (w *worker) emitDirectoryContents(parent types.WorkerMessage) error {
+	uids, err := w.uidsFromQuery(w.query)
+	if err != nil {
+		return fmt.Errorf("could not fetch uids: %v", err)
+	}
+	w.w.PostMessage(&types.DirectoryContents{
+		Message: types.RespondTo(parent),
+		Uids:    uids,
+	}, nil)
+	return nil
+}
+
+func (w *worker) emitMessageInfo(m *Message,
+	parent types.WorkerMessage) error {
+	info, err := m.MessageInfo()
+	if err != nil {
+		return fmt.Errorf("could not get MessageInfo: %v", err)
+	}
+	w.w.PostMessage(&types.MessageInfo{
+		Message: types.RespondTo(parent),
+		Info:    info,
+	}, nil)
+	return nil
+}
-- 
2.23.0
View this thread in the archives