Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Global search #40

Merged
merged 5 commits into from
Sep 19, 2024
Merged

Global search #40

merged 5 commits into from
Sep 19, 2024

Conversation

xxxserxxx
Copy link

This PR implements global search.

This adds a new "search" tab, which uses the Subsonic API to perform a DB search for artists, albums, and songs matching a search term. It filters the results into three columns (artist, album, and song), from where the user can add items to the queue.

The code uses Subsonic's search3, which was added in version API v1.8.0 (Subsonic v4.7); I couldn't find release dates, but given the fact that the current Subsonic version is 6.1.x, and Gonic implements search3, that most servers implementing the Subsonic API will support it.

Results are loaded in batches of 20, and are essentially instant, so no spinner is necessary. Sadly, while this means search3 supports paging, it critically doesn't report how many total results there are, preventing us from doing a background load with progress bar.

As a design decision, I chose to let the user load additional search results with the n binding, rather than implementing a spinner and loading in the background until there are no more results. I did this mainly because it was easier, but it also makes the client behave better in case the user starts a query for e.g. "n" -- which would end up loading nearly the entire DB into the song column. This could be mitigated by, e.g., cancelling the query when the user navigates away from the search tab, so it's still an option if we want to go that route. For now, this works and avoids a lot of extra work to make sure things behave correctly.

Artist and Album loading are handled different than in the browser tab. This is because of how search results are returned, and it prevents some undesirable edge cases. For example, if the user searches for "Black Sabbath," compilation albums are returned where at least one song is by Black Sabbath. If the user were to add that album through the UI, all of the songs on the album, including those not authored by Black Sabbath, would be added. This implementation ensures that only search result matches are added to the queue when the user adds either an artist or an album match.

In gui_handlers.go, handlePageInput() followed a pattern that didn't make sense for the function, so there's some cleanups in there. That pattern makes sense when we have to check for multiple different types of events, so we have to return nil on a successful catch; in this function, we can catch when we don't recognize the key and return the event, and return nil otherwise. It's not necessary for the feature, it just got lumped in because of another feature-relevant change to the function.

Also in this, I factored out the hard-coded indexes to buttonOrder into consts. We discussed it in another PR -- again, a clean-up that's not strictly necessary for the feature. I was motivated by the fact that changing the contents of buttonOrder can introduce tracking down and changing indexes in several places across multiple files; this fixes that.

@spezifisch spezifisch merged commit 1dc709d into spezifisch:main Sep 19, 2024
@spezifisch
Copy link
Owner

The 3 pane layout is a cool solution. I like it!
I'm having problems with the album column though. I'm using Navidrome v0.53.1 (latest Docker image).
Same thing can be reproduced with Navidrome's demo server, see https://github.com/spezifisch/stmps/blob/ecbb9df644ece15dabf0ddd99aac44fac5ec9e73/stmp-navidromedemo.toml:
2024-09-19T19:06:57,538497983+02:00

I merged it in this state though since it doesn't break any other features and apparently works for Gonic.

@xxxserxxx
Copy link
Author

Ok, thank you -- I think I have a Navidrome container on my music server that I was using before I switched to Gonic, so I'll try to reproduce (and fix) it.

Also, I meant to mention -- there are now screenshots in the README that need to be updated (because they're missing the search tab), and I haven't done that. Want me to create a ticket for it?

@xxxserxxx xxxserxxx deleted the global-search-2 branch September 19, 2024 17:35
@spezifisch
Copy link
Owner

Also, I meant to mention -- there are now screenshots in the README that need to be updated (because they're missing the search tab), and I haven't done that. Want me to create a ticket for it?

That's fine for now.

@xxxserxxx
Copy link
Author

xxxserxxx commented Sep 23, 2024

I'm having problems with the album column though. I'm using Navidrome v0.53.1 (latest Docker image). Same thing can be reproduced with Navidrome's demo server

Ok, it turns out that the Subsonic API definition is sufficiently vague as to leave some implementation details to the servers, and Gonic and Navidrome implement search3 results slightly differently. I'm submitting a patch for this in a moment, and will explain more there.

Edit Fixed by #45. Tested against both Gonic and Navidrome (latest).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants