diff --git a/README.md b/README.md index b56e762..74299d6 100644 --- a/README.md +++ b/README.md @@ -337,8 +337,9 @@ Thread categories are: New threads retain a `NEW THREAD` marker. When an existing thread receives new comments, diple places a `NEW MESSAGES` divider before the first unread comment and emphasizes the unread comment rail. Moving the thread-list cursor does not -clear this state. It is cleared after the last unread comment becomes visible -while scrolling the focused detail pane, or manually with +clear this state. It is cleared when the thread detail pane receives focus, when +the thread is resolved, after the last unread comment becomes visible while +scrolling the focused detail pane, or manually with `keybindings.threads.mark_read` (`m` by default). Within a category, `"file"` keeps paths together and `"timestamp"` sorts by diff --git a/tui.go b/tui.go index 45b78ea..5f6c724 100644 --- a/tui.go +++ b/tui.go @@ -415,7 +415,7 @@ func (m *App) startReply() { m.writeMode, m.writeThreadID, m.replyDraft, m.err = writeReply, thread.ID, "", nil m.restoreReplyDraft(thread.ID) m.folded[thread.ID] = false - m.focus = threadDetailPane + m.focusThreadDetail() m.scroll = m.detailMaxScroll() } @@ -960,6 +960,9 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.folded[thread.ID] = thread.IsResolved && m.foldResolved break } + if msg.thread.IsResolved { + m.markThreadRead(msg.threadID) + } sortReviewThreads(m.details.Threads, m.threadStatusOrder, m.threadWithinStatus) m.threadIndex = indexThread(m.details.Threads, selected) m.writeThreadID = "" @@ -1247,7 +1250,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if m.screen == threadScreen { m.listHidden = !m.listHidden if m.listHidden { - m.focus = threadDetailPane + m.focusThreadDetail() } else { m.focus = threadListPane } @@ -1290,7 +1293,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } if m.screen == threadScreen { - m.focus = threadDetailPane + m.focusThreadDetail() } case "h": if m.screen == threadScreen { @@ -1480,29 +1483,53 @@ func (m *App) trackThreadUpdates(details PRDetails) { } func (m *App) markCurrentThreadRead() { - if m.threadIndex >= 0 && m.threadIndex < len(m.details.Threads) { - thread := m.details.Threads[m.threadIndex] - delete(m.unreadThreads, thread.ID) - delete(m.newThreads, thread.ID) - prID := m.currentPRKey() - state := m.readState.Data[prID] - if state.Threads == nil { - state.Threads = make(map[string]bool) - } - if state.Comments == nil { - state.Comments = make(map[string]bool) - } - state.Initialized = true - state.Threads[thread.ID] = true - for _, comment := range thread.Comments { - state.Comments[comment.ID] = true - delete(m.unreadComments, comment.ID) - } - m.readState.Data[prID] = state - if err := m.readState.save(); err != nil { - m.recordHealth("read state", healthWarning, err.Error()) + if m.threadIndex < 0 || m.threadIndex >= len(m.details.Threads) { + return + } + m.markThreadRead(m.details.Threads[m.threadIndex].ID) +} + +func (m *App) markThreadRead(threadID string) { + thread := m.threadByID(threadID) + if thread == nil { + return + } + prID := m.currentPRKey() + state := m.readState.Data[prID] + changed := m.unreadThreads[thread.ID] || m.newThreads[thread.ID] || + !state.Initialized || !state.Threads[thread.ID] + for _, comment := range thread.Comments { + if m.unreadComments[comment.ID] || !state.Comments[comment.ID] { + changed = true + break } } + if !changed { + return + } + delete(m.unreadThreads, thread.ID) + delete(m.newThreads, thread.ID) + if state.Threads == nil { + state.Threads = make(map[string]bool) + } + if state.Comments == nil { + state.Comments = make(map[string]bool) + } + state.Initialized = true + state.Threads[thread.ID] = true + for _, comment := range thread.Comments { + state.Comments[comment.ID] = true + delete(m.unreadComments, comment.ID) + } + m.readState.Data[prID] = state + if err := m.readState.save(); err != nil { + m.recordHealth("read state", healthWarning, err.Error()) + } +} + +func (m *App) focusThreadDetail() { + m.focus = threadDetailPane + m.markCurrentThreadRead() } func (m *App) markCommentRead(commentID string) { @@ -1545,10 +1572,9 @@ func (m *App) scrollToFirstUnread() { return } width, _ := m.detailPaneSize() - dividerAnchor := "unread:" + firstUnread commentAnchor := "comment:" + firstUnread + ":header" for index, line := range m.renderedDetailLines(width) { - if line.anchor == dividerAnchor || line.anchor == commentAnchor { + if line.anchor == commentAnchor { m.scroll = min(index, m.detailMaxScroll()) return } @@ -1595,8 +1621,10 @@ func (m *App) moveToUnread(direction int) { } if m.unreadThreads[m.details.Threads[index].ID] { m.threadIndex, m.scroll = index, 0 - m.focus = threadDetailPane m.scrollToFirstUnread() + anchor := m.detailScrollAnchor() + m.focusThreadDetail() + m.restoreDetailAnchor(anchor) return } } diff --git a/tui_test.go b/tui_test.go index 74e0399..dfa8f80 100644 --- a/tui_test.go +++ b/tui_test.go @@ -1301,19 +1301,47 @@ func TestPollingMarksNewThreadCommentsUnread(t *testing.T) { } updated, _ = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("l")}) m = updated.(App) - if !m.unreadThreads["thread"] { - t.Fatal("focusing the thread detail marked the thread read") + if m.unreadThreads["thread"] || m.unreadComments["comment-2"] { + t.Fatal("focusing the thread detail did not mark the thread read") } + + m.unreadThreads["thread"] = true + m.unreadComments["comment-2"] = true updated, _ = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("n")}) m = updated.(App) - if !m.unreadThreads["thread"] || m.focus != threadDetailPane || - m.detailScrollAnchor() != "unread:comment-2" { - t.Fatal("next-unread did not preserve and position the unread update") + if m.unreadThreads["thread"] || m.unreadComments["comment-2"] || + m.focus != threadDetailPane { + t.Fatal("next-unread did not open and mark the thread discussion read") } - updated, _ = m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("j")}) +} + +func TestResolvingThreadMarksItRead(t *testing.T) { + service := &recordingService{} + m := NewApp(service, "o", "r", false, 50, time.Second) + m.screen, m.loading = threadScreen, false + m.details = PRDetails{ + PullRequest: PullRequest{ID: "pr", Owner: "o", Repository: "r", Number: 1}, + Threads: []ReviewThread{{ + ID: "thread", ViewerCanResolve: true, + Comments: []ReviewComment{{ID: "comment"}}, + }}, + } + m.unreadThreads["thread"] = true + m.unreadComments["comment"] = true + + updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("R")}) m = updated.(App) - if m.unreadThreads["thread"] || m.unreadComments["comment-2"] { - t.Fatal("scrolling the visible final unread comment did not mark the thread read") + updated, command := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune("y")}) + m = updated.(App) + updated, _ = m.Update(command()) + m = updated.(App) + + if m.unreadThreads["thread"] || m.unreadComments["comment"] { + t.Fatal("resolved thread remained unread") + } + state := m.readState.Data["pr"] + if !state.Threads["thread"] || !state.Comments["comment"] { + t.Fatalf("resolved thread read state was not persisted: %#v", state) } } diff --git a/version.go b/version.go index 4a7e09e..1081732 100644 --- a/version.go +++ b/version.go @@ -1,3 +1,3 @@ package main -const dipleVersion = "0.1.0" +const dipleVersion = "0.1.1"