CHANGELOG.md

@@ -12,8 +12,6 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
   `:rmdir`, `:archive` and the `copy-to` option.
 - Display messages from bottom to top with `reverse-msglist-order=true` in
   `aerc.conf`.
-- Display threads from bottom to top with `reverse-thread-order=true` in
-  `aerc.conf`.
 
 ### Fixed
 

completer/completer.go

@@ -10,9 +10,7 @@ import (
 	"os/exec"
 	"regexp"
 	"strings"
-	"syscall"
 
-	"git.sr.ht/~rjarry/aerc/logging"
 	"github.com/google/shlex"
 )
 
@@ -73,10 +71,6 @@ func isAddressHeader(h string) bool {
 	return false
 }
 
-const maxCompletionLines = 100
-
-var tooManyLines = fmt.Errorf("returned more than %d lines", maxCompletionLines)
-
 // completeAddress uses the configured address book completion command to fetch
 // completions for the specified string, returning a slice of completions and
 // a prefix to be prepended to the selected completion, or an error.
@@ -94,8 +88,6 @@ func (c *Completer) completeAddress(s string) ([]string, string, error) {
 	if err != nil {
 		return nil, "", fmt.Errorf("stderr: %w", err)
 	}
-	// reset the process group id to allow killing all its children
-	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
 	if err := cmd.Start(); err != nil {
 		return nil, "", fmt.Errorf("cmd start: %w", err)
 	}
@@ -107,12 +99,6 @@ func (c *Completer) completeAddress(s string) ([]string, string, error) {
 
 	completions, err := readCompletions(stdout)
 	if err != nil {
-		// make sure to kill the process *and* all its children
-		//nolint:errcheck // who cares?
-		syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
-		logging.Warnf("command %s killed: %s", cmd, err)
-	}
-	if err != nil && !errors.Is(err, tooManyLines) {
 		buf, _ := io.ReadAll(stderr)
 		msg := strings.TrimSpace(string(buf))
 		if msg != "" {
@@ -162,38 +148,27 @@ func (c *Completer) getAddressCmd(s string) (*exec.Cmd, error) {
 func readCompletions(r io.Reader) ([]string, error) {
 	buf := bufio.NewReader(r)
 	completions := []string{}
-	for i := 0; i < maxCompletionLines; i++ {
+	for {
 		line, err := buf.ReadString('\n')
 		if errors.Is(err, io.EOF) {
 			return completions, nil
 		} else if err != nil {
 			return nil, err
 		}
-		if strings.TrimSpace(line) == "" {
-			// skip empty lines
-			continue
-		}
 		parts := strings.SplitN(line, "\t", 3)
 		addr, err := mail.ParseAddress(strings.TrimSpace(parts[0]))
 		if err != nil {
-			logging.Warnf(
-				"line %d: %#v: could not parse address: %v",
-				line, err)
-			continue
+			return nil, err
 		}
 		if len(parts) > 1 {
 			addr.Name = strings.TrimSpace(parts[1])
 		}
 		decoded, err := decodeMIME(addr.String())
 		if err != nil {
-			logging.Warnf(
-				"line %d: %#v: could not decode MIME string: %v",
-				i+1, line, err)
-			continue
+			return nil, fmt.Errorf("could not decode MIME string: %w", err)
 		}
 		completions = append(completions, decoded)
 	}
-	return completions, tooManyLines
 }
 
 func decodeMIME(s string) (string, error) {

config/aerc.conf

@@ -194,14 +194,6 @@ completion-popovers=true
 # Default: false
 #reverse-msglist-order = false
 
-# Reverse display of the mesage threads. Default order is the the intial
-# message is on the top with all the replies being displayed below. The
-# reverse option will put the initial message at the bottom with the
-# replies on top.
-#
-# Default: false
-#reverse-thread-order=false
-
 #[ui:account=foo]
 #
 # Enable a threaded view of messages. If this is not supported by the backend

config/config.go

@@ -79,8 +79,7 @@ type UIConfig struct {
 	BorderCharVertical   rune `ini:"-"`
 	BorderCharHorizontal rune `ini:"-"`
 
-	ReverseOrder       bool `ini:"reverse-msglist-order"`
-	ReverseThreadOrder bool `ini:"reverse-thread-order"`
+	ReverseOrder bool `ini:"reverse-msglist-order"`
 }
 
 type ContextType int

doc/aerc-config.5.scd

@@ -335,14 +335,6 @@ These options are configured in the *[ui]* section of aerc.conf.
 
 	Default: false
 
-*reverse-thread-order*
-	Reverse display of the mesage threads. Default order is the the intial
-	message is on the top with all the replies being displayed below. The
-	reverse option will put the initial message at the bottom with the
-	replies on top.
-
-	Default: false
-
 *threading-enabled*
 	Enable a threaded view of messages. If this is not supported by the
 	backend (IMAP server or notmuch), threads will be built by the client.

lib/iterator/index.go

@@ -21,9 +21,6 @@ func FixBounds(i, lower, upper int) int {
 // WrapBounds will wrap the index i around its upper- or lower-bound if
 // out-of-bound
 func WrapBounds(i, lower, upper int) int {
-	if upper <= 0 {
-		return lower
-	}
 	switch {
 	case i > upper:
 		i = lower + (i-upper-1)%upper

lib/msgstore.go

@@ -39,10 +39,9 @@ type MessageStore struct {
 
 	sortCriteria []*types.SortCriterion
 
-	threadedView       bool
-	reverseThreadOrder bool
-	buildThreads       bool
-	builder            *ThreadBuilder
+	threadedView bool
+	buildThreads bool
+	builder      *ThreadBuilder
 
 	// Map of uids we've asked the worker to fetch
 	onUpdate       func(store *MessageStore) // TODO: multiple onUpdate handlers
@@ -75,7 +74,7 @@ func NewMessageStore(worker *types.Worker,
 	dirInfo *models.DirectoryInfo,
 	defaultSortCriteria []*types.SortCriterion,
 	thread bool, clientThreads bool, clientThreadsDelay time.Duration,
-	reverseOrder bool, reverseThreadOrder bool,
+	reverseOrder bool,
 	triggerNewEmail func(*models.MessageInfo),
 	triggerDirectoryChange func(),
 ) *MessageStore {
@@ -92,9 +91,8 @@ func NewMessageStore(worker *types.Worker,
 
 		bodyCallbacks: make(map[uint32][]func(*types.FullMessage)),
 
-		threadedView:       thread,
-		buildThreads:       clientThreads,
-		reverseThreadOrder: reverseThreadOrder,
+		threadedView: thread,
+		buildThreads: clientThreads,
 
 		filter:       []string{"filter"},
 		sortCriteria: defaultSortCriteria,
@@ -233,7 +231,7 @@ func (store *MessageStore) Update(msg types.WorkerMessage) {
 		newMap := make(map[uint32]*models.MessageInfo)
 
 		builder := NewThreadBuilder(store.iterFactory)
-		builder.RebuildUids(msg.Threads, store.reverseThreadOrder)
+		builder.RebuildUids(msg.Threads)
 		store.uids = builder.Uids()
 		store.threads = msg.Threads
 
@@ -366,14 +364,6 @@ func (store *MessageStore) update(threads bool) {
 	}
 }
 
-func (store *MessageStore) SetReverseThreadOrder(reverse bool) {
-	store.reverseThreadOrder = reverse
-}
-
-func (store *MessageStore) ReverseThreadOrder() bool {
-	return store.reverseThreadOrder
-}
-
 func (store *MessageStore) SetThreadedView(thread bool) {
 	store.threadedView = thread
 	if store.buildThreads {
@@ -434,7 +424,7 @@ func (store *MessageStore) runThreadBuilderNow() {
 		}
 	}
 	// build new threads
-	th := store.builder.Threads(store.uids, store.reverseThreadOrder)
+	th := store.builder.Threads(store.uids)
 
 	// save local threads to the message store variable and
 	// run callback if defined (callback should reposition cursor)
@@ -603,20 +593,29 @@ func (store *MessageStore) NextPrev(delta int) {
 	if len(uids) == 0 {
 		return
 	}
-
 	iter := store.iterFactory.NewIterator(uids)
 
-	newIdx := store.FindIndexByUid(store.SelectedUid())
+	uid := store.SelectedUid()
+
+	newIdx := store.FindIndexByUid(uid)
 	if newIdx < 0 {
 		store.Select(uids[iter.StartIndex()])
 		return
 	}
-	newIdx = iterator.MoveIndex(
-		newIdx,
-		delta,
-		iter,
-		iterator.FixBounds,
-	)
+
+	low, high := iter.EndIndex(), iter.StartIndex()
+	sign := -1
+	if high < low {
+		low, high = high, low
+		sign = 1
+	}
+	newIdx += sign * delta
+	if newIdx >= len(uids) {
+		newIdx = high
+	} else if newIdx < 0 {
+		newIdx = low
+	}
+
 	store.Select(uids[newIdx])
 
 	if store.BuildThreads() && store.ThreadedView() {
@@ -632,7 +631,15 @@ func (store *MessageStore) NextPrev(delta int) {
 	if store.marker != nil {
 		store.marker.UpdateVisualMark()
 	}
-	store.updateResults()
+
+	nextResultIndex := len(store.results) - store.resultIndex - 2*delta
+	if nextResultIndex < 0 || nextResultIndex >= len(store.results) {
+		return
+	}
+	nextResultUid := store.results[nextResultIndex]
+	if nextResultUid == store.SelectedUid() {
+		store.resultIndex += delta
+	}
 }
 
 func (store *MessageStore) Next() {
@@ -683,35 +690,18 @@ func (store *MessageStore) ApplyClear() {
 	store.Sort(nil, nil)
 }
 
-func (store *MessageStore) updateResults() {
-	if len(store.results) == 0 || store.resultIndex < 0 {
-		return
-	}
-	uid := store.SelectedUid()
-	for i, u := range store.results {
-		if uid == u {
-			store.resultIndex = i
-			break
-		}
-	}
-}
-
 func (store *MessageStore) nextPrevResult(delta int) {
 	if len(store.results) == 0 {
 		return
 	}
-	iter := store.iterFactory.NewIterator(store.results)
-	if store.resultIndex < 0 {
-		store.resultIndex = iter.StartIndex()
-	} else {
-		store.resultIndex = iterator.MoveIndex(
-			store.resultIndex,
-			delta,
-			iter,
-			iterator.WrapBounds,
-		)
+	store.resultIndex += delta
+	if store.resultIndex >= len(store.results) {
+		store.resultIndex = 0
 	}
-	store.Select(store.results[store.resultIndex])
+	if store.resultIndex < 0 {
+		store.resultIndex = len(store.results) - 1
+	}
+	store.Select(store.results[len(store.results)-store.resultIndex-1])
 	store.update(false)
 }
 

lib/threadbuilder.go

@@ -55,7 +55,7 @@ func (builder *ThreadBuilder) Update(msg *models.MessageInfo) {
 }
 
 // Threads returns a slice of threads for the given list of uids
-func (builder *ThreadBuilder) Threads(uids []uint32, inverse bool) []*types.Thread {
+func (builder *ThreadBuilder) Threads(uids []uint32) []*types.Thread {
 	builder.Lock()
 	defer builder.Unlock()
 
@@ -67,7 +67,7 @@ func (builder *ThreadBuilder) Threads(uids []uint32, inverse bool) []*types.Thre
 	builder.sortThreads(threads, uids)
 
 	// rebuild uids from threads
-	builder.RebuildUids(threads, inverse)
+	builder.RebuildUids(threads)
 
 	elapsed := time.Since(start)
 	logging.Infof("%d threads created in %s", len(threads), elapsed)
@@ -155,23 +155,15 @@ func (builder *ThreadBuilder) sortThreads(threads []*types.Thread, orderedUids [
 }
 
 // RebuildUids rebuilds the uids from the given slice of threads
-func (builder *ThreadBuilder) RebuildUids(threads []*types.Thread, inverse bool) {
+func (builder *ThreadBuilder) RebuildUids(threads []*types.Thread) {
 	uids := make([]uint32, 0, len(threads))
 	iterT := builder.iterFactory.NewIterator(threads)
 	for iterT.Next() {
-		var threaduids []uint32
 		_ = iterT.Value().(*types.Thread).Walk(
 			func(t *types.Thread, level int, currentErr error) error {
-				threaduids = append(threaduids, t.Uid)
+				uids = append(uids, t.Uid)
 				return nil
 			})
-		if inverse {
-			for j := len(threaduids) - 1; j >= 0; j-- {
-				uids = append(uids, threaduids[j])
-			}
-		} else {
-			uids = append(uids, threaduids...)
-		}
 	}
 	result := make([]uint32, 0, len(uids))
 	iterU := builder.iterFactory.NewIterator(uids)

widgets/account.go

@@ -286,7 +286,6 @@ func (acct *AccountView) onMessage(msg types.WorkerMessage) {
 				acct.dirlist.UiConfig(name).ForceClientThreads,
 				acct.dirlist.UiConfig(name).ClientThreadsDelay,
 				acct.dirlist.UiConfig(name).ReverseOrder,
-				acct.dirlist.UiConfig(name).ReverseThreadOrder,
 				func(msg *models.MessageInfo) {
 					acct.conf.Triggers.ExecNewEmail(acct.acct,
 						acct.conf, msg)

widgets/dirtree.go

@@ -249,7 +249,7 @@ func (dt *DirectoryTree) countVisible(list []*types.Thread) (n int) {
 
 func (dt *DirectoryTree) displayText(node *types.Thread) string {
 	elems := strings.Split(dt.treeDirs[getAnyUid(node)], dt.pathSeparator)
-	return fmt.Sprintf("%s%s%s", threadPrefix(node, false), getFlag(node), elems[countLevels(node)])
+	return fmt.Sprintf("%s%s%s", threadPrefix(node), getFlag(node), elems[countLevels(node)])
 }
 
 func (dt *DirectoryTree) getDirectory(node *types.Thread) string {

widgets/msglist.go

@@ -12,7 +12,6 @@ import (
 	"git.sr.ht/~rjarry/aerc/config"
 	"git.sr.ht/~rjarry/aerc/lib"
 	"git.sr.ht/~rjarry/aerc/lib/format"
-	"git.sr.ht/~rjarry/aerc/lib/iterator"
 	"git.sr.ht/~rjarry/aerc/lib/ui"
 	"git.sr.ht/~rjarry/aerc/logging"
 	"git.sr.ht/~rjarry/aerc/models"
@@ -89,61 +88,57 @@ func (ml *MessageList) Draw(ctx *ui.Context) {
 		row          int = 0
 	)
 
-	createBaseCtx := func(uid uint32, row int) format.Ctx {
-		return format.Ctx{
-			FromAddress: acct.acct.From,
-			AccountName: acct.Name(),
-			MsgInfo:     store.Messages[uid],
-			MsgNum:      row,
-			MsgIsMarked: store.Marker().IsMarked(uid),
-		}
-	}
-
 	if store.ThreadedView() {
-		var (
-			lastSubject string
-			prevThread  *types.Thread
-			i           int = 0
-		)
-		factory := iterator.NewFactory(!store.ReverseThreadOrder())
-	threadLoop:
-		for iter := store.ThreadsIterator(); iter.Next(); {
-			var cur []*types.Thread
-			err := iter.Value().(*types.Thread).Walk(
-				func(t *types.Thread, _ int, _ error,
-				) error {
-					if t.Hidden || t.Deleted {
-						return nil
-					}
-					cur = append(cur, t)
+		iter := store.ThreadsIterator()
+		var i int = 0
+
+		for iter.Next() {
+			thread := iter.Value().(*types.Thread)
+			var lastSubject string
+			err := thread.Walk(func(t *types.Thread, _ int, currentErr error) error {
+				if currentErr != nil {
+					return currentErr
+				}
+				if t.Hidden || t.Deleted {
 					return nil
-				})
-			if err != nil {
-				logging.Errorf("thread walk: %v", err)
-			}
-			for curIter := factory.NewIterator(cur); curIter.Next(); {
+				}
 				if i < ml.Scroll() {
 					i++
-					continue
+					return nil
 				}
-				if thread := curIter.Value().(*types.Thread); thread != nil {
-					fmtCtx := createBaseCtx(thread.Uid, row)
-					fmtCtx.ThreadPrefix = threadPrefix(thread,
-						store.ReverseThreadOrder())
-					if fmtCtx.MsgInfo != nil && fmtCtx.MsgInfo.Envelope != nil {
-						baseSubject, _ := sortthread.GetBaseSubject(
-							fmtCtx.MsgInfo.Envelope.Subject)
-						fmtCtx.ThreadSameSubject = baseSubject == lastSubject &&
-							sameParent(thread, prevThread) &&
-							!isParent(thread)
-						lastSubject = baseSubject
-						prevThread = thread
+				i++
+				msg := store.Messages[t.Uid]
+				var prefix string
+				var subject string
+				var normalizedSubject string
+				if msg != nil {
+					prefix = threadPrefix(t)
+					if msg.Envelope != nil {
+						subject = msg.Envelope.Subject
+						normalizedSubject, _ = sortthread.GetBaseSubject(subject)
 					}
-					if ml.drawRow(textWidth, ctx, thread.Uid, row, &needsHeaders, fmtCtx) {
-						break threadLoop
-					}
-					row += 1
 				}
+				fmtCtx := format.Ctx{
+					FromAddress:       acct.acct.From,
+					AccountName:       acct.Name(),
+					MsgInfo:           msg,
+					MsgNum:            row,
+					MsgIsMarked:       store.Marker().IsMarked(t.Uid),
+					ThreadPrefix:      prefix,
+					ThreadSameSubject: normalizedSubject == lastSubject,
+				}
+				if ml.drawRow(textWidth, ctx, t.Uid, row, &needsHeaders, fmtCtx) {
+					return types.ErrSkipThread
+				}
+				lastSubject = normalizedSubject
+				row++
+				return nil
+			})
+			if err != nil {
+				logging.Warnf("failed to walk threads: %v", err)
+			}
+			if row >= ctx.Height() {
+				break
 			}
 		}
 	} else {
@@ -153,7 +148,15 @@ func (ml *MessageList) Draw(ctx *ui.Context) {
 				continue
 			}
 			uid := iter.Value().(uint32)
-			fmtCtx := createBaseCtx(uid, row)
+
+			msg := store.Messages[uid]
+			fmtCtx := format.Ctx{
+				FromAddress: acct.acct.From,
+				AccountName: acct.Name(),
+				MsgInfo:     msg,
+				MsgNum:      row,
+				MsgIsMarked: store.Marker().IsMarked(uid),
+			}
 			if ml.drawRow(textWidth, ctx, uid, row, &needsHeaders, fmtCtx) {
 				break
 			}
@@ -426,17 +429,13 @@ func (ml *MessageList) drawEmptyMessage(ctx *ui.Context) {
 		uiConfig.GetStyle(config.STYLE_MSGLIST_DEFAULT), "%s", msg)
 }
 
-func threadPrefix(t *types.Thread, reverse bool) string {
+func threadPrefix(t *types.Thread) string {
 	var arrow string
 	if t.Parent != nil {
 		if t.NextSibling != nil {
 			arrow = "├─>"
 		} else {
-			if reverse {
-				arrow = "┌─>"
-			} else {
-				arrow = "└─>"
-			}
+			arrow = "└─>"
 		}
 	}
 	var prefix []string
@@ -459,11 +458,3 @@ func threadPrefix(t *types.Thread, reverse bool) string {
 	ps := strings.Join(prefix, "")
 	return fmt.Sprintf("%v%v", ps, arrow)
 }
-
-func sameParent(left, right *types.Thread) bool {
-	return left.Root() == right.Root()
-}
-
-func isParent(t *types.Thread) bool {
-	return t == t.Root()
-}