Koni Marti: 5 store: extract marking behavior and add tests store: remove unneeded header callback util: fetch message headers for nil messages mark: allow multiple visual selections mark: (un)mark message threads 20 files changed, 525 insertions(+), 205 deletions(-)
aerc/patches: SUCCESS in 7m2s [store: extract marking behavior and add tests][0] v3 from [Koni Marti][1] [0]: https://lists.sr.ht/~rjarry/aerc-devel/patches/34231 [1]: mailto:koni.marti@gmail.com ✓ #810631 SUCCESS aerc/patches/openbsd.yml https://builds.sr.ht/~rjarry/job/810631 ✓ #810630 SUCCESS aerc/patches/alpine-edge.yml https://builds.sr.ht/~rjarry/job/810630
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~rjarry/aerc-devel/patches/34231/mbox | git am -3Learn more about email & git
Separate the marking functions from the message store and extract the marking behavior into its own class with tests. Signed-off-by: Koni Marti <koni.marti@gmail.com> --- v2->v3: * rebased on current master commands/msg/copy.go | 4 +- commands/msg/mark.go | 18 ++-- commands/msg/modify-labels.go | 4 +- commands/msg/pipe.go | 4 +- commands/msg/read.go | 4 +- lib/marker/marker.go | 188 ++++++++++++++++++++++++++++++++++ lib/marker/marker_test.go | 140 +++++++++++++++++++++++++ lib/msgstore.go | 178 +++++--------------------------- widgets/account.go | 12 ++- widgets/msglist.go | 20 +++- widgets/msgviewer.go | 3 +- 11 files changed, 403 insertions(+), 172 deletions(-) create mode 100644 lib/marker/marker.go create mode 100644 lib/marker/marker_test.go diff --git a/commands/msg/copy.go b/commands/msg/copy.go index a68a22a..de70c6f 100644 --- a/commands/msg/copy.go +++ b/commands/msg/copy.go @@ -59,7 +59,9 @@ func (Copy) Execute(aerc *widgets.Aerc, args []string) error { switch msg := msg.(type) { case *types.Done: aerc.PushStatus("Messages copied.", 10*time.Second) - store.ClearVisualMark() + if m, err := store.Marker(); err == nil { + m.ClearVisualMark() + } case *types.Error: aerc.PushError(msg.Error.Error()) } diff --git a/commands/msg/mark.go b/commands/msg/mark.go index c446fc6..ef37d6e 100644 --- a/commands/msg/mark.go +++ b/commands/msg/mark.go @@ -31,6 +31,10 @@ func (Mark) Execute(aerc *widgets.Aerc, args []string) error { if err != nil { return err } + marker, err := store.Marker() + if err != nil { + return err + } opts, _, err := getopt.Getopts(args, "atv") if err != nil { return err @@ -57,9 +61,9 @@ func (Mark) Execute(aerc *widgets.Aerc, args []string) error { var modFunc func(uint32) if toggle { - modFunc = store.ToggleMark + modFunc = marker.ToggleMark } else { - modFunc = store.Mark + modFunc = marker.Mark } if all { uids := store.Uids() @@ -68,7 +72,7 @@ func (Mark) Execute(aerc *widgets.Aerc, args []string) error { } return nil } else if visual { - store.ToggleVisualMark() + marker.ToggleVisualMark() return nil } else { modFunc(selected.Uid) @@ -83,21 +87,21 @@ func (Mark) Execute(aerc *widgets.Aerc, args []string) error { if all && toggle { uids := store.Uids() for _, uid := range uids { - store.ToggleMark(uid) + marker.ToggleMark(uid) } return nil } else if all && !toggle { - store.ClearVisualMark() + marker.ClearVisualMark() return nil } else { - store.Unmark(selected.Uid) + marker.Unmark(selected.Uid) return nil } case "remark": if all || visual || toggle { return fmt.Errorf("Usage: :remark") } - store.Remark() + marker.Remark() return nil } return nil // never reached diff --git a/commands/msg/modify-labels.go b/commands/msg/modify-labels.go index a3b4900..f62003a 100644 --- a/commands/msg/modify-labels.go +++ b/commands/msg/modify-labels.go @@ -57,7 +57,9 @@ func (ModifyLabels) Execute(aerc *widgets.Aerc, args []string) error { switch msg := msg.(type) { case *types.Done: aerc.PushStatus("labels updated", 10*time.Second) - store.ClearVisualMark() + if m, err := store.Marker(); err == nil { + m.ClearVisualMark() + } case *types.Error: aerc.PushError(msg.Error.Error()) } diff --git a/commands/msg/pipe.go b/commands/msg/pipe.go index 35ee05b..8dec243 100644 --- a/commands/msg/pipe.go +++ b/commands/msg/pipe.go @@ -201,7 +201,9 @@ func (Pipe) Execute(aerc *widgets.Aerc, args []string) error { } }) } - provider.Store().ClearVisualMark() + if m, err := provider.Store().Marker(); err == nil { + m.ClearVisualMark() + } return nil } diff --git a/commands/msg/read.go b/commands/msg/read.go index c6bcd0d..c23200e 100644 --- a/commands/msg/read.go +++ b/commands/msg/read.go @@ -175,7 +175,9 @@ func (FlagMsg) Execute(aerc *widgets.Aerc, args []string) error { wg.Wait() if success { aerc.PushStatus(actionName+" flag '"+flagName+"' successful", 10*time.Second) - store.ClearVisualMark() + if m, err := store.Marker(); err == nil { + m.ClearVisualMark() + } } }() diff --git a/lib/marker/marker.go b/lib/marker/marker.go new file mode 100644 index 0000000..ad16ace --- /dev/null +++ b/lib/marker/marker.go @@ -0,0 +1,188 @@ +package marker + +import "fmt" + +// TODO: fix headers for message that are nil + +// Marker provides the interface for the marking behavior of messages +type Marker interface { + Mark(uint32) + Unmark(uint32) + ToggleMark(uint32) + Remark() + Marked() []uint32 + IsMarked(uint32) bool + ToggleVisualMark() + UpdateVisualMark() + ClearVisualMark() +} + +// UIDProvider provides the underlying uids and the selected message index +type UIDProvider interface { + Uids() []uint32 + SelectedIndex() int +} + +type controller struct { + uidProvider UIDProvider + marked map[uint32]struct{} + lastMarked map[uint32]struct{} + visualStartUID uint32 + visualMarkMode bool +} + +// New returns a new Marker +func New(up UIDProvider) (Marker, error) { + if up == nil { + return nil, fmt.Errorf("marker: no uid provider available") + } + return &controller{ + uidProvider: up, + marked: make(map[uint32]struct{}), + lastMarked: make(map[uint32]struct{}), + }, nil +} + +// Mark markes the uid as marked +func (mc *controller) Mark(uid uint32) { + if mc.visualMarkMode { + // visual mode has override, bogus input from user + return + } + mc.marked[uid] = struct{}{} +} + +// Unmark unmarks the uid +func (mc *controller) Unmark(uid uint32) { + if mc.visualMarkMode { + // user probably wanted to clear the visual marking + mc.ClearVisualMark() + return + } + delete(mc.marked, uid) +} + +// Remark restores the previous marks +func (mc *controller) Remark() { + mc.marked = mc.lastMarked +} + +// ToggleMark toggles the marked state for the given uid +func (mc *controller) ToggleMark(uid uint32) { + if mc.visualMarkMode { + // visual mode has override, bogus input from user + return + } + if mc.IsMarked(uid) { + mc.Unmark(uid) + } else { + mc.Mark(uid) + } +} + +// resetMark removes the marking from all messages +func (mc *controller) resetMark() { + mc.lastMarked = mc.marked + mc.marked = make(map[uint32]struct{}) +} + +// removeStaleUID removes uids that are no longer presents in the UIDProvider +func (mc *controller) removeStaleUID() { + for mark := range mc.marked { + present := false + for _, uid := range mc.uidProvider.Uids() { + if mark == uid { + present = true + break + } + } + if !present { + delete(mc.marked, mark) + } + } +} + +//IsMarked checks whether the given uid has been marked +func (mc *controller) IsMarked(uid uint32) bool { + _, marked := mc.marked[uid] + return marked +} + +// Marked returns the uids of all marked messages +func (mc *controller) Marked() []uint32 { + mc.removeStaleUID() + marked := make([]uint32, len(mc.marked)) + i := 0 + for uid := range mc.marked { + marked[i] = uid + i++ + } + return marked +} + +//ToggleVisualMark enters or leaves the visual marking mode +func (mc *controller) ToggleVisualMark() { + mc.visualMarkMode = !mc.visualMarkMode + if mc.visualMarkMode { + // just entered visual mode, reset whatever marking was already done + mc.resetMark() + uids := mc.uidProvider.Uids() + if idx := mc.uidProvider.SelectedIndex(); idx >= 0 && idx < len(uids) { + mc.visualStartUID = uids[idx] + mc.marked[mc.visualStartUID] = struct{}{} + } + } else { + // visual mode ended, nothing to do + } + return +} + +//ClearVisualMark leaves the visual marking mode and resets any marking +func (mc *controller) ClearVisualMark() { + mc.resetMark() + mc.visualMarkMode = false + mc.visualStartUID = 0 +} + +// UpdateVisualMark updates the index with the currently selected message +func (mc *controller) UpdateVisualMark() { + if !mc.visualMarkMode { + // nothing to do + return + } + startIdx := mc.visualStartIdx() + if startIdx < 0 { + // something deleted the startuid, abort the marking process + mc.ClearVisualMark() + return + } + + selectedIdx := mc.uidProvider.SelectedIndex() + if selectedIdx < 0 { + mc.ClearVisualMark() + return + } + + uids := mc.uidProvider.Uids() + + var visUids []uint32 + if selectedIdx > startIdx { + visUids = uids[startIdx : selectedIdx+1] + } else { + visUids = uids[selectedIdx : startIdx+1] + } + mc.resetMark() + for _, uid := range visUids { + mc.marked[uid] = struct{}{} + } +} + +// returns the index of needle in haystack or -1 if not found +func (mc *controller) visualStartIdx() int { + for idx, u := range mc.uidProvider.Uids() { + if u == mc.visualStartUID { + return idx + } + } + return -1 +} diff --git a/lib/marker/marker_test.go b/lib/marker/marker_test.go new file mode 100644 index 0000000..c76d1f8 --- /dev/null +++ b/lib/marker/marker_test.go @@ -0,0 +1,140 @@ +package marker_test + +import ( + "testing" + + "git.sr.ht/~rjarry/aerc/lib/marker" +) + +// mockUidProvider implements the UidProvider interface and mocks the message +// store for testing +type mockUidProvider struct { + uids []uint32 + idx int +} + +func (mock *mockUidProvider) Uids() []uint32 { + return mock.uids +} + +func (mock *mockUidProvider) SelectedIndex() int { + return mock.idx +} + +func createMarker() (marker.Marker, *mockUidProvider) { + uidProvider := &mockUidProvider{ + uids: []uint32{1, 2, 3, 4}, + idx: 1, + } + m, _ := marker.New(uidProvider) + return m, uidProvider +} + +func TestMarker_MarkUnmark(t *testing.T) { + m, _ := createMarker() + uid := uint32(4) + + m.Mark(uid) + if !m.IsMarked(uid) { + t.Errorf("Marking failed") + } + + m.Unmark(uid) + if m.IsMarked(uid) { + t.Errorf("Unmarking failed") + } +} + +func TestMarker_ToggleMark(t *testing.T) { + m, _ := createMarker() + uid := uint32(4) + + if m.IsMarked(uid) { + t.Errorf("ToggleMark: uid should not be marked") + } + + m.ToggleMark(uid) + if !m.IsMarked(uid) { + t.Errorf("ToggleMark: uid should be marked") + } + + m.ToggleMark(uid) + if m.IsMarked(uid) { + t.Errorf("ToggleMark: uid should not be marked") + } +} + +func TestMarker_Marked(t *testing.T) { + m, _ := createMarker() + expected := map[uint32]struct{}{ + uint32(1): struct{}{}, + uint32(4): struct{}{}, + } + for uid := range expected { + m.Mark(uid) + } + + got := m.Marked() + if len(expected) != len(got) { + t.Errorf("Marked: expected len of %d but got %d", len(expected), len(got)) + } + + for _, uid := range got { + if _, ok := expected[uid]; !ok { + t.Errorf("Marked: received uid %d as marked but it should not be", uid) + } + } +} + +func TestMarker_VisualMode(t *testing.T) { + m, up := createMarker() + + // activate visual mode + m.ToggleVisualMark() + + // marking should now fail silently because we're in visual mode + m.Mark(1) + if m.IsMarked(1) { + t.Errorf("marking in visual mode should not work") + } + + // move selection index to last item + up.idx = len(up.uids) - 1 + m.UpdateVisualMark() + expectedMarked := []uint32{2, 3, 4} + + for _, uidMarked := range expectedMarked { + if !m.IsMarked(uidMarked) { + t.Logf("expected: %#v, got: %#v", expectedMarked, m.Marked()) + t.Errorf("updatevisual: uid %v should be marked in visual mode", uidMarked) + } + } + + // clear all + m.ClearVisualMark() + if len(m.Marked()) > 0 { + t.Errorf("no uids should be marked after clearing visual mark") + } + + // test remark + m.Remark() + for _, uidMarked := range expectedMarked { + if !m.IsMarked(uidMarked) { + t.Errorf("remark: uid %v should be marked in visual mode", uidMarked) + } + } + +} + +func TestMarker_MarkOutOfBound(t *testing.T) { + m, _ := createMarker() + + outOfBoundUid := uint32(100) + + m.Mark(outOfBoundUid) + for _, markedUid := range m.Marked() { + if markedUid == outOfBoundUid { + t.Errorf("out-of-bound uid should not be marked") + } + } +} diff --git a/lib/msgstore.go b/lib/msgstore.go index d9140d7..6776821 100644 --- a/lib/msgstore.go +++ b/lib/msgstore.go @@ -1,10 +1,12 @@ package lib import ( + "fmt" "io" "sync" "time" + "git.sr.ht/~rjarry/aerc/lib/marker" "git.sr.ht/~rjarry/aerc/lib/sort" "git.sr.ht/~rjarry/aerc/logging" "git.sr.ht/~rjarry/aerc/models" @@ -27,11 +29,8 @@ type MessageStore struct { bodyCallbacks map[uint32][]func(*types.FullMessage) headerCallbacks map[uint32][]func(*types.MessageInfo) - //marking - marked map[uint32]struct{} - lastMarked map[uint32]struct{} - visualStartUid uint32 - visualMarkMode bool + // marking + marker marker.Marker // Search/filter results results []uint32 @@ -79,8 +78,8 @@ func NewMessageStore(worker *types.Worker, DirInfo: *dirInfo, Messages: make(map[uint32]*models.MessageInfo), - selectedUid: MagicUid, - marked: make(map[uint32]struct{}), + selectedUid: MagicUid, + bodyCallbacks: make(map[uint32][]func(*types.FullMessage)), headerCallbacks: make(map[uint32][]func(*types.MessageInfo)), @@ -219,7 +218,6 @@ func (store *MessageStore) Update(msg types.WorkerMessage) { } store.Messages = newMap store.uids = msg.Uids - store.checkMark() update = true case *types.DirectoryThreaded: var uids []uint32 @@ -240,7 +238,6 @@ func (store *MessageStore) Update(msg types.WorkerMessage) { } store.Messages = newMap store.uids = uids - store.checkMark() store.threads = msg.Threads update = true case *types.MessageInfo: @@ -296,7 +293,6 @@ func (store *MessageStore) Update(msg types.WorkerMessage) { toDelete[uid] = nil delete(store.Messages, uid) delete(store.Deleted, uid) - delete(store.marked, uid) } uids := make([]uint32, len(store.uids)-len(msg.Uids)) j := 0 @@ -547,144 +543,11 @@ func (store *MessageStore) SelectedUid() uint32 { func (store *MessageStore) Select(uid uint32) { store.selectedUid = uid - store.updateVisual() -} - -// Mark sets the marked state on a MessageInfo -func (store *MessageStore) Mark(uid uint32) { - if store.visualMarkMode { - // visual mode has override, bogus input from user - return - } - store.marked[uid] = struct{}{} -} - -// Unmark removes the marked state on a MessageInfo -func (store *MessageStore) Unmark(uid uint32) { - if store.visualMarkMode { - // user probably wanted to clear the visual marking - store.ClearVisualMark() - return - } - delete(store.marked, uid) -} - -func (store *MessageStore) Remark() { - store.marked = store.lastMarked -} - -// ToggleMark toggles the marked state on a MessageInfo -func (store *MessageStore) ToggleMark(uid uint32) { - if store.visualMarkMode { - // visual mode has override, bogus input from user - return - } - if store.IsMarked(uid) { - store.Unmark(uid) - } else { - store.Mark(uid) - } -} - -// resetMark removes the marking from all messages -func (store *MessageStore) resetMark() { - store.lastMarked = store.marked - store.marked = make(map[uint32]struct{}) -} - -// checkMark checks that no stale uids remain marked -func (store *MessageStore) checkMark() { - for mark := range store.marked { - present := false - for _, uid := range store.uids { - if mark == uid { - present = true - break - } - } - if !present { - delete(store.marked, mark) - } - } -} - -//IsMarked checks whether a MessageInfo has been marked -func (store *MessageStore) IsMarked(uid uint32) bool { - _, marked := store.marked[uid] - return marked -} - -//ToggleVisualMark enters or leaves the visual marking mode -func (store *MessageStore) ToggleVisualMark() { - store.visualMarkMode = !store.visualMarkMode - switch store.visualMarkMode { - case true: - // just entered visual mode, reset whatever marking was already done - store.resetMark() - store.visualStartUid = store.Selected().Uid - store.marked[store.visualStartUid] = struct{}{} - case false: - // visual mode ended, nothing to do - return + if store.marker != nil { + store.marker.UpdateVisualMark() } } -//ClearVisualMark leaves the visual marking mode and resets any marking -func (store *MessageStore) ClearVisualMark() { - store.resetMark() - store.visualMarkMode = false - store.visualStartUid = 0 -} - -// Marked returns the uids of all marked messages -func (store *MessageStore) Marked() []uint32 { - marked := make([]uint32, len(store.marked)) - i := 0 - for uid := range store.marked { - marked[i] = uid - i++ - } - return marked -} - -func (store *MessageStore) updateVisual() { - if !store.visualMarkMode { - // nothing to do - return - } - startIdx := store.visualStartIdx() - if startIdx < 0 { - // something deleted the startuid, abort the marking process - store.ClearVisualMark() - return - } - - selectedIdx := store.FindIndexByUid(store.SelectedUid()) - if selectedIdx < 0 { - store.ClearVisualMark() - return - } - - var visUids []uint32 - if selectedIdx > startIdx { - visUids = store.Uids()[startIdx : selectedIdx+1] - } else { - visUids = store.Uids()[selectedIdx : startIdx+1] - } - - store.resetMark() - for _, uid := range visUids { - store.marked[uid] = struct{}{} - } - missing := make([]uint32, 0) - for _, uid := range visUids { - if msg := store.Messages[uid]; msg == nil { - missing = append(missing, uid) - } - } - store.FetchHeaders(missing, nil) -} - func (store *MessageStore) NextPrev(delta int) { uids := store.Uids() if len(uids) == 0 { @@ -707,7 +570,9 @@ func (store *MessageStore) NextPrev(delta int) { store.Select(uids[newIdx]) - store.updateVisual() + if store.marker != nil { + store.marker.UpdateVisualMark() + } nextResultIndex := len(store.results) - store.resultIndex - 2*delta if nextResultIndex < 0 || nextResultIndex >= len(store.results) { @@ -828,14 +693,15 @@ func (store *MessageStore) GetCurrentSortCriteria() []*types.SortCriterion { return store.sortCriteria } -// returns the index of needle in haystack or -1 if not found -func (store *MessageStore) visualStartIdx() int { - for idx, u := range store.Uids() { - if u == store.visualStartUid { - return idx - } +func (store *MessageStore) SetMarker(m marker.Marker) { + store.marker = m +} + +func (store *MessageStore) Marker() (marker.Marker, error) { + if store.marker == nil { + return nil, fmt.Errorf("no marker set in message store") } - return -1 + return store.marker, nil } // FindIndexByUid returns the index in store.Uids() or -1 if not found @@ -852,3 +718,9 @@ func (store *MessageStore) FindIndexByUid(uid uint32) int { func (store *MessageStore) Capabilities() *models.Capabilities { return store.DirInfo.Caps } + +// SelectedIndex returns the index of the selected message in the uid list or +// -1 if not found +func (store *MessageStore) SelectedIndex() int { + return store.FindIndexByUid(store.selectedUid) +} diff --git a/widgets/account.go b/widgets/account.go index 50b0fab..eb6a63b 100644 --- a/widgets/account.go +++ b/widgets/account.go @@ -9,6 +9,7 @@ import ( "git.sr.ht/~rjarry/aerc/config" "git.sr.ht/~rjarry/aerc/lib" + "git.sr.ht/~rjarry/aerc/lib/marker" "git.sr.ht/~rjarry/aerc/lib/sort" "git.sr.ht/~rjarry/aerc/lib/statusline" "git.sr.ht/~rjarry/aerc/lib/ui" @@ -218,8 +219,12 @@ func (acct *AccountView) SelectedMessage() (*models.MessageInfo, error) { } func (acct *AccountView) MarkedMessages() ([]uint32, error) { - store := acct.Store() - return store.Marked(), nil + if store := acct.Store(); store != nil { + if marker, err := store.Marker(); marker != nil && err == nil { + return marker.Marked(), nil + } + } + return nil, errors.New("no store available") } func (acct *AccountView) SelectedMessagePart() *PartInfo { @@ -297,6 +302,9 @@ func (acct *AccountView) onMessage(msg types.WorkerMessage) { acct.host.Beep() } }) + if marker, err := marker.New(store); err == nil { + store.SetMarker(marker) + } acct.dirlist.SetMsgStore(msg.Info.Name, store) } case *types.DirectoryContents: diff --git a/widgets/msglist.go b/widgets/msglist.go index 23eb410..2568e5b 100644 --- a/widgets/msglist.go +++ b/widgets/msglist.go @@ -87,6 +87,16 @@ func (ml *MessageList) Draw(ctx *ui.Context) { row int = 0 ) + isMarked := func(uid uint32) bool { + return false + } + marker, err := store.Marker() + if err == nil { + isMarked = func(uid uint32) bool { + return marker.IsMarked(uid) + } + } + if store.ThreadedView() { threads := store.Threads() counter := len(store.Uids()) @@ -121,7 +131,7 @@ func (ml *MessageList) Draw(ctx *ui.Context) { AccountName: acct.Name(), MsgInfo: msg, MsgNum: row, - MsgIsMarked: store.IsMarked(t.Uid), + MsgIsMarked: isMarked(t.Uid), ThreadPrefix: prefix, ThreadSameSubject: normalizedSubject == lastSubject, } @@ -146,7 +156,7 @@ func (ml *MessageList) Draw(ctx *ui.Context) { AccountName: acct.Name(), MsgInfo: msg, MsgNum: row, - MsgIsMarked: store.IsMarked(uid), + MsgIsMarked: isMarked(uid), } if ml.drawRow(textWidth, ctx, uid, row, &needsHeaders, fmtCtx) { break @@ -240,8 +250,10 @@ func (ml *MessageList) drawRow(textWidth int, ctx *ui.Context, uid uint32, row i } // marked message - if store.IsMarked(msg.Uid) { - msg_styles = append(msg_styles, config.STYLE_MSGLIST_MARKED) + if marker, err := store.Marker(); err == nil { + if marker.IsMarked(msg.Uid) { + msg_styles = append(msg_styles, config.STYLE_MSGLIST_MARKED) + } } var style tcell.Style diff --git a/widgets/msgviewer.go b/widgets/msgviewer.go index eec786c..54774b3 100644 --- a/widgets/msgviewer.go +++ b/widgets/msgviewer.go @@ -302,8 +302,7 @@ func (mv *MessageViewer) SelectedMessage() (*models.MessageInfo, error) { } func (mv *MessageViewer) MarkedMessages() ([]uint32, error) { - store := mv.Store() - return store.Marked(), nil + return mv.acct.MarkedMessages() } func (mv *MessageViewer) ToggleHeaders() { -- 2.36.1
The message store keeps a map of callbacks for headers that are being fetched. This is not used anywhere in the code base. Instead of a func(*types.MessageInfo) callback use a general func(types.WorkerMessage) in the worker.PostAction function to make it more useful. This callback allows now to get a feedback when all headers are fetched successfully. Note that the pending header map remains so that the same header is not fetched multiple times. Signed-off-by: Koni Marti <koni.marti@gmail.com> --- v2->v3: * rebased on current master lib/msgstore.go | 28 ++++++++-------------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/lib/msgstore.go b/lib/msgstore.go index 6776821..55eec7e 100644 --- a/lib/msgstore.go +++ b/lib/msgstore.go @@ -24,10 +24,9 @@ type MessageStore struct { uids []uint32 threads []*types.Thread - selectedUid uint32 - reselect *models.MessageInfo - bodyCallbacks map[uint32][]func(*types.FullMessage) - headerCallbacks map[uint32][]func(*types.MessageInfo) + selectedUid uint32 + + bodyCallbacks map[uint32][]func(*types.FullMessage) // marking marker marker.Marker @@ -80,8 +79,7 @@ func NewMessageStore(worker *types.Worker, selectedUid: MagicUid, - bodyCallbacks: make(map[uint32][]func(*types.FullMessage)), - headerCallbacks: make(map[uint32][]func(*types.MessageInfo)), + bodyCallbacks: make(map[uint32][]func(*types.FullMessage)), threadedView: thread, buildThreads: clientThreads, @@ -101,7 +99,7 @@ func NewMessageStore(worker *types.Worker, } func (store *MessageStore) FetchHeaders(uids []uint32, - cb func(*types.MessageInfo)) { + cb func(types.WorkerMessage)) { // TODO: this could be optimized by pre-allocating toFetch and trimming it // at the end. In practice we expect to get most messages back in one frame. @@ -110,13 +108,6 @@ func (store *MessageStore) FetchHeaders(uids []uint32, if _, ok := store.pendingHeaders[uid]; !ok { toFetch = append(toFetch, uid) store.pendingHeaders[uid] = nil - if cb != nil { - if list, ok := store.headerCallbacks[uid]; ok { - store.headerCallbacks[uid] = append(list, cb) - } else { - store.headerCallbacks[uid] = []func(*types.MessageInfo){cb} - } - } } } if len(toFetch) > 0 { @@ -125,9 +116,11 @@ func (store *MessageStore) FetchHeaders(uids []uint32, case *types.Error: for _, uid := range toFetch { delete(store.pendingHeaders, uid) - delete(store.headerCallbacks, uid) } } + if cb != nil { + cb(msg) + } }) } } @@ -262,11 +255,6 @@ func (store *MessageStore) Update(msg types.WorkerMessage) { } if _, ok := store.pendingHeaders[msg.Info.Uid]; msg.Info.Envelope != nil && ok { delete(store.pendingHeaders, msg.Info.Uid) - if cbs, ok := store.headerCallbacks[msg.Info.Uid]; ok { - for _, cb := range cbs { - cb(msg) - } - } } if store.builder != nil { store.builder.Update(msg.Info) -- 2.36.1
Fix large archive operations that covers messages in the store with unfetched headers. Commit e5ad877af562 ("msgstore: fetch missing headers in visual mode") fixed this for the visual selection mode but omitted the case when 'mark -a' is used to mark all messages. Signed-off-by: Koni Marti <koni.marti@gmail.com> --- v2->v3: * no changes commands/msg/utils.go | 11 +++++++++-- commands/util.go | 21 +++++++++++++++++++-- lib/marker/marker.go | 2 -- 3 files changed, 28 insertions(+), 6 deletions(-) diff --git a/commands/msg/utils.go b/commands/msg/utils.go index 4ce82a7..8a00a35 100644 --- a/commands/msg/utils.go +++ b/commands/msg/utils.go @@ -2,6 +2,7 @@ package msg import ( "errors" + "time" "git.sr.ht/~rjarry/aerc/commands" "git.sr.ht/~rjarry/aerc/lib" @@ -11,10 +12,16 @@ import ( type helper struct { msgProvider widgets.ProvidesMessages + statusInfo func(string) } func newHelper(aerc *widgets.Aerc) *helper { - return &helper{aerc.SelectedTabContent().(widgets.ProvidesMessages)} + return &helper{ + msgProvider: aerc.SelectedTabContent().(widgets.ProvidesMessages), + statusInfo: func(s string) { + aerc.PushStatus(s, 10*time.Second) + }, + } } func (h *helper) markedOrSelectedUids() ([]uint32, error) { @@ -46,5 +53,5 @@ func (h *helper) messages() ([]*models.MessageInfo, error) { if err != nil { return nil, err } - return commands.MsgInfoFromUids(store, uid) + return commands.MsgInfoFromUids(store, uid, h.statusInfo) } diff --git a/commands/util.go b/commands/util.go index 1c4a8c9..b14e969 100644 --- a/commands/util.go +++ b/commands/util.go @@ -16,6 +16,7 @@ import ( "git.sr.ht/~rjarry/aerc/logging" "git.sr.ht/~rjarry/aerc/models" "git.sr.ht/~rjarry/aerc/widgets" + "git.sr.ht/~rjarry/aerc/worker/types" "github.com/gdamore/tcell/v2" "github.com/mitchellh/go-homedir" ) @@ -185,8 +186,9 @@ func UidsFromMessageInfos(msgs []*models.MessageInfo) []uint32 { return uids } -func MsgInfoFromUids(store *lib.MessageStore, uids []uint32) ([]*models.MessageInfo, error) { +func MsgInfoFromUids(store *lib.MessageStore, uids []uint32, statusInfo func(string)) ([]*models.MessageInfo, error) { infos := make([]*models.MessageInfo, len(uids)) + needHeaders := make([]uint32, 0) for i, uid := range uids { var ok bool infos[i], ok = store.Messages[uid] @@ -194,9 +196,24 @@ func MsgInfoFromUids(store *lib.MessageStore, uids []uint32) ([]*models.MessageI return nil, fmt.Errorf("uid not found") } if infos[i] == nil { - return nil, fmt.Errorf("message store not ready yet") + needHeaders = append(needHeaders, uid) } } + if len(needHeaders) > 0 { + store.FetchHeaders(needHeaders, func(msg types.WorkerMessage) { + var info string + switch m := msg.(type) { + case *types.Done: + info = "All headers fetched. Please repeat command." + case *types.Error: + info = fmt.Sprintf("Encountered error while fetching headers: %v", m.Error) + } + if statusInfo != nil { + statusInfo(info) + } + }) + return nil, fmt.Errorf("Fetching missing message headers. Please wait.") + } return infos, nil } diff --git a/lib/marker/marker.go b/lib/marker/marker.go index ad16ace..7bab9c2 100644 --- a/lib/marker/marker.go +++ b/lib/marker/marker.go @@ -2,8 +2,6 @@ package marker import "fmt" -// TODO: fix headers for message that are nil - // Marker provides the interface for the marking behavior of messages type Marker interface { Mark(uint32) -- 2.36.1
When entering visual selection mode, the current selection is deleted. This patch extends the visual mode behavior to select multiple blocks of messages. Signed-off-by: Koni Marti <koni.marti@gmail.com>
testing on my end reveals that the visual selector fails to maintain the marked messages when scrolling in visual mode through a series of messages in the same thread with threading enabled.
--- v2->v3: * rebased on current master lib/marker/marker.go | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/marker/marker.go b/lib/marker/marker.go index 7bab9c2..4d08519 100644 --- a/lib/marker/marker.go +++ b/lib/marker/marker.go @@ -27,6 +27,7 @@ type controller struct { lastMarked map[uint32]struct{} visualStartUID uint32 visualMarkMode bool + visualBase map[uint32]struct{} } // New returns a new Marker @@ -123,11 +124,14 @@ func (mc *controller) ToggleVisualMark() { mc.visualMarkMode = !mc.visualMarkMode if mc.visualMarkMode { // just entered visual mode, reset whatever marking was already done - mc.resetMark() uids := mc.uidProvider.Uids() if idx := mc.uidProvider.SelectedIndex(); idx >= 0 && idx < len(uids) { mc.visualStartUID = uids[idx] mc.marked[mc.visualStartUID] = struct{}{} + mc.visualBase = make(map[uint32]struct{}) + for key, value := range mc.marked { + mc.visualBase[key] = value + } } } else { // visual mode ended, nothing to do @@ -169,7 +173,10 @@ func (mc *controller) UpdateVisualMark() { } else { visUids = uids[selectedIdx : startIdx+1] } - mc.resetMark() + mc.marked = make(map[uint32]struct{}) + for uid := range mc.visualBase { + mc.marked[uid] = struct{}{} + } for _, uid := range visUids { mc.marked[uid] = struct{}{} } -- 2.36.1
builds.sr.ht <builds@sr.ht>aerc/patches: SUCCESS in 7m2s [store: extract marking behavior and add tests][0] v3 from [Koni Marti][1] [0]: https://lists.sr.ht/~rjarry/aerc-devel/patches/34231 [1]: mailto:koni.marti@gmail.com ✓ #810631 SUCCESS aerc/patches/openbsd.yml https://builds.sr.ht/~rjarry/job/810631 ✓ #810630 SUCCESS aerc/patches/alpine-edge.yml https://builds.sr.ht/~rjarry/job/810630
Mark or unmark the shown message threads. Threads must be available in the message store. Use the -T option for the mark or unmark commands. Can be used in combination with the toggle flag (-t). Signed-off-by: Koni Marti <koni.marti@gmail.com> --- v2->v3: * added in v3 commands/msg/mark.go | 35 +++++++++++++++++++++++++++++++---- doc/aerc.1.scd | 4 +++- lib/msgstore.go | 19 +++++++++++++++++++ worker/types/thread.go | 24 ++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 5 deletions(-) diff --git a/commands/msg/mark.go b/commands/msg/mark.go index ef37d6e..5de823d 100644 --- a/commands/msg/mark.go +++ b/commands/msg/mark.go @@ -35,13 +35,14 @@ func (Mark) Execute(aerc *widgets.Aerc, args []string) error { if err != nil { return err } - opts, _, err := getopt.Getopts(args, "atv") + opts, _, err := getopt.Getopts(args, "atvT") if err != nil { return err } var all bool var toggle bool var visual bool + var thread bool for _, opt := range opts { switch opt.Option { case 'a': @@ -50,9 +51,23 @@ func (Mark) Execute(aerc *widgets.Aerc, args []string) error { visual = true case 't': toggle = true + case 'T': + thread = true } } + if thread && len(store.Threads()) == 0 { + return fmt.Errorf("No threads found") + } + + if thread && all { + return fmt.Errorf("-a and -T are mutually exclusive") + } + + if thread && visual { + return fmt.Errorf("-v and -T are mutually exclusive") + } + switch args[0] { case "mark": if all && visual { @@ -75,7 +90,13 @@ func (Mark) Execute(aerc *widgets.Aerc, args []string) error { marker.ToggleVisualMark() return nil } else { - modFunc(selected.Uid) + if thread { + for _, uid := range store.SelectedThread().Root().Uids() { + modFunc(uid) + } + } else { + modFunc(selected.Uid) + } return nil } @@ -94,11 +115,17 @@ func (Mark) Execute(aerc *widgets.Aerc, args []string) error { marker.ClearVisualMark() return nil } else { - marker.Unmark(selected.Uid) + if thread { + for _, uid := range store.SelectedThread().Root().Uids() { + marker.Unmark(uid) + } + } else { + marker.Unmark(selected.Uid) + } return nil } case "remark": - if all || visual || toggle { + if all || visual || toggle || thread { return fmt.Errorf("Usage: :remark") } marker.Remark() diff --git a/doc/aerc.1.scd b/doc/aerc.1.scd index b06dd43..8058d8c 100644 --- a/doc/aerc.1.scd +++ b/doc/aerc.1.scd @@ -395,7 +395,7 @@ message list, the message in the message viewer, etc). *-a*: Save all attachments. Individual filenames cannot be specified. -*mark* [-atv] +*mark* [-atvT] Marks messages. Commands will execute on all marked messages instead of the highlighted one if applicable. The flags below can be combined as needed. @@ -405,6 +405,8 @@ message list, the message in the message viewer, etc). *-v*: Enter / leave visual mark mode + *-T*: marks the shown message thread of the selected message. + *unmark* [-at] Unmarks messages. The flags below can be combined as needed. diff --git a/lib/msgstore.go b/lib/msgstore.go index 55eec7e..a1e550c 100644 --- a/lib/msgstore.go +++ b/lib/msgstore.go @@ -419,6 +419,25 @@ func (store *MessageStore) runThreadBuilder() { }) } +// SelectedThread returns the thread with the UID from the selected message +func (store *MessageStore) SelectedThread() *types.Thread { + var thread *types.Thread + for _, root := range store.Threads() { + found := false + root.Walk(func(t *types.Thread, _ int, _ error) error { + if t.Uid == store.SelectedUid() { + thread = t + found = true + } + return nil + }) + if found { + break + } + } + return thread +} + func (store *MessageStore) Delete(uids []uint32, cb func(msg types.WorkerMessage)) { diff --git a/worker/types/thread.go b/worker/types/thread.go index 7c0cc5b..fe1470e 100644 --- a/worker/types/thread.go +++ b/worker/types/thread.go @@ -48,6 +48,30 @@ func (t *Thread) Walk(walkFn NewThreadWalkFn) error { return err } +// Root returns the root thread of the thread tree +func (t *Thread) Root() *Thread { + if t == nil { + return nil + } + var iter *Thread + for iter = t; iter.Parent != nil; iter = iter.Parent { + } + return iter +} + +// Uids returns all associated uids for the given thread and its children +func (t *Thread) Uids() []uint32 { + if t == nil { + return nil + } + uids := make([]uint32, 0) + t.Walk(func(node *Thread, _ int, _ error) error { + uids = append(uids, node.Uid) + return nil + }) + return uids +} + func (t *Thread) String() string { if t == nil { return "<nil>" -- 2.36.1