diff --git a/external/spotify/album_test.go b/external/spotify/album_test.go new file mode 100644 index 0000000..091f675 --- /dev/null +++ b/external/spotify/album_test.go @@ -0,0 +1,157 @@ +//go:build !windows + +package spotify + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "strconv" + "strings" + "testing" + + "golang.org/x/oauth2" +) + +// albumSpotify fakes the album endpoints plus a search that returns both albums +// and tracks. It records the search type parameter so the query itself is +// covered: asking for tracks only is exactly the bug this guards against. +func albumSpotify(t *testing.T, albumHits, trackHits, albumTracks int) (*SpotifyProvider, *string) { + t.Helper() + var searchType string + + originalTransport := http.DefaultTransport + http.DefaultTransport = roundTripFunc(func(req *http.Request) (*http.Response, error) { + var payload map[string]any + + switch path := req.URL.Path; { + case path == "/v1/search": + searchType = req.URL.Query().Get("type") + albums := []map[string]any{} + for i := range albumHits { + albums = append(albums, map[string]any{ + "id": fmt.Sprintf("al%d", i), + "name": fmt.Sprintf("Album %d", i), + "type": "album", + "uri": fmt.Sprintf("spotify:album:al%d", i), + "release_date": "1994-07-19", + "artists": []map[string]any{{"name": "NOFX"}}, + }) + } + tracks := []map[string]any{} + for i := range trackHits { + tracks = append(tracks, map[string]any{ + "id": fmt.Sprintf("t%d", i), + "name": fmt.Sprintf("Track %d", i), + "type": "track", + "uri": fmt.Sprintf("spotify:track:t%d", i), + }) + } + payload = map[string]any{ + "albums": map[string]any{"items": albums}, + "tracks": map[string]any{"items": tracks}, + "episodes": map[string]any{"items": []any{}}, + } + + case strings.HasSuffix(path, "/tracks"): + offset, _ := strconv.Atoi(req.URL.Query().Get("offset")) + items := []map[string]any{} + for i := offset; i < offset+spotifyTrackPageSize && i < albumTracks; i++ { + items = append(items, map[string]any{ + "id": fmt.Sprintf("at%d", i), + "name": fmt.Sprintf("Album track %d", i), + "type": "track", + "uri": fmt.Sprintf("spotify:track:at%d", i), + "track_number": i + 1, + "artists": []map[string]any{{"name": "NOFX"}}, + }) + } + payload = map[string]any{"items": items} + + case strings.HasPrefix(path, "/v1/albums/"): + payload = map[string]any{ + "id": "al0", + "name": "Punk In Drublic", + "type": "album", + "uri": "spotify:album:al0", + "release_date": "1994-07-19", + "artists": []map[string]any{{"name": "NOFX"}}, + } + + default: + return nil, fmt.Errorf("unexpected Spotify API path %q", path) + } + + body, _ := json.Marshal(payload) + return &http.Response{ + StatusCode: http.StatusOK, + Status: "200 OK", + Header: make(http.Header), + Body: io.NopCloser(strings.NewReader(string(body))), + Request: req, + }, nil + }) + t.Cleanup(func() { http.DefaultTransport = originalTransport }) + + sess := &Session{tokenSource: oauth2.StaticTokenSource(&oauth2.Token{AccessToken: "token"})} + return New(sess, "client", 320), &searchType +} + +func TestSearchTracksLeadsWithAlbums(t *testing.T) { + p, searchType := albumSpotify(t, 2, 3, 0) + + got, err := p.SearchTracks(context.Background(), "nofx", 10) + if err != nil { + t.Fatalf("SearchTracks() error = %v", err) + } + if *searchType != "album,track,episode" { + t.Errorf("search type = %q, want %q", *searchType, "album,track,episode") + } + if len(got) != 5 { + t.Fatalf("got %d results, want 5", len(got)) + } + for i, want := range []bool{true, true, false, false, false} { + if got[i].IsAlbum() != want { + t.Errorf("result %d IsAlbum() = %v, want %v", i, got[i].IsAlbum(), want) + } + } + if id := got[0].AlbumID(); id != "al0" { + t.Errorf("album id = %q, want %q", id, "al0") + } + if got[0].Year != 1994 { + t.Errorf("album year = %d, want 1994", got[0].Year) + } + if got[2].AlbumID() != "" { + t.Errorf("track reported album id %q, want none", got[2].AlbumID()) + } +} + +func TestAlbumTracksPagesAndFillsAlbumMetadata(t *testing.T) { + const total = spotifyTrackPageSize + 3 + p, _ := albumSpotify(t, 0, 0, total) + + got, err := p.AlbumTracks("al0") + if err != nil { + t.Fatalf("AlbumTracks() error = %v", err) + } + if len(got) != total { + t.Fatalf("got %d tracks, want %d", len(got), total) + } + for i, tr := range got { + // /v1/albums/{id}/tracks omits the album, so every track must inherit it. + if tr.Album != "Punk In Drublic" { + t.Fatalf("track %d album = %q, want %q", i, tr.Album, "Punk In Drublic") + } + if tr.Year != 1994 { + t.Fatalf("track %d year = %d, want 1994", i, tr.Year) + } + if tr.IsAlbum() { + t.Fatalf("track %d is marked as an album placeholder", i) + } + } + if got[0].Path != "spotify:track:at0" { + t.Errorf("first track path = %q, want %q", got[0].Path, "spotify:track:at0") + } +} diff --git a/external/spotify/provider.go b/external/spotify/provider.go index a2ffc10..45e6f95 100644 --- a/external/spotify/provider.go +++ b/external/spotify/provider.go @@ -585,6 +585,9 @@ func friendlySearchError(err error) error { // spotifySearchPage is one page of /v1/search results. type spotifySearchPage struct { + Albums struct { + Items []*spotifyAlbumItem `json:"items"` + } `json:"albums"` Tracks struct { Items []*spotifyItem `json:"items"` } `json:"tracks"` @@ -600,7 +603,7 @@ type spotifySearchPage struct { func (p *SpotifyProvider) searchPage(ctx context.Context, query string, limit, offset int) (*spotifySearchPage, error) { q := url.Values{ "q": {query}, - "type": {"track,episode"}, + "type": {"album,track,episode"}, "limit": {strconv.Itoa(limit)}, } if offset > 0 { @@ -630,21 +633,27 @@ func (p *SpotifyProvider) searchPaged(ctx context.Context, query string, limit i if err != nil { return nil, fmt.Errorf("page at offset %d: %w", offset, err) } + combined.Albums.Items = append(combined.Albums.Items, page.Albums.Items...) combined.Tracks.Items = append(combined.Tracks.Items, page.Tracks.Items...) combined.Episodes.Items = append(combined.Episodes.Items, page.Episodes.Items...) - // Both result kinds exhausted, so further pages are empty. - if len(page.Tracks.Items) < size && len(page.Episodes.Items) < size { + // Every result kind exhausted, so further pages are empty. + if len(page.Albums.Items) < size && len(page.Tracks.Items) < size && len(page.Episodes.Items) < size { break } } return combined, nil } -// SearchTracks searches Spotify for tracks and podcast episodes, returning up -// to limit results of each. Episodes (e.g. podcasts) are routed through their -// spotify:episode: URI so they play correctly. +// SearchTracks searches Spotify for albums, tracks and podcast episodes, +// returning up to limit results of each. Episodes (e.g. podcasts) are routed +// through their spotify:episode: URI so they play correctly. // limit is clamped to Spotify's accepted range of 1..50. // +// Album hits lead the results as album placeholders (playlist.Track.IsAlbum), +// because a query is usually an artist or record name and the album is the +// more useful answer than whichever of its tracks Spotify ranks highest. They +// are not playable as-is: the caller expands the chosen one with AlbumTracks. +// // Apps in Development Mode cap /v1/search at devModeSearchLimit results per // request, so a rejected limit is retried as several smaller pages instead of // being reported as a blocked search. @@ -670,6 +679,12 @@ func (p *SpotifyProvider) SearchTracks(ctx context.Context, query string, limit } var tracks []playlist.Track + for _, a := range result.Albums.Items { + if a == nil || a.ID == "" { + continue // skip null/unavailable results + } + tracks = append(tracks, albumFromItem(a)) + } for _, items := range [][]*spotifyItem{result.Tracks.Items, result.Episodes.Items} { for _, t := range items { if t == nil || t.ID == "" { @@ -681,6 +696,86 @@ func (p *SpotifyProvider) SearchTracks(ctx context.Context, query string, limit return tracks, nil } +// AlbumTracks returns every track of a Spotify album, in disc and track order. +// Implements provider.AlbumTrackLoader, so an album placeholder from +// SearchTracks can be expanded into a playable list. +// +// /v1/albums/{id}/tracks returns simplified track objects that omit the album +// they belong to, so the album's own name, artist and release year are fetched +// once and filled in on every track for display. +func (p *SpotifyProvider) AlbumTracks(albumID string) ([]playlist.Track, error) { + if err := p.ensureSession(); err != nil { + return nil, err + } + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) + defer cancel() + + album, err := p.album(ctx, albumID) + if err != nil { + return nil, err + } + placeholder := albumFromItem(album) + + var tracks []playlist.Track + for offset := 0; ; offset += spotifyTrackPageSize { + page, err := p.albumTracksPage(ctx, albumID, offset) + if err != nil { + return nil, err + } + for _, item := range page { + if item == nil || item.ID == "" { + continue // skip null/unavailable results + } + track := trackFromItem(item) + track.Album = placeholder.Album + track.Year = placeholder.Year + if track.Artist == "" { + track.Artist = placeholder.Artist + } + tracks = append(tracks, track) + } + if len(page) < spotifyTrackPageSize { + break + } + } + return tracks, nil +} + +// album fetches an album's own metadata. +func (p *SpotifyProvider) album(ctx context.Context, albumID string) (*spotifyAlbumItem, error) { + resp, err := p.webAPI(ctx, "GET", "/v1/albums/"+url.PathEscape(albumID), nil) + if err != nil { + return nil, fmt.Errorf("spotify: album %s: %w", albumID, err) + } + var album spotifyAlbumItem + if err := decodeBody(resp, &album); err != nil { + return nil, fmt.Errorf("spotify: parse album %s: %w", albumID, err) + } + return &album, nil +} + +// albumTracksPage fetches one page of an album's track list. +func (p *SpotifyProvider) albumTracksPage(ctx context.Context, albumID string, offset int) ([]*spotifyItem, error) { + q := url.Values{ + "limit": {strconv.Itoa(spotifyTrackPageSize)}, + } + if offset > 0 { + q.Set("offset", strconv.Itoa(offset)) + } + + resp, err := p.webAPI(ctx, "GET", "/v1/albums/"+url.PathEscape(albumID)+"/tracks", q) + if err != nil { + return nil, fmt.Errorf("spotify: album %s tracks: %w", albumID, err) + } + var page struct { + Items []*spotifyItem `json:"items"` + } + if err := decodeBody(resp, &page); err != nil { + return nil, fmt.Errorf("spotify: parse album %s tracks: %w", albumID, err) + } + return page.Items, nil +} + // AddTrackToPlaylist adds a track to an existing Spotify playlist. // The track's Path is used as the Spotify URI (e.g. "spotify:track:..." or // "spotify:episode:..."); the Spotify API accepts either. diff --git a/external/spotify/provider_shared.go b/external/spotify/provider_shared.go index 9551c35..dc1590c 100644 --- a/external/spotify/provider_shared.go +++ b/external/spotify/provider_shared.go @@ -60,6 +60,55 @@ type spotifyItem struct { } `json:"restrictions"` } +// spotifyAlbumItem is a simplified album object from the Spotify Web API, as +// returned by /v1/search?type=album and /v1/albums/{id}. +type spotifyAlbumItem struct { + ID string `json:"id"` + Name string `json:"name"` + AlbumType string `json:"album_type"` // "album", "single" or "compilation" + URI string `json:"uri"` // canonical spotify:album:... + TotalTracks int `json:"total_tracks"` + ReleaseDate string `json:"release_date"` + Artists []spotifyArtist `json:"artists"` +} + +// albumFromItem converts an album search hit into an album placeholder Track. +// +// The result is deliberately not playable: Path carries the spotify:album: URI +// so the entry is identifiable, but go-librespot cannot stream an album URI. +// Callers must expand it through SearchTracks' companion AlbumTracks before +// queueing it, which playlist.Track.IsAlbum signals to the UI. +func albumFromItem(a *spotifyAlbumItem) playlist.Track { + artists := make([]string, len(a.Artists)) + for i, ar := range a.Artists { + artists[i] = ar.Name + } + + var year int + if len(a.ReleaseDate) >= 4 { + if y, err := strconv.Atoi(a.ReleaseDate[:4]); err == nil { + year = y + } + } + + uri := a.URI + if uri == "" { + uri = fmt.Sprintf("spotify:album:%s", a.ID) + } + + return playlist.Track{ + Path: uri, + Title: a.Name, + Artist: strings.Join(artists, ", "), + Album: a.Name, + Year: year, + ProviderMeta: map[string]string{ + playlist.MetaKind: playlist.MetaKindAlbum, + playlist.MetaAlbumID: a.ID, + }, + } +} + // trackFromItem converts a Spotify playlist/library item into a playlist.Track, // handling both music tracks and podcast episodes. It uses the canonical uri // the API returns (spotify:track:... or spotify:episode:...) as the path, so diff --git a/playlist/playlist.go b/playlist/playlist.go index c7ce6ca..aad3ab8 100644 --- a/playlist/playlist.go +++ b/playlist/playlist.go @@ -304,6 +304,35 @@ func (t Track) DisplayName() string { return t.Title } +// ProviderMeta keys shared across providers. Unlike the provider-namespaced +// keys (e.g. "navidrome.id"), these describe what a Track stands for, so the +// UI can handle it without knowing which provider produced it. +const ( + // MetaKind marks a Track that is not a plain playable track. + MetaKind = "kind" + // MetaKindAlbum is the MetaKind value for an album placeholder: a search + // result standing for a whole album, expanded to its tracks when chosen. + MetaKindAlbum = "album" + // MetaAlbumID carries the provider-side album id of an album placeholder. + MetaAlbumID = "albumID" +) + +// IsAlbum reports whether the track is an album placeholder rather than +// something playable on its own. Callers must expand it with the provider's +// AlbumTracks before handing it to the player. +func (t Track) IsAlbum() bool { + return t.ProviderMeta[MetaKind] == MetaKindAlbum +} + +// AlbumID returns the provider-side album id of an album placeholder, or "" +// when the track is not one. +func (t Track) AlbumID() string { + if !t.IsAlbum() { + return "" + } + return t.ProviderMeta[MetaAlbumID] +} + // Playlist manages an ordered list of tracks with shuffle and repeat support. // All exported methods are safe for concurrent use: the Bubbletea UI loop // mutates the playlist while Lua plugin goroutines read state through it. diff --git a/ui/model/commands.go b/ui/model/commands.go index 5123ed7..4129065 100644 --- a/ui/model/commands.go +++ b/ui/model/commands.go @@ -432,6 +432,33 @@ type spotSearchResultsMsg struct { gen uint64 } +// spotAlbumAction is what to do with an album's tracks once they arrive. +type spotAlbumAction int + +const ( + spotAlbumPlay spotAlbumAction = iota // start the album now + spotAlbumAppend // add to the end of the queue + spotAlbumQueueNext // play right after the current track +) + +type spotAlbumTracksMsg struct { + tracks []playlist.Track + album playlist.Track + action spotAlbumAction + err error + gen uint64 +} + +// fetchSpotAlbumTracksCmd expands an album placeholder from the search results +// into its tracks. Album entries carry no streamable path of their own, so this +// runs before the album can reach the player. +func fetchSpotAlbumTracksCmd(loader provider.AlbumTrackLoader, album playlist.Track, action spotAlbumAction, gen uint64) tea.Cmd { + return func() tea.Msg { + tracks, err := loader.AlbumTracks(album.AlbumID()) + return spotAlbumTracksMsg{tracks: tracks, album: album, action: action, err: err, gen: gen} + } +} + type spotPlaylistsMsg struct { playlists []playlist.PlaylistInfo err error diff --git a/ui/model/inline_overlays.go b/ui/model/inline_overlays.go index e3c7e49..bafa25f 100644 --- a/ui/model/inline_overlays.go +++ b/ui/model/inline_overlays.go @@ -83,6 +83,25 @@ func bodyMessage(msg string, budget int) string { return bodyLines([]string{dimStyle.Render(" " + msg)}, budget) } +// renderSpotSearchResults renders the search results grouped into labeled +// sections, so albums are visibly a different kind of result than the tracks +// below them rather than one long undifferentiated list. +func (m Model) renderSpotSearchResults(budget int) string { + lines := make([]string, 0, budget) + for row := range spotSearchRows(m.spotSearch.results, m.spotSearch.scroll) { + if len(lines) >= budget { + break + } + if row.Index < 0 { + lines = append(lines, dimStyle.Render(labeledSeparator("", row.Section))) + continue + } + label := truncate(fmt.Sprintf("%s - %s", row.Track.Artist, row.Track.Title), ui.PanelWidth-8) + lines = append(lines, cursorLine(label, row.Index == m.spotSearch.cursor)) + } + return strings.Join(padLines(lines, budget, len(lines)), "\n") +} + // renderTrackRowsBody renders a track list with album-header separators into // the playlist-region budget, highlighting the row at cursor. Shared by the // nav browser and playlist manager unfiltered track views. @@ -496,14 +515,13 @@ func (m Model) renderSpotSearchBody() string { var body string switch m.spotSearch.screen { case spotSearchResults: - if len(m.spotSearch.results) == 0 { + switch { + case m.spotSearch.albumLoading: + body = bodyLines([]string{loadingLine("Loading album…")}, budget) + case len(m.spotSearch.results) == 0: body = bodyMessage("No results", budget) - } else { - items := make([]string, len(m.spotSearch.results)) - for i, t := range m.spotSearch.results { - items[i] = truncate(fmt.Sprintf("%s - %s", t.Artist, t.Title), ui.PanelWidth-8) - } - body = windowList(items, m.spotSearch.cursor, m.spotSearch.scroll, budget) + default: + body = m.renderSpotSearchResults(budget) } case spotSearchPlaylist: if m.spotSearch.loading { diff --git a/ui/model/keys_spotify_search.go b/ui/model/keys_spotify_search.go index dff7404..23572d9 100644 --- a/ui/model/keys_spotify_search.go +++ b/ui/model/keys_spotify_search.go @@ -6,6 +6,7 @@ import ( tea "charm.land/bubbletea/v2" + "github.com/bjarneo/cliamp/playlist" "github.com/bjarneo/cliamp/provider" ) @@ -59,6 +60,12 @@ func (m *Model) handleSpotSearchInputKey(msg tea.KeyPressMsg) tea.Cmd { func (m *Model) spotSearchResultsMaybeAdjustScroll(visible int) { clampScroll(&m.spotSearch.cursor, &m.spotSearch.scroll, len(m.spotSearch.results), visible) + // Section separators take rows of their own, so a window sized purely by + // result count can push the cursor off the bottom. + for m.spotSearch.scroll < m.spotSearch.cursor && + spotSearchRowsToCursor(m.spotSearch.results, m.spotSearch.scroll, m.spotSearch.cursor) > visible { + m.spotSearch.scroll++ + } } // handleSpotSearchResultsKey handles navigation through search results. @@ -84,26 +91,42 @@ func (m *Model) handleSpotSearchResultsKey(msg tea.KeyPressMsg) tea.Cmd { } m.spotSearchResultsMaybeAdjustScroll(m.spotSearchResultsVisible()) case "enter": - if count > 0 && !m.spotSearch.loading { + if count > 0 && !m.spotSearchBusy() { track := m.spotSearch.results[m.spotSearch.cursor] + if track.IsAlbum() { + return m.expandSpotAlbum(track, spotAlbumPlay) + } m.closeSpotSearch() return m.playTrackImmediate(track) } case "a": - if count > 0 && !m.spotSearch.loading { + if count > 0 && !m.spotSearchBusy() { track := m.spotSearch.results[m.spotSearch.cursor] + if track.IsAlbum() { + return m.expandSpotAlbum(track, spotAlbumAppend) + } m.closeSpotSearch() return m.appendTrack(track) } case "q": - if count > 0 && !m.spotSearch.loading { + if count > 0 && !m.spotSearchBusy() { track := m.spotSearch.results[m.spotSearch.cursor] + if track.IsAlbum() { + return m.expandSpotAlbum(track, spotAlbumQueueNext) + } m.closeSpotSearch() return m.queueTrackNext(track) } case "p": - if count > 0 && !m.spotSearch.loading { - m.spotSearch.selTrack = m.spotSearch.results[m.spotSearch.cursor] + if count > 0 && !m.spotSearchBusy() { + track := m.spotSearch.results[m.spotSearch.cursor] + // The playlist picker adds one track; an album is many, and Spotify + // has no single call to add a whole record. + if track.IsAlbum() { + m.spotSearch.err = "Open the album with Enter, then add tracks from the queue." + return nil + } + m.spotSearch.selTrack = track m.spotSearch.loading = true m.spotSearch.err = "" return fetchSpotPlaylistsCmd(m.spotSearch.prov, nextRequest(&m.requests.spotLists)) @@ -136,6 +159,26 @@ func (m *Model) handleSpotSearchResultsKey(msg tea.KeyPressMsg) tea.Cmd { return nil } +// spotSearchBusy reports whether a request for the results screen is in flight, +// covering both the playlist fetch and an album expansion. +func (m *Model) spotSearchBusy() bool { + return m.spotSearch.loading || m.spotSearch.albumLoading +} + +// expandSpotAlbum fetches the tracks of the selected album placeholder. The +// overlay stays open while it runs: closing it would bump the request +// generation and drop the response. +func (m *Model) expandSpotAlbum(album playlist.Track, action spotAlbumAction) tea.Cmd { + loader, ok := m.spotSearch.prov.(provider.AlbumTrackLoader) + if !ok { + m.spotSearch.err = "This provider cannot open albums." + return nil + } + m.spotSearch.albumLoading = true + m.spotSearch.err = "" + return fetchSpotAlbumTracksCmd(loader, album, action, nextRequest(&m.requests.spotAlbum)) +} + func (m *Model) spotSearchPlaylistMaybeAdjustScroll(visible int) { count := len(m.spotSearch.playlists) + 1 clampScroll(&m.spotSearch.cursor, &m.spotSearch.scroll, count, max(1, visible-1)) diff --git a/ui/model/playback.go b/ui/model/playback.go index a17158f..f45e2bc 100644 --- a/ui/model/playback.go +++ b/ui/model/playback.go @@ -139,6 +139,64 @@ func (m *Model) appendTrack(track playlist.Track) tea.Cmd { return nil } +// playAlbumImmediate appends an expanded album to the queue and starts it at +// its first track. Like playTrackImmediate it adds rather than replaces, so a +// queue built up over an evening survives picking an album from search. +func (m *Model) playAlbumImmediate(album playlist.Track, tracks []playlist.Track) tea.Cmd { + m.player.Stop() + m.player.ClearPreload() + idx := m.playlist.Len() + m.playlist.Add(tracks...) + m.loadedPlaylist = "" + m.addToHeaderState(tracks) + m.playlist.SetIndex(idx) + m.plCursor = idx + m.adjustScroll() + m.status.Showf(statusTTLMedium, "Playing album: %s (%d tracks)", album.Title, len(tracks)) + cmd := m.playCurrentTrack() + m.notifyPlayback() + return cmd +} + +// appendAlbum appends an expanded album to the queue; auto-plays from its first +// track if nothing is playing. +func (m *Model) appendAlbum(album playlist.Track, tracks []playlist.Track) tea.Cmd { + wasEmpty := m.playlist.Len() == 0 + idx := m.playlist.Len() + m.playlist.Add(tracks...) + m.loadedPlaylist = "" + m.addToHeaderState(tracks) + m.status.Showf(statusTTLMedium, "Added album: %s (%d tracks)", album.Title, len(tracks)) + if wasEmpty || !m.player.IsPlaying() { + m.playlist.SetIndex(idx) + m.plCursor = idx + m.adjustScroll() + cmd := m.playCurrentTrack() + m.notifyPlayback() + return cmd + } + return nil +} + +// queueAlbumNext queues a whole album to play after the current track, keeping +// its running order. +func (m *Model) queueAlbumNext(album playlist.Track, tracks []playlist.Track) tea.Cmd { + idx := m.playlist.Len() + m.playlist.Add(tracks...) + m.loadedPlaylist = "" + m.addToHeaderState(tracks) + for i := range tracks { + m.playlist.Queue(idx + i) + } + m.status.Showf(statusTTLMedium, "Queued album: %s (%d tracks)", album.Title, len(tracks)) + if !m.player.IsPlaying() { + cmd := m.nextTrack() + m.notifyPlayback() + return cmd + } + return nil +} + // closeNetSearch fully resets the net search overlay and restores focus, // dropping any cached results so they don't linger between sessions. func (m *Model) closeNetSearch() { diff --git a/ui/model/spot_search_sections_test.go b/ui/model/spot_search_sections_test.go new file mode 100644 index 0000000..a4c7191 --- /dev/null +++ b/ui/model/spot_search_sections_test.go @@ -0,0 +1,123 @@ +package model + +import ( + "testing" + + "github.com/bjarneo/cliamp/playlist" +) + +func albumResult(name string) playlist.Track { + return playlist.Track{ + Title: name, Album: name, Artist: "NOFX", + ProviderMeta: map[string]string{ + playlist.MetaKind: playlist.MetaKindAlbum, + playlist.MetaAlbumID: name, + }, + } +} + +func trackResult(name string) playlist.Track { + return playlist.Track{Title: name, Artist: "NOFX"} +} + +func TestSpotSearchRowsSections(t *testing.T) { + results := []playlist.Track{ + albumResult("Punk In Drublic"), + albumResult("The Decline"), + trackResult("Linoleum"), + trackResult("Bob"), + } + + tests := []struct { + name string + scroll int + want []string // "=Albums" for a separator, otherwise the title + }{ + { + name: "separates albums from tracks", + scroll: 0, + want: []string{"=Albums", "Punk In Drublic", "The Decline", "=Tracks", "Linoleum", "Bob"}, + }, + { + // Scrolled past the album header, the section must still be named. + name: "repeats the header when the viewport opens mid-section", + scroll: 1, + want: []string{"=Albums", "The Decline", "=Tracks", "Linoleum", "Bob"}, + }, + { + name: "names the tracks section when albums are scrolled away", + scroll: 3, + want: []string{"=Tracks", "Bob"}, + }, + { + name: "yields nothing past the end", + scroll: 4, + want: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got []string + for row := range spotSearchRows(results, tt.scroll) { + if row.Index < 0 { + got = append(got, "="+row.Section) + continue + } + got = append(got, row.Track.Title) + } + if len(got) != len(tt.want) { + t.Fatalf("rows = %v, want %v", got, tt.want) + } + for i := range got { + if got[i] != tt.want[i] { + t.Fatalf("rows = %v, want %v", got, tt.want) + } + } + }) + } +} + +func TestSpotSearchRowsToCursorCountsSeparators(t *testing.T) { + results := []playlist.Track{ + albumResult("Punk In Drublic"), + albumResult("The Decline"), + trackResult("Linoleum"), + } + + tests := []struct { + name string + scroll, cursor int + want int + }{ + {name: "first result sits below its header", scroll: 0, cursor: 0, want: 2}, + {name: "second album adds one row", scroll: 0, cursor: 1, want: 3}, + {name: "crossing into tracks costs a second header", scroll: 0, cursor: 2, want: 5}, + {name: "cursor above scroll counts nothing", scroll: 2, cursor: 1, want: 0}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := spotSearchRowsToCursor(results, tt.scroll, tt.cursor); got != tt.want { + t.Errorf("spotSearchRowsToCursor(%d, %d) = %d, want %d", tt.scroll, tt.cursor, got, tt.want) + } + }) + } +} + +// The separators must not push the selected result out of the window. +func TestSpotSearchResultsScrollKeepsCursorVisible(t *testing.T) { + m := &Model{} + for i := range 6 { + m.spotSearch.results = append(m.spotSearch.results, albumResult("Album "+string(rune('A'+i)))) + } + m.spotSearch.results = append(m.spotSearch.results, trackResult("Linoleum")) + m.spotSearch.cursor = 6 + + const visible = 5 + m.spotSearchResultsMaybeAdjustScroll(visible) + + if rows := spotSearchRowsToCursor(m.spotSearch.results, m.spotSearch.scroll, m.spotSearch.cursor); rows > visible { + t.Errorf("cursor sits %d rows below scroll %d, window is %d", rows, m.spotSearch.scroll, visible) + } +} diff --git a/ui/model/state.go b/ui/model/state.go index 5bbbc0d..6ff3a0a 100644 --- a/ui/model/state.go +++ b/ui/model/state.go @@ -244,6 +244,7 @@ type requestState struct { lyrics uint64 netSearch uint64 spotSearch uint64 + spotAlbum uint64 spotLists uint64 spotMutation uint64 auth uint64 @@ -269,19 +270,22 @@ const ( // spotSearchState holds state for the provider search + add-to-playlist overlay. type spotSearchState struct { - prov playlist.Provider // the provider being searched (may differ from active provider) - visible bool - screen spotSearchScreenType - query string - results []playlist.Track - cursor int - scroll int - loading bool - playlists []playlist.PlaylistInfo // user's Spotify playlists for picker - selTrack playlist.Track // track selected to add - newName string // new playlist name input - err string - cancel func() + prov playlist.Provider // the provider being searched (may differ from active provider) + visible bool + screen spotSearchScreenType + query string + results []playlist.Track + cursor int + scroll int + loading bool + // albumLoading is separate from loading so the results screen can say an + // album is being expanded without claiming so during the playlist fetch. + albumLoading bool + playlists []playlist.PlaylistInfo // user's Spotify playlists for picker + selTrack playlist.Track // track selected to add + newName string // new playlist name input + err string + cancel func() } // catalogBatchState holds state for lazy-loading catalog entries from a provider.CatalogLoader. diff --git a/ui/model/update.go b/ui/model/update.go index 14c9830..334d856 100644 --- a/ui/model/update.go +++ b/ui/model/update.go @@ -733,6 +733,31 @@ func (m Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.clampActiveScrollState() return m, nil + case spotAlbumTracksMsg: + if msg.gen != m.requests.spotAlbum { + return m, nil + } + m.spotSearch.albumLoading = false + if msg.err != nil { + m.spotSearch.err = msg.err.Error() + return m, nil + } + if len(msg.tracks) == 0 { + m.spotSearch.err = "That album has no tracks available here." + return m, nil + } + album := msg.album + tracks := msg.tracks + m.closeSpotSearch() + switch msg.action { + case spotAlbumAppend: + return m, m.appendAlbum(album, tracks) + case spotAlbumQueueNext: + return m, m.queueAlbumNext(album, tracks) + default: + return m, m.playAlbumImmediate(album, tracks) + } + case spotPlaylistsMsg: if !m.isCurrentSpotListRequest(msg.gen, msg.providerName) { return m, nil diff --git a/ui/model/view_helpers.go b/ui/model/view_helpers.go index 373b6a7..14cfc16 100644 --- a/ui/model/view_helpers.go +++ b/ui/model/view_helpers.go @@ -263,6 +263,65 @@ func (m Model) playlistRows(tracks []playlist.Track, scroll int, showHeaders boo } } +// spotSearchRow is one rendered row of the provider search results: a section +// separator when Index is negative, otherwise the result at Index. +type spotSearchRow struct { + Index int + Track playlist.Track + Section string +} + +// spotSearchSection names the section a search result belongs to. Albums are +// placeholders that expand into a record; everything else plays as-is. +func spotSearchSection(t playlist.Track) string { + if t.IsAlbum() { + return "Albums" + } + return "Tracks" +} + +// spotSearchRows walks the search results from scroll, emitting a separator +// whenever the section changes. The provider returns albums first, so this +// yields at most two headers, plus a sticky one at the top of the viewport so +// the section stays named while scrolling through a long run of results. +func spotSearchRows(results []playlist.Track, scroll int) iter.Seq[spotSearchRow] { + return func(yield func(spotSearchRow) bool) { + if len(results) == 0 || scroll < 0 || scroll >= len(results) { + return + } + + prev := "" + for i := scroll; i < len(results); i++ { + section := spotSearchSection(results[i]) + if section != prev { + if !yield(spotSearchRow{Index: -1, Section: section}) { + return + } + } + if !yield(spotSearchRow{Index: i, Track: results[i]}) { + return + } + prev = section + } + } +} + +// spotSearchRowsToCursor counts rendered rows from scroll to cursor inclusive, +// separators included, so scrolling can account for the space they take. +func spotSearchRowsToCursor(results []playlist.Track, scroll, cursor int) int { + if len(results) == 0 || scroll < 0 || cursor < scroll || cursor >= len(results) { + return 0 + } + rows := 0 + for row := range spotSearchRows(results, scroll) { + rows++ + if row.Index == cursor { + break + } + } + return rows +} + // albumSeparatorRows counts rendered rows between scroll and cursor (inclusive) // in a playlist view that emits an album-separator row whenever the album // changes. Streaming tracks are treated as not contributing a separator,