Add ordering for threads
This commit is contained in:
@@ -80,6 +80,12 @@ thread_list_width_percent = 33 # 20-60
|
|||||||
[paths]
|
[paths]
|
||||||
scroll = false
|
scroll = false
|
||||||
scroll_interval = "350ms" # minimum 50ms
|
scroll_interval = "350ms" # minimum 50ms
|
||||||
|
|
||||||
|
[threads]
|
||||||
|
# Each status must occur exactly once. "outdated" means unresolved and outdated;
|
||||||
|
# resolved threads remain in "resolved" even when they are also outdated.
|
||||||
|
status_order = ["unresolved", "outdated", "resolved"]
|
||||||
|
within_status = "file" # "file" or "timestamp" (oldest first)
|
||||||
```
|
```
|
||||||
|
|
||||||
Command-line flags override the configuration. `GH_REPO` overrides the
|
Command-line flags override the configuration. `GH_REPO` overrides the
|
||||||
|
|||||||
37
config.go
37
config.go
@@ -33,6 +33,7 @@ type Config struct {
|
|||||||
Endpoint string `toml:"endpoint"`
|
Endpoint string `toml:"endpoint"`
|
||||||
Display DisplayConfig `toml:"display"`
|
Display DisplayConfig `toml:"display"`
|
||||||
Paths PathConfig `toml:"paths"`
|
Paths PathConfig `toml:"paths"`
|
||||||
|
Threads ThreadConfig `toml:"threads"`
|
||||||
}
|
}
|
||||||
|
|
||||||
type DisplayConfig struct {
|
type DisplayConfig struct {
|
||||||
@@ -45,6 +46,11 @@ type PathConfig struct {
|
|||||||
ScrollInterval configDuration `toml:"scroll_interval"`
|
ScrollInterval configDuration `toml:"scroll_interval"`
|
||||||
}
|
}
|
||||||
|
|
||||||
|
type ThreadConfig struct {
|
||||||
|
StatusOrder []string `toml:"status_order"`
|
||||||
|
WithinStatus string `toml:"within_status"`
|
||||||
|
}
|
||||||
|
|
||||||
func defaultConfig() Config {
|
func defaultConfig() Config {
|
||||||
return Config{
|
return Config{
|
||||||
Theme: "dark",
|
Theme: "dark",
|
||||||
@@ -59,6 +65,10 @@ func defaultConfig() Config {
|
|||||||
Scroll: false,
|
Scroll: false,
|
||||||
ScrollInterval: configDuration{350 * time.Millisecond},
|
ScrollInterval: configDuration{350 * time.Millisecond},
|
||||||
},
|
},
|
||||||
|
Threads: ThreadConfig{
|
||||||
|
StatusOrder: []string{"unresolved", "outdated", "resolved"},
|
||||||
|
WithinStatus: "file",
|
||||||
|
},
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -129,8 +139,35 @@ func validateConfig(config Config) error {
|
|||||||
if config.Display.ThreadListWidthPercent < 20 || config.Display.ThreadListWidthPercent > 60 {
|
if config.Display.ThreadListWidthPercent < 20 || config.Display.ThreadListWidthPercent > 60 {
|
||||||
return fmt.Errorf("display.thread_list_width_percent must be between 20 and 60")
|
return fmt.Errorf("display.thread_list_width_percent must be between 20 and 60")
|
||||||
}
|
}
|
||||||
|
if err := validateThreadStatusOrder(config.Threads.StatusOrder); err != nil {
|
||||||
|
return err
|
||||||
|
}
|
||||||
|
switch config.Threads.WithinStatus {
|
||||||
|
case "file", "timestamp":
|
||||||
|
default:
|
||||||
|
return fmt.Errorf("threads.within_status must be file or timestamp")
|
||||||
|
}
|
||||||
if config.ShowAll && config.Repository == "" {
|
if config.ShowAll && config.Repository == "" {
|
||||||
return fmt.Errorf("show_all requires repository")
|
return fmt.Errorf("show_all requires repository")
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func validateThreadStatusOrder(order []string) error {
|
||||||
|
if len(order) != 3 {
|
||||||
|
return fmt.Errorf("threads.status_order must contain unresolved, outdated, and resolved exactly once")
|
||||||
|
}
|
||||||
|
seen := map[string]bool{}
|
||||||
|
for _, status := range order {
|
||||||
|
switch status {
|
||||||
|
case "unresolved", "outdated", "resolved":
|
||||||
|
if seen[status] {
|
||||||
|
return fmt.Errorf("threads.status_order contains %q more than once", status)
|
||||||
|
}
|
||||||
|
seen[status] = true
|
||||||
|
default:
|
||||||
|
return fmt.Errorf("threads.status_order contains unknown status %q", status)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
|
|||||||
@@ -40,6 +40,10 @@ thread_list_width_percent = 45
|
|||||||
[paths]
|
[paths]
|
||||||
scroll = true
|
scroll = true
|
||||||
scroll_interval = "125ms"
|
scroll_interval = "125ms"
|
||||||
|
|
||||||
|
[threads]
|
||||||
|
status_order = ["resolved", "unresolved", "outdated"]
|
||||||
|
within_status = "timestamp"
|
||||||
`
|
`
|
||||||
if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
|
if err := os.WriteFile(path, []byte(content), 0o600); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
@@ -54,7 +58,9 @@ scroll_interval = "125ms"
|
|||||||
if got.Theme != "light" || got.RefreshInterval.Duration != 25*time.Second ||
|
if got.Theme != "light" || got.RefreshInterval.Duration != 25*time.Second ||
|
||||||
got.Repository != "owner/repo" || !got.ShowAll || got.Limit != 75 ||
|
got.Repository != "owner/repo" || !got.ShowAll || got.Limit != 75 ||
|
||||||
got.Display.FoldResolved || got.Display.ThreadListWidthPercent != 45 ||
|
got.Display.FoldResolved || got.Display.ThreadListWidthPercent != 45 ||
|
||||||
!got.Paths.Scroll || got.Paths.ScrollInterval.Duration != 125*time.Millisecond {
|
!got.Paths.Scroll || got.Paths.ScrollInterval.Duration != 125*time.Millisecond ||
|
||||||
|
strings.Join(got.Threads.StatusOrder, ",") != "resolved,unresolved,outdated" ||
|
||||||
|
got.Threads.WithinStatus != "timestamp" {
|
||||||
t.Fatalf("config = %#v", got)
|
t.Fatalf("config = %#v", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -126,3 +132,16 @@ func TestValidateConfigRejectsUnsafeAnimationRate(t *testing.T) {
|
|||||||
t.Fatal("unsafe path scrolling interval was accepted")
|
t.Fatal("unsafe path scrolling interval was accepted")
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestValidateConfigRejectsInvalidThreadOrdering(t *testing.T) {
|
||||||
|
config := defaultConfig()
|
||||||
|
config.Threads.StatusOrder = []string{"unresolved", "resolved", "resolved"}
|
||||||
|
if err := validateConfig(config); err == nil {
|
||||||
|
t.Fatal("duplicate thread status was accepted")
|
||||||
|
}
|
||||||
|
config = defaultConfig()
|
||||||
|
config.Threads.WithinStatus = "author"
|
||||||
|
if err := validateConfig(config); err == nil {
|
||||||
|
t.Fatal("unknown within-status ordering was accepted")
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
2
main.go
2
main.go
@@ -96,6 +96,8 @@ func main() {
|
|||||||
ThreadListWidthPercent: config.Display.ThreadListWidthPercent,
|
ThreadListWidthPercent: config.Display.ThreadListWidthPercent,
|
||||||
PathScroll: config.Paths.Scroll,
|
PathScroll: config.Paths.Scroll,
|
||||||
PathScrollInterval: config.Paths.ScrollInterval.Duration,
|
PathScrollInterval: config.Paths.ScrollInterval.Duration,
|
||||||
|
ThreadStatusOrder: config.Threads.StatusOrder,
|
||||||
|
ThreadWithinStatus: config.Threads.WithinStatus,
|
||||||
},
|
},
|
||||||
)
|
)
|
||||||
if _, err := tea.NewProgram(app, tea.WithAltScreen()).Run(); err != nil {
|
if _, err := tea.NewProgram(app, tea.WithAltScreen()).Run(); err != nil {
|
||||||
|
|||||||
85
tui.go
85
tui.go
@@ -73,6 +73,8 @@ type App struct {
|
|||||||
pathScroll bool
|
pathScroll bool
|
||||||
pathScrollInterval time.Duration
|
pathScrollInterval time.Duration
|
||||||
pathScrollStep int
|
pathScrollStep int
|
||||||
|
threadStatusOrder []string
|
||||||
|
threadWithinStatus string
|
||||||
}
|
}
|
||||||
|
|
||||||
type AppSettings struct {
|
type AppSettings struct {
|
||||||
@@ -80,6 +82,8 @@ type AppSettings struct {
|
|||||||
ThreadListWidthPercent int
|
ThreadListWidthPercent int
|
||||||
PathScroll bool
|
PathScroll bool
|
||||||
PathScrollInterval time.Duration
|
PathScrollInterval time.Duration
|
||||||
|
ThreadStatusOrder []string
|
||||||
|
ThreadWithinStatus string
|
||||||
}
|
}
|
||||||
|
|
||||||
func defaultAppSettings() AppSettings {
|
func defaultAppSettings() AppSettings {
|
||||||
@@ -87,6 +91,8 @@ func defaultAppSettings() AppSettings {
|
|||||||
FoldResolved: true,
|
FoldResolved: true,
|
||||||
ThreadListWidthPercent: 33,
|
ThreadListWidthPercent: 33,
|
||||||
PathScrollInterval: 350 * time.Millisecond,
|
PathScrollInterval: 350 * time.Millisecond,
|
||||||
|
ThreadStatusOrder: []string{"unresolved", "outdated", "resolved"},
|
||||||
|
ThreadWithinStatus: "file",
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -107,6 +113,8 @@ func NewAppWithSettings(
|
|||||||
folded: make(map[string]bool), loading: true,
|
folded: make(map[string]bool), loading: true,
|
||||||
foldResolved: settings.FoldResolved, threadListWidthPercent: settings.ThreadListWidthPercent,
|
foldResolved: settings.FoldResolved, threadListWidthPercent: settings.ThreadListWidthPercent,
|
||||||
pathScroll: settings.PathScroll, pathScrollInterval: settings.PathScrollInterval,
|
pathScroll: settings.PathScroll, pathScrollInterval: settings.PathScrollInterval,
|
||||||
|
threadStatusOrder: append([]string(nil), settings.ThreadStatusOrder...),
|
||||||
|
threadWithinStatus: settings.ThreadWithinStatus,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -203,6 +211,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
|
|||||||
if m.threadIndex < len(m.details.Threads) {
|
if m.threadIndex < len(m.details.Threads) {
|
||||||
selected = m.details.Threads[m.threadIndex].ID
|
selected = m.details.Threads[m.threadIndex].ID
|
||||||
}
|
}
|
||||||
|
sortReviewThreads(msg.details.Threads, m.threadStatusOrder, m.threadWithinStatus)
|
||||||
m.details = msg.details
|
m.details = msg.details
|
||||||
m.threadIndex = indexThread(m.details.Threads, selected)
|
m.threadIndex = indexThread(m.details.Threads, selected)
|
||||||
if selected != "" && (len(m.details.Threads) == 0 || m.details.Threads[m.threadIndex].ID != selected) {
|
if selected != "" && (len(m.details.Threads) == 0 || m.details.Threads[m.threadIndex].ID != selected) {
|
||||||
@@ -444,6 +453,82 @@ func (m App) matchingThreadIndices() []int {
|
|||||||
return indices
|
return indices
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func sortReviewThreads(threads []ReviewThread, statusOrder []string, withinStatus string) {
|
||||||
|
ranks := map[string]int{
|
||||||
|
"unresolved": 0,
|
||||||
|
"outdated": 1,
|
||||||
|
"resolved": 2,
|
||||||
|
}
|
||||||
|
for rank, status := range statusOrder {
|
||||||
|
ranks[status] = rank
|
||||||
|
}
|
||||||
|
if withinStatus == "" {
|
||||||
|
withinStatus = "file"
|
||||||
|
}
|
||||||
|
sort.SliceStable(threads, func(i, j int) bool {
|
||||||
|
left, right := threads[i], threads[j]
|
||||||
|
leftRank, rightRank := ranks[threadStatus(left)], ranks[threadStatus(right)]
|
||||||
|
if leftRank != rightRank {
|
||||||
|
return leftRank < rightRank
|
||||||
|
}
|
||||||
|
if withinStatus == "timestamp" {
|
||||||
|
if less, decided := compareThreadTimestamps(left, right); decided {
|
||||||
|
return less
|
||||||
|
}
|
||||||
|
}
|
||||||
|
leftPath, rightPath := strings.ToLower(left.Path), strings.ToLower(right.Path)
|
||||||
|
if leftPath != rightPath {
|
||||||
|
return leftPath < rightPath
|
||||||
|
}
|
||||||
|
if left.Line != right.Line {
|
||||||
|
return left.Line < right.Line
|
||||||
|
}
|
||||||
|
if withinStatus != "timestamp" {
|
||||||
|
if less, decided := compareThreadTimestamps(left, right); decided {
|
||||||
|
return less
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return false
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
func threadStatus(thread ReviewThread) string {
|
||||||
|
if thread.IsResolved {
|
||||||
|
return "resolved"
|
||||||
|
}
|
||||||
|
if thread.IsOutdated {
|
||||||
|
return "outdated"
|
||||||
|
}
|
||||||
|
return "unresolved"
|
||||||
|
}
|
||||||
|
|
||||||
|
func threadOpenedAt(thread ReviewThread) time.Time {
|
||||||
|
var opened time.Time
|
||||||
|
for _, comment := range thread.Comments {
|
||||||
|
if comment.CreatedAt.IsZero() || (!opened.IsZero() && !comment.CreatedAt.Before(opened)) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
|
opened = comment.CreatedAt
|
||||||
|
}
|
||||||
|
return opened
|
||||||
|
}
|
||||||
|
|
||||||
|
func compareThreadTimestamps(left, right ReviewThread) (less, decided bool) {
|
||||||
|
leftTime, rightTime := threadOpenedAt(left), threadOpenedAt(right)
|
||||||
|
switch {
|
||||||
|
case leftTime.IsZero() && rightTime.IsZero():
|
||||||
|
return false, false
|
||||||
|
case leftTime.IsZero():
|
||||||
|
return false, true
|
||||||
|
case rightTime.IsZero():
|
||||||
|
return true, true
|
||||||
|
case !leftTime.Equal(rightTime):
|
||||||
|
return leftTime.Before(rightTime), true
|
||||||
|
default:
|
||||||
|
return false, false
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func (m *App) toStart() {
|
func (m *App) toStart() {
|
||||||
if m.screen == prScreen {
|
if m.screen == prScreen {
|
||||||
m.prIndex = 0
|
m.prIndex = 0
|
||||||
|
|||||||
46
tui_test.go
46
tui_test.go
@@ -89,6 +89,52 @@ func TestSelectionSurvivesRefresh(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestDefaultThreadOrderingUsesStatusThenFile(t *testing.T) {
|
||||||
|
threads := []ReviewThread{
|
||||||
|
{ID: "resolved-outdated", Path: "a.go", IsResolved: true, IsOutdated: true},
|
||||||
|
{ID: "outdated", Path: "a.go", IsOutdated: true},
|
||||||
|
{ID: "current-z", Path: "z.go"},
|
||||||
|
{ID: "resolved", Path: "z.go", IsResolved: true},
|
||||||
|
{ID: "current-a", Path: "a.go"},
|
||||||
|
}
|
||||||
|
settings := defaultAppSettings()
|
||||||
|
sortReviewThreads(threads, settings.ThreadStatusOrder, settings.ThreadWithinStatus)
|
||||||
|
got := make([]string, len(threads))
|
||||||
|
for i, thread := range threads {
|
||||||
|
got[i] = thread.ID
|
||||||
|
}
|
||||||
|
want := []string{"current-a", "current-z", "outdated", "resolved-outdated", "resolved"}
|
||||||
|
if strings.Join(got, ",") != strings.Join(want, ",") {
|
||||||
|
t.Fatalf("thread order = %v, want %v", got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestCustomThreadOrderingUsesOpeningTimestamp(t *testing.T) {
|
||||||
|
base := time.Date(2026, 7, 1, 12, 0, 0, 0, time.UTC)
|
||||||
|
threads := []ReviewThread{
|
||||||
|
{ID: "current", Path: "a.go", Comments: []ReviewComment{{CreatedAt: base}}},
|
||||||
|
{ID: "resolved-new", IsResolved: true, Path: "a.go", Comments: []ReviewComment{{CreatedAt: base.Add(2 * time.Hour)}}},
|
||||||
|
{ID: "outdated", IsOutdated: true, Path: "a.go", Comments: []ReviewComment{{CreatedAt: base.Add(time.Hour)}}},
|
||||||
|
{ID: "resolved-old", IsResolved: true, Path: "z.go", Comments: []ReviewComment{
|
||||||
|
{CreatedAt: base.Add(3 * time.Hour)},
|
||||||
|
{CreatedAt: base.Add(-time.Hour)},
|
||||||
|
}},
|
||||||
|
}
|
||||||
|
sortReviewThreads(
|
||||||
|
threads,
|
||||||
|
[]string{"resolved", "outdated", "unresolved"},
|
||||||
|
"timestamp",
|
||||||
|
)
|
||||||
|
got := make([]string, len(threads))
|
||||||
|
for i, thread := range threads {
|
||||||
|
got[i] = thread.ID
|
||||||
|
}
|
||||||
|
want := []string{"resolved-old", "resolved-new", "outdated", "current"}
|
||||||
|
if strings.Join(got, ",") != strings.Join(want, ",") {
|
||||||
|
t.Fatalf("thread order = %v, want %v", got, want)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func TestPaneFocusAndNavigation(t *testing.T) {
|
func TestPaneFocusAndNavigation(t *testing.T) {
|
||||||
m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
|
m := NewApp(nil, "o", "r", false, 50, 10*time.Second)
|
||||||
m.screen = threadScreen
|
m.screen = threadScreen
|
||||||
|
|||||||
Reference in New Issue
Block a user