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

Show all artists in TrackList #613

Merged
merged 5 commits into from
Dec 20, 2021
Merged

Show all artists in TrackList #613

merged 5 commits into from
Dec 20, 2021

Conversation

FKD13
Copy link
Contributor

@FKD13 FKD13 commented Nov 9, 2021

In this PR I have slightly modified the code to show all artists of a track instead of just one.

If I missed things be sure to let me know, its the first code I contribute to the project so it's likely I missed something.

@martpie
Copy link
Owner

martpie commented Nov 10, 2021

This macOS error is fine and expected ;)

Ok, about the feature, I remember explicitly not implementing this, as it may screw up the sorting with some albums where the original artist organize a featuring.

For example:

Let's say there is a BB. King + Eric Clapton song on an Eric Clapton album, the song may not be grouped together with album.

What we could do eventually (and what this PR may be compatible with), is if we only display the artists that way, but we keep the current sorting algorithm, with artist[0]

@FKD13
Copy link
Contributor Author

FKD13 commented Nov 15, 2021

This is true, but this behavior can be achieved by first searching for an artist and then sorting by album name.

The reason I wanted to add this feature is because when you would search for an artist the search results includes all featuring artists. Meaning you could have results with no visual link to the query. e.g. when searching for BB. King you would get a number from Eric Clapton in an Eric Clapton album.

@martpie
Copy link
Owner

martpie commented Dec 20, 2021

Finally found some time to test it, look good to me except one thing, could change parseArtist in sort-orders.ts to adapt to that?

const parseArtist = (t: Track): string => t.loweredMetas.artist[0].toString();

This will actually even be a fix compared to the previous behavior. It will allow Album tracks to be correct ordered by artist -> album -> disc no.

Before:

Screenshot 2021-12-20 at 04 14 12

After:

Screenshot 2021-12-20 at 04 13 53

@martpie martpie merged commit ae6c017 into martpie:master Dec 20, 2021
@martpie
Copy link
Owner

martpie commented Dec 20, 2021

Thanks!

@FKD13 FKD13 deleted the more-artists branch December 20, 2021 20:56
martpie added a commit that referenced this pull request May 25, 2022
Show all artists in TrackList
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