Skip to content

Commit aee0d2a

Browse files
wesmclaude
andauthored
Use display_name if set in filter modal (#51)
This fixes a bug where a repository having a `display_name` in its .roborev.toml would show up as its repo name in the filter modal, not its display name. --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 1e82407 commit aee0d2a

File tree

2 files changed

+217
-62
lines changed

2 files changed

+217
-62
lines changed

cmd/roborev/tui.go

Lines changed: 99 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ const (
5858
tuiViewFilter
5959
)
6060

61-
// repoFilterItem represents a repo in the filter modal with its review count
61+
// repoFilterItem represents a repo (or group of repos with same display name) in the filter modal
6262
type repoFilterItem struct {
63-
name string // Display name (basename). Empty string means "All repos"
64-
rootPath string // Unique identifier (full path). Empty for "All repos"
65-
count int
63+
name string // Display name. Empty string means "All repos"
64+
rootPaths []string // Repo paths that share this display name. Empty for "All repos"
65+
count int
6666
}
6767

6868
type tuiModel struct {
@@ -98,8 +98,8 @@ type tuiModel struct {
9898
filterSearch string // Search/filter text typed by user
9999

100100
// Active filter (applied to queue view)
101-
activeRepoFilter string // Empty = show all, otherwise repo root_path to filter by
102-
hideAddressed bool // When true, hide jobs with addressed reviews
101+
activeRepoFilter []string // Empty = show all, otherwise repo root_paths to filter by
102+
hideAddressed bool // When true, hide jobs with addressed reviews
103103

104104
// Display name cache (keyed by repo path)
105105
displayNames map[string]string
@@ -232,8 +232,12 @@ func (m tuiModel) fetchJobs() tea.Cmd {
232232
// - If we've paginated beyond visible area, maintain current view size
233233
// - Otherwise fetch enough to fill visible area
234234
var url string
235-
if m.activeRepoFilter != "" {
236-
url = fmt.Sprintf("%s/api/jobs?limit=0&repo=%s", m.serverAddr, neturl.QueryEscape(m.activeRepoFilter))
235+
if len(m.activeRepoFilter) == 1 {
236+
// Single repo filter - use API filter
237+
url = fmt.Sprintf("%s/api/jobs?limit=0&repo=%s", m.serverAddr, neturl.QueryEscape(m.activeRepoFilter[0]))
238+
} else if len(m.activeRepoFilter) > 1 {
239+
// Multiple repos (shared display name) - fetch all, filter client-side
240+
url = fmt.Sprintf("%s/api/jobs?limit=0", m.serverAddr)
237241
} else if m.hideAddressed {
238242
// Fetch all jobs when hiding addressed - client-side filtering needs full dataset
239243
url = fmt.Sprintf("%s/api/jobs?limit=0", m.serverAddr)
@@ -268,7 +272,7 @@ func (m tuiModel) fetchJobs() tea.Cmd {
268272
func (m tuiModel) fetchMoreJobs() tea.Cmd {
269273
return func() tea.Msg {
270274
// Only fetch more when not filtering (filtered view loads all)
271-
if m.activeRepoFilter != "" {
275+
if len(m.activeRepoFilter) > 0 {
272276
return nil
273277
}
274278
offset := len(m.jobs)
@@ -348,10 +352,29 @@ func (m tuiModel) fetchRepos() tea.Cmd {
348352
return tuiErrMsg(err)
349353
}
350354

351-
// Convert to repoFilterItem slice
352-
repos := make([]repoFilterItem, len(result.Repos))
353-
for i, r := range result.Repos {
354-
repos[i] = repoFilterItem{name: r.Name, rootPath: r.RootPath, count: r.Count}
355+
// Aggregate repos by display name
356+
displayNameMap := make(map[string]*repoFilterItem)
357+
var displayNameOrder []string // Preserve order for stable display
358+
for _, r := range result.Repos {
359+
displayName := config.GetDisplayName(r.RootPath)
360+
if displayName == "" {
361+
displayName = r.Name
362+
}
363+
if item, ok := displayNameMap[displayName]; ok {
364+
item.rootPaths = append(item.rootPaths, r.RootPath)
365+
item.count += r.Count
366+
} else {
367+
displayNameMap[displayName] = &repoFilterItem{
368+
name: displayName,
369+
rootPaths: []string{r.RootPath},
370+
count: r.Count,
371+
}
372+
displayNameOrder = append(displayNameOrder, displayName)
373+
}
374+
}
375+
repos := make([]repoFilterItem, len(displayNameOrder))
376+
for i, name := range displayNameOrder {
377+
repos[i] = *displayNameMap[name]
355378
}
356379
return tuiReposMsg{repos: repos, totalCount: result.TotalCount}
357380
}
@@ -722,8 +745,21 @@ func (m *tuiModel) getVisibleFilterRepos() []repoFilterItem {
722745
var visible []repoFilterItem
723746
for _, r := range m.filterRepos {
724747
// Always include "All repos" option, filter others by search
725-
if r.name == "" || strings.Contains(strings.ToLower(r.name), search) {
748+
if r.name == "" {
726749
visible = append(visible, r)
750+
continue
751+
}
752+
// Search by display name
753+
if strings.Contains(strings.ToLower(r.name), search) {
754+
visible = append(visible, r)
755+
continue
756+
}
757+
// Also search by underlying repo path basenames
758+
for _, p := range r.rootPaths {
759+
if strings.Contains(strings.ToLower(filepath.Base(p)), search) {
760+
visible = append(visible, r)
761+
break
762+
}
727763
}
728764
}
729765
return visible
@@ -753,9 +789,19 @@ func (m *tuiModel) getSelectedFilterRepo() *repoFilterItem {
753789
return nil
754790
}
755791

792+
// repoMatchesFilter checks if a repo path matches the active filter
793+
func (m tuiModel) repoMatchesFilter(repoPath string) bool {
794+
for _, p := range m.activeRepoFilter {
795+
if p == repoPath {
796+
return true
797+
}
798+
}
799+
return false
800+
}
801+
756802
// isJobVisible checks if a job passes all active filters
757803
func (m tuiModel) isJobVisible(job storage.ReviewJob) bool {
758-
if m.activeRepoFilter != "" && job.RepoPath != m.activeRepoFilter {
804+
if len(m.activeRepoFilter) > 0 && !m.repoMatchesFilter(job.RepoPath) {
759805
return false
760806
}
761807
if m.hideAddressed {
@@ -772,7 +818,7 @@ func (m tuiModel) isJobVisible(job storage.ReviewJob) bool {
772818

773819
// getVisibleJobs returns jobs filtered by active filters (repo, addressed)
774820
func (m tuiModel) getVisibleJobs() []storage.ReviewJob {
775-
if m.activeRepoFilter == "" && !m.hideAddressed {
821+
if len(m.activeRepoFilter) == 0 && !m.hideAddressed {
776822
return m.jobs
777823
}
778824
var visible []storage.ReviewJob
@@ -790,7 +836,7 @@ func (m tuiModel) getVisibleSelectedIdx() int {
790836
if m.selectedIdx < 0 {
791837
return -1
792838
}
793-
if m.activeRepoFilter == "" && !m.hideAddressed {
839+
if len(m.activeRepoFilter) == 0 && !m.hideAddressed {
794840
return m.selectedIdx
795841
}
796842
count := 0
@@ -868,7 +914,7 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
868914
case "enter":
869915
selected := m.getSelectedFilterRepo()
870916
if selected != nil {
871-
m.activeRepoFilter = selected.rootPath
917+
m.activeRepoFilter = selected.rootPaths
872918
m.currentView = tuiViewQueue
873919
m.filterSearch = ""
874920
// Invalidate selection until refetch completes - prevents
@@ -980,7 +1026,7 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
9801026
if nextIdx >= 0 {
9811027
m.selectedIdx = nextIdx
9821028
m.updateSelectedJobID()
983-
} else if m.hasMore && !m.loadingMore && !m.loadingJobs && m.activeRepoFilter == "" {
1029+
} else if m.hasMore && !m.loadingMore && !m.loadingJobs && len(m.activeRepoFilter) == 0 {
9841030
// At bottom with more jobs available - load them
9851031
m.loadingMore = true
9861032
return m, m.fetchMoreJobs()
@@ -998,7 +1044,7 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
9981044
if nextIdx >= 0 {
9991045
m.selectedIdx = nextIdx
10001046
m.updateSelectedJobID()
1001-
} else if m.hasMore && !m.loadingMore && !m.loadingJobs && m.activeRepoFilter == "" {
1047+
} else if m.hasMore && !m.loadingMore && !m.loadingJobs && len(m.activeRepoFilter) == 0 {
10021048
// At bottom with more jobs available - load them
10031049
m.loadingMore = true
10041050
return m, m.fetchMoreJobs()
@@ -1059,7 +1105,7 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
10591105
}
10601106
m.updateSelectedJobID()
10611107
// If we hit the end, try to load more
1062-
if reachedEnd && m.hasMore && !m.loadingMore && !m.loadingJobs && m.activeRepoFilter == "" {
1108+
if reachedEnd && m.hasMore && !m.loadingMore && !m.loadingJobs && len(m.activeRepoFilter) == 0 {
10631109
m.loadingMore = true
10641110
return m, m.fetchMoreJobs()
10651111
}
@@ -1225,9 +1271,9 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
12251271
}
12261272

12271273
case "esc":
1228-
if m.currentView == tuiViewQueue && (m.activeRepoFilter != "" || m.hideAddressed) {
1274+
if m.currentView == tuiViewQueue && (len(m.activeRepoFilter) > 0 || m.hideAddressed) {
12291275
// Clear filters and refetch all jobs
1230-
m.activeRepoFilter = ""
1276+
m.activeRepoFilter = nil
12311277
m.hideAddressed = false
12321278
// Reset to default view (clear jobs so fetchJobs uses appropriate limit)
12331279
m.jobs = nil
@@ -1261,7 +1307,7 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
12611307

12621308
// If terminal can show more jobs than we have, re-fetch to fill screen
12631309
// Gate on !loadingMore and !loadingJobs to avoid race conditions
1264-
if !m.loadingMore && !m.loadingJobs && len(m.jobs) > 0 && m.hasMore && m.activeRepoFilter == "" {
1310+
if !m.loadingMore && !m.loadingJobs && len(m.jobs) > 0 && m.hasMore && len(m.activeRepoFilter) == 0 {
12651311
newVisibleRows := m.height - 9 + 10
12661312
if newVisibleRows > len(m.jobs) {
12671313
m.loadingJobs = true
@@ -1319,7 +1365,7 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
13191365
// Job was removed - clamp index to valid range
13201366
m.selectedIdx = max(0, min(len(m.jobs)-1, m.selectedIdx))
13211367
// If any filter is active, ensure we're on a visible job
1322-
if m.activeRepoFilter != "" || m.hideAddressed {
1368+
if len(m.activeRepoFilter) > 0 || m.hideAddressed {
13231369
firstVisible := m.findFirstVisibleJob()
13241370
if firstVisible >= 0 {
13251371
m.selectedIdx = firstVisible
@@ -1366,7 +1412,7 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
13661412
if firstVisible >= 0 {
13671413
m.selectedIdx = firstVisible
13681414
m.selectedJobID = m.jobs[firstVisible].ID
1369-
} else if m.activeRepoFilter == "" && len(m.jobs) > 0 {
1415+
} else if len(m.activeRepoFilter) == 0 && len(m.jobs) > 0 {
13701416
// No filter, just select first job
13711417
m.selectedIdx = 0
13721418
m.selectedJobID = m.jobs[0].ID
@@ -1447,11 +1493,21 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
14471493
m.filterRepos = []repoFilterItem{{name: "", count: msg.totalCount}}
14481494
m.filterRepos = append(m.filterRepos, msg.repos...)
14491495
// Pre-select current filter if active
1450-
if m.activeRepoFilter != "" {
1496+
if len(m.activeRepoFilter) > 0 {
14511497
for i, r := range m.filterRepos {
1452-
if r.rootPath == m.activeRepoFilter {
1453-
m.filterSelectedIdx = i
1454-
break
1498+
if len(r.rootPaths) == len(m.activeRepoFilter) && len(r.rootPaths) > 0 {
1499+
// Check if all paths match
1500+
match := true
1501+
for j, p := range r.rootPaths {
1502+
if p != m.activeRepoFilter[j] {
1503+
match = false
1504+
break
1505+
}
1506+
}
1507+
if match {
1508+
m.filterSelectedIdx = i
1509+
break
1510+
}
14551511
}
14561512
}
14571513
}
@@ -1489,8 +1545,10 @@ func (m tuiModel) renderQueueView() string {
14891545

14901546
// Title with version, optional update notification, and filter indicators
14911547
title := fmt.Sprintf("roborev queue (%s)", version.Version)
1492-
if m.activeRepoFilter != "" {
1493-
title += fmt.Sprintf(" [f: %s]", filepath.Base(m.activeRepoFilter))
1548+
if len(m.activeRepoFilter) > 0 {
1549+
// Show display name for the filter (all paths share the same display name)
1550+
filterName := m.getDisplayName(m.activeRepoFilter[0], filepath.Base(m.activeRepoFilter[0]))
1551+
title += fmt.Sprintf(" [f: %s]", filterName)
14941552
}
14951553
if m.hideAddressed {
14961554
title += " [hiding addressed]"
@@ -1500,10 +1558,13 @@ func (m tuiModel) renderQueueView() string {
15001558

15011559
// Status line - show filtered counts when filter is active
15021560
var statusLine string
1503-
if m.activeRepoFilter != "" {
1504-
// Calculate counts from jobs (all pre-filtered by API)
1561+
if len(m.activeRepoFilter) > 0 {
1562+
// Calculate counts from visible jobs (handles multi-path client-side filtering)
15051563
var done, failed, canceled int
15061564
for _, job := range m.jobs {
1565+
if !m.repoMatchesFilter(job.RepoPath) {
1566+
continue
1567+
}
15071568
switch job.Status {
15081569
case storage.JobStatusDone:
15091570
done++
@@ -1529,7 +1590,7 @@ func (m tuiModel) renderQueueView() string {
15291590
visibleSelectedIdx := m.getVisibleSelectedIdx()
15301591

15311592
if len(visibleJobList) == 0 {
1532-
if m.activeRepoFilter != "" || m.hideAddressed {
1593+
if len(m.activeRepoFilter) > 0 || m.hideAddressed {
15331594
b.WriteString("No jobs matching filters\n")
15341595
} else {
15351596
b.WriteString("No jobs in queue\n")
@@ -1603,7 +1664,7 @@ func (m tuiModel) renderQueueView() string {
16031664
var scrollInfo string
16041665
if m.loadingMore {
16051666
scrollInfo = fmt.Sprintf("[showing %d-%d of %d] Loading more...", start+1, end, len(visibleJobList))
1606-
} else if m.hasMore && m.activeRepoFilter == "" {
1667+
} else if m.hasMore && len(m.activeRepoFilter) == 0 {
16071668
scrollInfo = fmt.Sprintf("[showing %d-%d of %d+] scroll down to load more", start+1, end, len(visibleJobList))
16081669
} else if len(visibleJobList) > visibleRows {
16091670
scrollInfo = fmt.Sprintf("[showing %d-%d of %d]", start+1, end, len(visibleJobList))
@@ -1633,7 +1694,7 @@ func (m tuiModel) renderQueueView() string {
16331694
// Help (two lines)
16341695
helpText := "up/down/pgup/pgdn: navigate | enter: review | p: prompt | f: filter | h: hide addressed | q: quit\n" +
16351696
"a: toggle addressed | x: cancel | r: rerun"
1636-
if m.activeRepoFilter != "" || m.hideAddressed {
1697+
if len(m.activeRepoFilter) > 0 || m.hideAddressed {
16371698
helpText += " | esc: clear filters"
16381699
}
16391700
b.WriteString(tuiHelpStyle.Render(helpText))
@@ -2058,6 +2119,7 @@ func (m tuiModel) renderFilterView() string {
20582119
if repo.name == "" {
20592120
line = fmt.Sprintf("All repos (%d)", repo.count)
20602121
} else {
2122+
// repo.name is already the display name (aggregated in fetchRepos)
20612123
line = fmt.Sprintf("%s (%d)", repo.name, repo.count)
20622124
}
20632125

0 commit comments

Comments
 (0)