Skip to content

Added MPRIS OpenUri support#993

Open
Bettehem wants to merge 1 commit into
aome510:masterfrom
Bettehem:mpris-openuri
Open

Added MPRIS OpenUri support#993
Bettehem wants to merge 1 commit into
aome510:masterfrom
Bettehem:mpris-openuri

Conversation

@Bettehem
Copy link
Copy Markdown

Implemented MPRIS OpenUri and it now works with Spotify URIs, for example:
spotify:track:6CfwYtEnSDgMxosLZ5Vbiu
and
spotify:album:3HNzOyPbz5vPvUie7lI97X

I'm still a beginner at rust so please let me know if there is anything that should be done differently

MPRIS OpenUri now works with Spotify URIs, for example:
spotify:track:6CfwYtEnSDgMxosLZ5Vbiu
spotify:album:3HNzOyPbz5vPvUie7lI97X
Comment on lines +142 to +173
MediaControlEvent::OpenUri(uri) => {
let id = uri[uri.rfind(":").unwrap_or(0) + 1..uri.len()].to_string();
let uri_type = &uri[uri.find(":").unwrap_or(0) + 1..uri.rfind(":").unwrap_or(0)];
let playback: Option<Playback> = match uri_type {
"album" => {
let album_id = AlbumId::from_id(id).unwrap();
Some(Playback::Context(ContextId::Album(album_id), None))
}
"track" => {
let track_id = PlayableId::Track(TrackId::from_id(id).unwrap());
Some(Playback::URIs(vec![track_id], None))
}
"playlist" => {
let playlist_id = PlaylistId::from_id(id).unwrap();
Some(Playback::Context(ContextId::Playlist(playlist_id), None))
}
"show" => {
let show_id = ShowId::from_id(id).unwrap();
Some(Playback::Context(ContextId::Show(show_id), None))
}
"episode" => {
let episode_id = PlayableId::Episode(EpisodeId::from_id(id).unwrap());
Some(Playback::URIs(vec![episode_id], None))
}
_ => None,
};
if let Some(s) = playback {
client_pub
.send(ClientRequest::Player(PlayerRequest::StartPlayback(s, None)))
.unwrap_or_default();
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would implement it like this:

Suggested change
MediaControlEvent::OpenUri(uri) => {
let id = uri[uri.rfind(":").unwrap_or(0) + 1..uri.len()].to_string();
let uri_type = &uri[uri.find(":").unwrap_or(0) + 1..uri.rfind(":").unwrap_or(0)];
let playback: Option<Playback> = match uri_type {
"album" => {
let album_id = AlbumId::from_id(id).unwrap();
Some(Playback::Context(ContextId::Album(album_id), None))
}
"track" => {
let track_id = PlayableId::Track(TrackId::from_id(id).unwrap());
Some(Playback::URIs(vec![track_id], None))
}
"playlist" => {
let playlist_id = PlaylistId::from_id(id).unwrap();
Some(Playback::Context(ContextId::Playlist(playlist_id), None))
}
"show" => {
let show_id = ShowId::from_id(id).unwrap();
Some(Playback::Context(ContextId::Show(show_id), None))
}
"episode" => {
let episode_id = PlayableId::Episode(EpisodeId::from_id(id).unwrap());
Some(Playback::URIs(vec![episode_id], None))
}
_ => None,
};
if let Some(s) = playback {
client_pub
.send(ClientRequest::Player(PlayerRequest::StartPlayback(s, None)))
.unwrap_or_default();
}
}
MediaControlEvent::OpenUri(uri) => {
let mut split = uri.split(':');
let (Some("spotify"), Some(uri_type), Some(id)) =
(split.next(), split.next(), split.next())
else {
return;
};
let id = id.to_string();
let playback = match uri_type {
"album" => AlbumId::from_id(id)
.ok()
.map(|album_id| Playback::Context(ContextId::Album(album_id), None)),
"track" => TrackId::from_id(id)
.ok()
.map(|track_id| Playback::URIs(vec![PlayableId::Track(track_id)], None)),
"playlist" => PlaylistId::from_id(id).ok().map(|playlist_id| {
Playback::Context(ContextId::Playlist(playlist_id), None)
}),
"show" => ShowId::from_id(id)
.ok()
.map(|show_id| Playback::Context(ContextId::Show(show_id), None)),
"episode" => EpisodeId::from_id(id).ok().map(|episode_id| {
Playback::URIs(vec![PlayableId::Episode(episode_id)], None)
}),
_ => None,
};
if let Some(playback) = playback {
client_pub
.send(ClientRequest::Player(PlayerRequest::StartPlayback(
playback, None,
)))
.unwrap_or_default();
}
}

I'd argue that especially the parsing of the URI is more readable this way, plus it checks that the first part is actually 'spotify'.

The changes in the match statement get rid of the unwrap() calls you had, which is more safe.

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