pick random song from tracklist#339
Conversation
780ec70 to
50400ab
Compare
50400ab to
b368296
Compare
There was a problem hiding this comment.
This looks like a useful feature, thank you @BKitor!
I've left a few comments that should simplify the implementation a little. Take a look and let me know if I've preserved the functionality 👍
At the end of the review, I've rewritten the play_random_song function with all my requested changes.
| } | ||
|
|
||
| fn play_random_song(app: &mut App) { | ||
| let TrackTable { context, .. } = &app.track_table; |
There was a problem hiding this comment.
We can reduce the nesting here by avoiding destructuring e.g.
if let Some(context) = &app.track_table.context {
match context {|
@Rigellute Thanks for the advice! I appreciate it! |
Rigellute
left a comment
There was a problem hiding this comment.
Thanks for pointing out that we do need to deserialise the total number of tracks in a playlist.
I've just left a few more comments - main one is about removing unwrap, what do you think?
| } | ||
|
|
||
| fn play_random_song(app: &mut App) { | ||
| // let TrackTable { context, .. } = &app.track_table; |
There was a problem hiding this comment.
Are you able to remove this commented line?
| .library | ||
| .made_for_you_playlists | ||
| .get_results(Some(0)) | ||
| .unwrap() |
There was a problem hiding this comment.
Are you able to remove these unwraps? Something like this?
TrackTableContext::MadeForYou => {
if let Some(playlist) = &app
.library
.made_for_you_playlists
.get_results(Some(0))
.and_then(|playlist| playlist.items.get(app.made_for_you_index))
{
if let Some(num_tracks) = &playlist
.tracks
.get("total")
.and_then(|total| -> Option<usize> { from_value(total.clone()).ok() })
{
let uri = Some(playlist.uri.clone());
app.dispatch(IoEvent::StartPlayback(
uri,
None,
Some(thread_rng().gen_range(0, num_tracks)),
));
}
};
}There was a problem hiding this comment.
This works great! I didn't know and_then() existed. Thanks for the tip.
|
@all-contributors please add @BKitor for code |
|
I've put up a pull request to add @BKitor! 🎉 |
…ift-s pick random song from tracklist
When using Spotify I often use their Shuffle Playlist button. Spotify-tui doesn't seem to have that kind of feature. What if Shift-s on Track Table chooses a random song from whatever context is active, and plays it.