Skip to content

Tags: Make method names consistent #171

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.

Sign up for GitHub

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

Merged
Serial-ATA merged 1 commit into from
Mar 18, 2023
Merged

Tags: Make method names consistent #171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Keys will now be verified according to spec before insertion
- Getters will now case-insensitively search for keys
- `TRACKNUM` will now be considered in the `Accessor::*track` methods
- **Tags**: Method names are more consistent

### Fixed
- **ID3v2**:
Expand Down
62 changes: 48 additions & 14 deletions src/ape/tag/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ macro_rules! impl_accessor {
$(
fn $name(&self) -> Option<Cow<'_, str>> {
$(
if let Some(i) = self.get_key($key) {
if let Some(i) = self.get($key) {
if let ItemValue::Text(val) = i.value() {
return Some(Cow::Borrowed(val));
}
Expand All @@ -43,7 +43,7 @@ macro_rules! impl_accessor {

fn [<remove_ $name>](&mut self) {
$(
self.remove_key($key);
self.remove($key);
)+
}
)+
Expand Down Expand Up @@ -94,7 +94,21 @@ impl ApeTag {
///
/// NOTE: While `APE` items are supposed to be case-sensitive,
/// this rule is rarely followed, so this will ignore case when searching.
pub fn get_key(&self, key: &str) -> Option<&ApeItem> {
///
/// # Examples
///
/// ```rust
/// use lofty::ape::ApeTag;
/// use lofty::Accessor;
///
/// let mut ape_tag = ApeTag::new();
/// ape_tag.set_title(String::from("Foo title"));
///
/// // Get the title by its key
/// let title = ape_tag.get("Title");
/// assert!(title.is_some());
/// ```
pub fn get(&self, key: &str) -> Option<&ApeItem> {
self.items
.iter()
.find(|i| i.key().eq_ignore_ascii_case(key))
Expand All @@ -104,22 +118,42 @@ impl ApeTag {
///
/// This will remove any item with the same key prior to insertion
pub fn insert(&mut self, value: ApeItem) {
self.remove_key(value.key());
self.remove(value.key());
self.items.push(value);
}

/// Remove an [`ApeItem`] by key
///
/// NOTE: Like [`ApeTag::get_key`], this is not case-sensitive
pub fn remove_key(&mut self, key: &str) {
/// NOTE: Like [`ApeTag::get`], this is not case-sensitive
///
/// # Examples
///
/// ```rust
/// use lofty::ape::ApeTag;
/// use lofty::Accessor;
///
/// let mut ape_tag = ApeTag::new();
/// ape_tag.set_title(String::from("Foo title"));
///
/// // Get the title by its key
/// let title = ape_tag.get("Title");
/// assert!(title.is_some());
///
/// // Remove the title
/// ape_tag.remove("Title");
///
/// let title = ape_tag.get("Title");
/// assert!(title.is_none());
/// ```
pub fn remove(&mut self, key: &str) {
self.items.retain(|i| !i.key().eq_ignore_ascii_case(key));
}

fn split_num_pair(&self, key: &str) -> (Option<u32>, Option<u32>) {
if let Some(ApeItem {
value: ItemValue::Text(ref text),
..
}) = self.get_key(key)
}) = self.get(key)
{
let mut split = text.split('/').flat_map(str::parse::<u32>);
return (split.next(), split.next());
Expand Down Expand Up @@ -165,7 +199,7 @@ impl Accessor for ApeTag {
}

fn remove_track(&mut self) {
self.remove_key("Track");
self.remove("Track");
}

fn track_total(&self) -> Option<u32> {
Expand All @@ -180,7 +214,7 @@ impl Accessor for ApeTag {

fn remove_track_total(&mut self) {
let existing_track_number = self.track();
self.remove_key("Track");
self.remove("Track");

if let Some(track) = existing_track_number {
self.insert(ApeItem::text("Track", track.to_string()));
Expand All @@ -196,7 +230,7 @@ impl Accessor for ApeTag {
}

fn remove_disk(&mut self) {
self.remove_key("Disc");
self.remove("Disc");
}

fn disk_total(&self) -> Option<u32> {
Expand All @@ -211,7 +245,7 @@ impl Accessor for ApeTag {

fn remove_disk_total(&mut self) {
let existing_track_number = self.track();
self.remove_key("Disc");
self.remove("Disc");

if let Some(track) = existing_track_number {
self.insert(ApeItem::text("Disc", track.to_string()));
Expand All @@ -222,7 +256,7 @@ impl Accessor for ApeTag {
if let Some(ApeItem {
value: ItemValue::Text(ref text),
..
}) = self.get_key("Year")
}) = self.get("Year")
{
return try_parse_year(text);
}
Expand All @@ -235,7 +269,7 @@ impl Accessor for ApeTag {
}

fn remove_year(&mut self) {
self.remove_key("Year");
self.remove("Year");
}
}

Expand Down Expand Up @@ -577,7 +611,7 @@ mod tests {
fn tag_to_ape() {
fn verify_key(tag: &ApeTag, key: &str, expected_val: &str) {
assert_eq!(
tag.get_key(key).map(ApeItem::value),
tag.get(key).map(ApeItem::value),
Some(&ItemValue::Text(String::from(expected_val)))
);
}
Expand Down
44 changes: 22 additions & 22 deletions src/id3/v2/tag.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1243,7 +1243,7 @@ mod tests {
#[test]
fn tag_to_id3v2_popm() {
let mut tag = Tag::new(TagType::ID3v2);
tag.insert_item(TagItem::new(
tag.insert(TagItem::new(
ItemKey::Popularimeter,
ItemValue::Binary(vec![
b'f', b'o', b'o', b'@', b'b', b'a', b'r', b'.', b'c', b'o', b'm', 0, 196, 0, 0,
Expand Down Expand Up @@ -1535,15 +1535,15 @@ mod tests {
use crate::traits::Accessor;
let mut tag = Tag::new(TagType::ID3v2);

tag.push_item_unchecked(TagItem::new(
tag.push_unchecked(TagItem::new(
ItemKey::TrackArtist,
ItemValue::Text(String::from("foo")),
));
tag.push_item_unchecked(TagItem::new(
tag.push_unchecked(TagItem::new(
ItemKey::TrackArtist,
ItemValue::Text(String::from("bar")),
));
tag.push_item_unchecked(TagItem::new(
tag.push_unchecked(TagItem::new(
ItemKey::TrackArtist,
ItemValue::Text(String::from("baz")),
));
Expand Down Expand Up @@ -1590,53 +1590,53 @@ mod tests {
let mut tag = Tag::new(TagType::ID3v2);
// 1st: Multi-valued text frames
tag.insert_text(ItemKey::TrackArtist, "TrackArtist 1".to_owned());
tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::TrackArtist,
ItemValue::Text("TrackArtist 2".to_owned()),
));
tag.insert_text(ItemKey::AlbumArtist, "AlbumArtist 1".to_owned());
tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::AlbumArtist,
ItemValue::Text("AlbumArtist 2".to_owned()),
));
tag.insert_text(ItemKey::TrackTitle, "TrackTitle 1".to_owned());
tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::TrackTitle,
ItemValue::Text("TrackTitle 2".to_owned()),
));
tag.insert_text(ItemKey::AlbumTitle, "AlbumTitle 1".to_owned());
tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::AlbumTitle,
ItemValue::Text("AlbumTitle 2".to_owned()),
));
tag.insert_text(ItemKey::ContentGroup, "ContentGroup 1".to_owned());
tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::ContentGroup,
ItemValue::Text("ContentGroup 2".to_owned()),
));
tag.insert_text(ItemKey::Genre, "Genre 1".to_owned());
tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::Genre,
ItemValue::Text("Genre 2".to_owned()),
));
tag.insert_text(ItemKey::Mood, "Mood 1".to_owned());
tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::Mood,
ItemValue::Text("Mood 2".to_owned()),
));
tag.insert_text(ItemKey::Composer, "Composer 1".to_owned());
tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::Composer,
ItemValue::Text("Composer 2".to_owned()),
));
tag.insert_text(ItemKey::Conductor, "Conductor 1".to_owned());
tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::Conductor,
ItemValue::Text("Conductor 2".to_owned()),
));
// 2nd: Multi-valued language frames
tag.insert_text(ItemKey::Comment, "Comment 1".to_owned());
tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::Comment,
ItemValue::Text("Comment 2".to_owned()),
));
Expand Down Expand Up @@ -1909,7 +1909,7 @@ mod tests {

let mut tag = Tag::new(TagType::ID3v2);

tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::TrackNumber,
ItemValue::Text(track_number.to_string()),
));
Expand All @@ -1927,7 +1927,7 @@ mod tests {

let mut tag = Tag::new(TagType::ID3v2);

tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::TrackTotal,
ItemValue::Text(track_total.to_string()),
));
Expand All @@ -1946,12 +1946,12 @@ mod tests {

let mut tag = Tag::new(TagType::ID3v2);

tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::TrackNumber,
ItemValue::Text(track_number.to_string()),
));

tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::TrackTotal,
ItemValue::Text(track_total.to_string()),
));
Expand All @@ -1969,7 +1969,7 @@ mod tests {

let mut tag = Tag::new(TagType::ID3v2);

tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::DiscNumber,
ItemValue::Text(disk_number.to_string()),
));
Expand All @@ -1987,7 +1987,7 @@ mod tests {

let mut tag = Tag::new(TagType::ID3v2);

tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::DiscTotal,
ItemValue::Text(disk_total.to_string()),
));
Expand All @@ -2006,12 +2006,12 @@ mod tests {

let mut tag = Tag::new(TagType::ID3v2);

tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::DiscNumber,
ItemValue::Text(disk_number.to_string()),
));

tag.push_item(TagItem::new(
tag.push(TagItem::new(
ItemKey::DiscTotal,
ItemValue::Text(disk_total.to_string()),
));
Expand Down
4 changes: 2 additions & 2 deletions src/iff/aiff/tag.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -581,11 +581,11 @@ mod tests {
tag.insert_text(ItemKey::TrackTitle, String::from("Foo title"));
tag.insert_text(ItemKey::TrackArtist, String::from("Bar artist"));
tag.insert_text(ItemKey::CopyrightMessage, String::from("Baz copyright"));
tag.push_item_unchecked(TagItem::new(
tag.push_unchecked(TagItem::new(
ItemKey::Comment,
ItemValue::Text(String::from("Qux annotation")),
));
tag.push_item_unchecked(TagItem::new(
tag.push_unchecked(TagItem::new(
ItemKey::Comment,
ItemValue::Text(String::from("Quux annotation")),
));
Expand Down
Loading