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

add multi-line genre editing #928

Merged
merged 2 commits into from
Jan 30, 2024

Conversation

gaycodegal
Copy link
Contributor

Adds new string and updates album and song tag editors to allow the one-per-line style input. Genre parsing already handles this, so this was only a UI change

@gaycodegal
Copy link
Contributor Author

fixes #914

Copy link
Collaborator

@soncaokim soncaokim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please drop the album_genre_multiple and use 'genre_multipe` instead string?

app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
@gaycodegal
Copy link
Contributor Author

gaycodegal commented Jan 30, 2024

Now I do agree with you but if you look at the artist line, the same logic could apply there. If you look up the artist id3 tags they should be being set per song (mapped across all songs in the album) and not on a album wide basis (e.g in a specific album-artist field), unless we're doing something extra spicy behind the scenes (I didn't check, just assuming). So would you like me to also remove the album_artist_multiple string and just use artist_multiple?

This would reduce translation burden

@soncaokim
Copy link
Collaborator

soncaokim commented Jan 30, 2024 via email

Copy link
Collaborator

@soncaokim soncaokim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@soncaokim soncaokim merged commit 9fed4a1 into VinylMusicPlayer:master Jan 30, 2024
@soncaokim soncaokim mentioned this pull request Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants