cbd-tui: no empty parentheses in the now-playing notification
`Album.release_date` is an optional proto field and most providers never set it, so the notification body read "album ()" for nearly every track: the parentheses were part of the format string rather than of the date. Move the body into `notification_body`, where the parentheses belong to the date and an absent one (unset or empty, since prost's accessor returns "" for both) simply drops them, and cover the three shapes with tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
2bfdac25a7
commit
bbebee0b4f
|
|
@ -619,18 +619,7 @@ impl NowPlaying {
|
|||
/// if the binary is renamed or wrapped.
|
||||
#[cfg(feature = "notifications")]
|
||||
fn notify_now_playing(track: &Track) {
|
||||
let body = if let Some(ref album) = track.album {
|
||||
format!(
|
||||
"{} by {}\n\n{} ({})",
|
||||
track.title,
|
||||
track.artist,
|
||||
album.title,
|
||||
// FIXME: get out year and format differently if it's missing
|
||||
album.release_date()
|
||||
)
|
||||
} else {
|
||||
format!("{} by {}", track.title, track.artist)
|
||||
};
|
||||
let body = notification_body(track);
|
||||
if let Err(err) = Notification::new()
|
||||
.appname("crabidy")
|
||||
.summary("Now playing")
|
||||
|
|
@ -641,6 +630,26 @@ fn notify_now_playing(track: &Track) {
|
|||
}
|
||||
}
|
||||
|
||||
/// The notification body: the track, and on its own line the album with the
|
||||
/// release date the provider gave. Most providers leave `release_date` unset
|
||||
/// (it is an optional proto field, so absent reads as ""), which is why the
|
||||
/// parentheses belong to the date and not to the album line — otherwise a
|
||||
/// dateless album shows up as "Album ()".
|
||||
#[cfg(feature = "notifications")]
|
||||
fn notification_body(track: &Track) -> String {
|
||||
let Some(album) = &track.album else {
|
||||
return format!("{} by {}", track.title, track.artist);
|
||||
};
|
||||
let released = match album.release_date() {
|
||||
"" => String::new(),
|
||||
date => format!(" ({date})"),
|
||||
};
|
||||
format!(
|
||||
"{} by {}\n\n{}{released}",
|
||||
track.title, track.artist, album.title
|
||||
)
|
||||
}
|
||||
|
||||
/// Built without the `notifications` feature: nothing to show
|
||||
/// (architecture/build-features.md D1).
|
||||
#[cfg(not(feature = "notifications"))]
|
||||
|
|
@ -679,6 +688,43 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
/// The notification body for a track on `album`, which carries
|
||||
/// `release_date`.
|
||||
#[cfg(feature = "notifications")]
|
||||
fn body_for(release_date: Option<&str>) -> String {
|
||||
let mut track = now_playing(0, 0).track.expect("track");
|
||||
track.album = Some(crabidy_core::proto::crabidy::Album {
|
||||
title: "album".to_string(),
|
||||
release_date: release_date.map(str::to_string),
|
||||
});
|
||||
notification_body(&track)
|
||||
}
|
||||
|
||||
#[cfg(feature = "notifications")]
|
||||
#[test]
|
||||
fn notification_shows_the_release_date_when_there_is_one() {
|
||||
assert_eq!(
|
||||
body_for(Some("1977-10-28")),
|
||||
"title by artist\n\nalbum (1977-10-28)"
|
||||
);
|
||||
}
|
||||
|
||||
/// No date must mean no parentheses, whether the provider left the field
|
||||
/// unset or sent it empty.
|
||||
#[cfg(feature = "notifications")]
|
||||
#[test]
|
||||
fn notification_omits_empty_parentheses() {
|
||||
assert_eq!(body_for(None), "title by artist\n\nalbum");
|
||||
assert_eq!(body_for(Some("")), "title by artist\n\nalbum");
|
||||
}
|
||||
|
||||
#[cfg(feature = "notifications")]
|
||||
#[test]
|
||||
fn notification_without_an_album_is_just_the_track() {
|
||||
let track = now_playing(0, 0).track.expect("track");
|
||||
assert_eq!(notification_body(&track), "title by artist");
|
||||
}
|
||||
|
||||
/// One flat color, no markers: the glyph tests are about the bars'
|
||||
/// geometry, so they pin the paint down and let the color tests own it.
|
||||
fn flat() -> SpectrumStyle {
|
||||
|
|
|
|||
Loading…
Reference in New Issue