misc: Introduce WriteOptions

This allows the caller to tweak how Lofty writes their tags in various ways.

As this is just a dumping ground for all sorts of format-specific settings, this is best used as an application global config that gets set once.

In its current state, it will only respect `uppercase_id3v2_chunk` and `preferred_padding` (for some formats).

`respect_read_only` and `remove_others` are defined for later use.

closes #228

CHANGELOG.md

@@ -6,6 +6,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+- `WriteOptions` ([issue](https://github.com/Serial-ATA/lofty-rs/issues/228)) ([PR](https://github.com/Serial-ATA/lofty-rs/pull/361)):
+	- ⚠️ Important ⚠️: This update introduces `WriteOptions` to allow for finer grained control over how
+	  Lofty writes tags. These are best used as global user-configurable options, as most options will
+	  not apply to all files. The defaults are set to be as safe as possible,
+	  see [here](https://docs.rs/lofty/latest/lofty/struct.WriteOptions.html#impl-Default-for-WriteOptions).
+
 ## [0.18.2] - 2024-01-23
 
 ### Fixed

Cargo.toml

@@ -18,7 +18,7 @@ byteorder     = "1.5.0"
 # ID3 compressed frames
 flate2        = { version = "1.0.28", optional = true }
 # Proc macros
-lofty_attr    = "0.9.0"
+lofty_attr = { path = "lofty_attr" }
 # Debug logging
 log           = "0.4.20"
 # OGG Vorbis/Opus

examples/tag_writer.rs

@@ -1,4 +1,4 @@
-use lofty::{Accessor, Probe, Tag, TagExt, TaggedFileExt};
+use lofty::{Accessor, Probe, Tag, TagExt, TaggedFileExt, WriteOptions};
 
 use structopt::StructOpt;
 
@@ -75,7 +75,7 @@ fn main() {
 		tag.set_genre(genre)
 	}
 
-	tag.save_to_path(&opt.path)
+	tag.save_to_path(&opt.path, WriteOptions::new())
 		.expect("ERROR: Failed to write the tag!");
 
 	println!("INFO: Tag successfully updated!");

lofty_attr/src/internal.rs

@@ -48,32 +48,32 @@ pub(crate) fn init_write_lookup(
 			read_only: false,
 			items: lofty::ape::tag::tagitems_into_ape(tag),
 		}
-		.write_to(data)
+		.write_to(data, write_options)
 	});
 
 	insert!(map, Id3v1, {
-		Into::<lofty::id3::v1::tag::Id3v1TagRef<'_>>::into(tag).write_to(data)
+		Into::<lofty::id3::v1::tag::Id3v1TagRef<'_>>::into(tag).write_to(data, write_options)
 	});
 
 	if id3v2_strippable {
 		insert!(map, Id3v2, {
-			lofty::id3::v2::tag::Id3v2TagRef::empty().write_to(data)
+			lofty::id3::v2::tag::Id3v2TagRef::empty().write_to(data, write_options)
 		});
 	} else {
 		insert!(map, Id3v2, {
 			lofty::id3::v2::tag::Id3v2TagRef {
 				flags: lofty::id3::v2::Id3v2TagFlags::default(),
 				frames: lofty::id3::v2::tag::tag_frames(tag),
 			}
-			.write_to(data)
+			.write_to(data, write_options)
 		});
 	}
 
 	insert!(map, RiffInfo, {
 		lofty::iff::wav::tag::RIFFInfoListRef::new(lofty::iff::wav::tag::tagitems_into_riff(
 			tag.items(),
 		))
-		.write_to(data)
+		.write_to(data, write_options)
 	});
 
 	insert!(map, AiffText, {
@@ -84,7 +84,7 @@ pub(crate) fn init_write_lookup(
 			annotations: Some(tag.get_strings(&lofty::tag::item::ItemKey::Comment)),
 			comments: None,
 		}
-		.write_to(data)
+		.write_to(data, write_options)
 	});
 
 	map
@@ -111,7 +111,7 @@ pub(crate) fn write_module(
 	quote! {
 		pub(crate) mod write {
 			#[allow(unused_variables)]
-			pub(crate) fn write_to(data: &mut ::std::fs::File, tag: &::lofty::Tag) -> ::lofty::error::Result<()> {
+			pub(crate) fn write_to(data: &mut ::std::fs::File, tag: &::lofty::Tag, write_options: ::lofty::WriteOptions) -> ::lofty::error::Result<()> {
 				match tag.tag_type() {
 					#( #applicable_formats )*
 					_ => crate::macros::err!(UnsupportedTag),

lofty_attr/src/lofty_file.rs

@@ -445,7 +445,7 @@ fn generate_audiofile_impl(file: &LoftyFile) -> syn::Result<proc_macro2::TokenSt
 				#read_fn(reader, parse_options)
 			}
 
-			fn save_to(&self, file: &mut ::std::fs::File) -> ::lofty::error::Result<()> {
+			fn save_to(&self, file: &mut ::std::fs::File, write_options: ::lofty::WriteOptions) -> ::lofty::error::Result<()> {
 				use ::lofty::TagExt as _;
 				use ::std::io::Seek as _;
 				#save_to_body
@@ -480,7 +480,7 @@ fn get_save_to_body(
 	// Custom write fn
 	if let Some(write_fn) = write_fn {
 		return quote! {
-			#write_fn(&self, file)
+			#write_fn(&self, file, write_options)
 		};
 	}
 
@@ -490,13 +490,13 @@ fn get_save_to_body(
 			quote! {
 				if let Some(ref tag) = self.#name {
 					file.rewind()?;
-					tag.save_to(file)?;
+					tag.save_to(file, write_options)?;
 				}
 			}
 		} else {
 			quote! {
 				file.rewind()?;
-				self.#name.save_to(file)?;
+				self.#name.save_to(file, write_options)?;
 			}
 		}
 	});

src/ape/tag/mod.rs

@@ -8,6 +8,7 @@ use crate::id3::v2::util::pairs::{format_number_pair, set_number, NUMBER_PAIR_KE
 use crate::tag::item::{ItemKey, ItemValue, ItemValueRef, TagItem};
 use crate::tag::{try_parse_year, Tag, TagType};
 use crate::traits::{Accessor, MergeTag, SplitTag, TagExt};
+use crate::write_options::WriteOptions;
 
 use std::borrow::Cow;
 use std::fs::File;
@@ -320,25 +321,33 @@ impl TagExt for ApeTag {
 	///
 	/// * Attempting to write the tag to a format that does not support it
 	/// * An existing tag has an invalid size
-	fn save_to(&self, file: &mut File) -> std::result::Result<(), Self::Err> {
+	fn save_to(
+		&self,
+		file: &mut File,
+		write_options: WriteOptions,
+	) -> std::result::Result<(), Self::Err> {
 		ApeTagRef {
 			read_only: self.read_only,
 			items: self.items.iter().map(Into::into),
 		}
-		.write_to(file)
+		.write_to(file, write_options)
 	}
 
 	/// Dumps the tag to a writer
 	///
 	/// # Errors
 	///
 	/// * [`std::io::Error`]
-	fn dump_to<W: Write>(&self, writer: &mut W) -> std::result::Result<(), Self::Err> {
+	fn dump_to<W: Write>(
+		&self,
+		writer: &mut W,
+		write_options: WriteOptions,
+	) -> std::result::Result<(), Self::Err> {
 		ApeTagRef {
 			read_only: self.read_only,
 			items: self.items.iter().map(Into::into),
 		}
-		.dump_to(writer)
+		.dump_to(writer, write_options)
 	}
 
 	fn remove_from_path<P: AsRef<Path>>(&self, path: P) -> std::result::Result<(), Self::Err> {
@@ -482,11 +491,15 @@ impl<'a, I> ApeTagRef<'a, I>
 where
 	I: Iterator<Item = ApeItemRef<'a>>,
 {
-	pub(crate) fn write_to(&mut self, file: &mut File) -> Result<()> {
-		write::write_to(file, self)
+	pub(crate) fn write_to(&mut self, file: &mut File, write_options: WriteOptions) -> Result<()> {
+		write::write_to(file, self, write_options)
 	}
 
-	pub(crate) fn dump_to<W: Write>(&mut self, writer: &mut W) -> Result<()> {
+	pub(crate) fn dump_to<W: Write>(
+		&mut self,
+		writer: &mut W,
+		_write_options: WriteOptions,
+	) -> Result<()> {
 		let temp = write::create_ape_tag(self)?;
 		writer.write_all(&temp)?;
 
@@ -531,7 +544,7 @@ pub(crate) fn tagitems_into_ape(tag: &Tag) -> impl Iterator<Item = ApeItemRef<'_
 #[cfg(test)]
 mod tests {
 	use crate::ape::{ApeItem, ApeTag};
-	use crate::{Accessor, ItemKey, ItemValue, Tag, TagExt, TagItem, TagType};
+	use crate::{Accessor, ItemKey, ItemValue, Tag, TagExt, TagItem, TagType, WriteOptions};
 
 	use crate::id3::v2::util::pairs::DEFAULT_NUMBER_IN_PAIR;
 	use std::io::Cursor;
@@ -608,7 +621,9 @@ mod tests {
 			.unwrap();
 
 		let mut writer = Vec::new();
-		parsed_tag.dump_to(&mut writer).unwrap();
+		parsed_tag
+			.dump_to(&mut writer, WriteOptions::new())
+			.unwrap();
 
 		let mut temp_reader = Cursor::new(writer);
 

src/ape/tag/write.rs

@@ -7,14 +7,19 @@ use crate::id3::{find_id3v1, find_id3v2, find_lyrics3v2, FindId3v2Config};
 use crate::macros::{decode_err, err};
 use crate::probe::Probe;
 use crate::tag::item::ItemValueRef;
+use crate::write_options::WriteOptions;
 
 use std::fs::File;
 use std::io::{Cursor, Read, Seek, SeekFrom, Write};
 
 use byteorder::{LittleEndian, WriteBytesExt};
 
 #[allow(clippy::shadow_unrelated)]
-pub(crate) fn write_to<'a, I>(data: &mut File, tag: &mut ApeTagRef<'a, I>) -> Result<()>
+pub(crate) fn write_to<'a, I>(
+	data: &mut File,
+	tag: &mut ApeTagRef<'a, I>,
+	_write_options: WriteOptions,
+) -> Result<()>
 where
 	I: Iterator<Item = ApeItemRef<'a>>,
 {
@@ -40,6 +45,7 @@ where
 	// If one is found, it'll be removed and rewritten at the bottom, where it should be
 	let mut header_ape_tag = (false, (0, 0));
 
+	// TODO: Respect read only
 	let start = data.stream_position()?;
 	match read::read_ape_tag(data, false)? {
 		Some((mut existing_tag, header)) => {

src/file.rs

@@ -5,6 +5,7 @@ use crate::properties::FileProperties;
 use crate::resolve::custom_resolvers;
 use crate::tag::{Tag, TagType};
 use crate::traits::TagExt;
+use crate::write_options::WriteOptions;
 
 use std::ffi::OsStr;
 use std::fs::{File, OpenOptions};
@@ -39,19 +40,22 @@ pub trait AudioFile: Into<TaggedFile> {
 	/// # Examples
 	///
 	/// ```rust,no_run
-	/// use lofty::{AudioFile, TaggedFileExt};
+	/// use lofty::{AudioFile, TaggedFileExt, WriteOptions};
 	///
 	/// # fn main() -> lofty::Result<()> {
 	/// # let path = "tests/files/assets/minimal/full_test.mp3";
 	/// let mut tagged_file = lofty::read_from_path(path)?;
 	///
 	/// // Edit the tags
 	///
-	/// tagged_file.save_to_path(path)?;
+	/// tagged_file.save_to_path(path, WriteOptions::new())?;
 	/// # Ok(()) }
 	/// ```
-	fn save_to_path(&self, path: impl AsRef<Path>) -> Result<()> {
-		self.save_to(&mut OpenOptions::new().read(true).write(true).open(path)?)
+	fn save_to_path(&self, path: impl AsRef<Path>, write_options: WriteOptions) -> Result<()> {
+		self.save_to(
+			&mut OpenOptions::new().read(true).write(true).open(path)?,
+			write_options,
+		)
 	}
 
 	/// Attempts to write all tags to a file
@@ -63,7 +67,7 @@ pub trait AudioFile: Into<TaggedFile> {
 	/// # Examples
 	///
 	/// ```rust,no_run
-	/// use lofty::{AudioFile, TaggedFileExt};
+	/// use lofty::{AudioFile, TaggedFileExt, WriteOptions};
 	/// use std::fs::OpenOptions;
 	///
 	/// # fn main() -> lofty::Result<()> {
@@ -73,10 +77,10 @@ pub trait AudioFile: Into<TaggedFile> {
 	/// // Edit the tags
 	///
 	/// let mut file = OpenOptions::new().read(true).write(true).open(path)?;
-	/// tagged_file.save_to(&mut file)?;
+	/// tagged_file.save_to(&mut file, WriteOptions::new())?;
 	/// # Ok(()) }
 	/// ```
-	fn save_to(&self, file: &mut File) -> Result<()>;
+	fn save_to(&self, file: &mut File, write_options: WriteOptions) -> Result<()>;
 
 	/// Returns a reference to the file's properties
 	fn properties(&self) -> &Self::Properties;
@@ -492,12 +496,12 @@ impl AudioFile for TaggedFile {
 			.read()
 	}
 
-	fn save_to(&self, file: &mut File) -> Result<()> {
+	fn save_to(&self, file: &mut File, write_options: WriteOptions) -> Result<()> {
 		for tag in &self.tags {
 			// TODO: This is a temporary solution. Ideally we should probe once and use
 			//       the format-specific writing to avoid these rewinds.
 			file.rewind()?;
-			tag.save_to(file)?;
+			tag.save_to(file, write_options)?;
 		}
 
 		Ok(())
@@ -528,7 +532,7 @@ impl From<BoundTaggedFile> for TaggedFile {
 /// For example:
 ///
 /// ```rust,no_run
-/// use lofty::{AudioFile, Tag, TagType, TaggedFileExt};
+/// use lofty::{AudioFile, Tag, TagType, TaggedFileExt, WriteOptions};
 /// # fn main() -> lofty::Result<()> {
 /// # let path = "tests/files/assets/minimal/full_test.mp3";
 ///
@@ -543,15 +547,17 @@ impl From<BoundTaggedFile> for TaggedFile {
 /// // After saving, our file still "contains" the ID3v2 tag, but if we were to read
 /// // "foo.mp3", it would not have an ID3v2 tag. Lofty does not write empty tags, but this
 /// // change will not be reflected in `TaggedFile`.
-/// tagged_file.save_to_path("foo.mp3")?;
+/// tagged_file.save_to_path("foo.mp3", WriteOptions::new())?;
 /// assert!(tagged_file.contains_tag_type(TagType::Id3v2));
 /// # Ok(()) }
 /// ```
 ///
 /// However, when using `BoundTaggedFile`:
 ///
 /// ```rust,no_run
-/// use lofty::{AudioFile, BoundTaggedFile, ParseOptions, Tag, TagType, TaggedFileExt};
+/// use lofty::{
+/// 	AudioFile, BoundTaggedFile, ParseOptions, Tag, TagType, TaggedFileExt, WriteOptions,
+/// };
 /// use std::fs::OpenOptions;
 /// # fn main() -> lofty::Result<()> {
 /// # let path = "tests/files/assets/minimal/full_test.mp3";
@@ -570,7 +576,7 @@ impl From<BoundTaggedFile> for TaggedFile {
 ///
 /// // Now when saving, we no longer have to specify a path, and the tags in the `BoundTaggedFile`
 /// // reflect those in the actual file on disk.
-/// bound_tagged_file.save()?;
+/// bound_tagged_file.save(WriteOptions::new())?;
 /// assert!(!bound_tagged_file.contains_tag_type(TagType::Id3v2));
 /// # Ok(()) }
 /// ```
@@ -620,7 +626,9 @@ impl BoundTaggedFile {
 	/// # Examples
 	///
 	/// ```rust,no_run
-	/// use lofty::{AudioFile, BoundTaggedFile, ParseOptions, Tag, TagType, TaggedFileExt};
+	/// use lofty::{
+	/// 	AudioFile, BoundTaggedFile, ParseOptions, Tag, TagType, TaggedFileExt, WriteOptions,
+	/// };
 	/// use std::fs::OpenOptions;
 	/// # fn main() -> lofty::Result<()> {
 	/// # let path = "tests/files/assets/minimal/full_test.mp3";
@@ -634,11 +642,11 @@ impl BoundTaggedFile {
 	/// // Do some work to the tags...
 	///
 	/// // This will save the tags to the file we provided to `read_from`
-	/// bound_tagged_file.save()?;
+	/// bound_tagged_file.save(WriteOptions::new())?;
 	/// # Ok(()) }
 	/// ```
-	pub fn save(&mut self) -> Result<()> {
-		self.inner.save_to(&mut self.file_handle)?;
+	pub fn save(&mut self, write_options: WriteOptions) -> Result<()> {
+		self.inner.save_to(&mut self.file_handle, write_options)?;
 		self.inner.tags.retain(|tag| !tag.is_empty());
 
 		Ok(())
@@ -692,8 +700,8 @@ impl AudioFile for BoundTaggedFile {
 		)
 	}
 
-	fn save_to(&self, file: &mut File) -> Result<()> {
-		self.inner.save_to(file)
+	fn save_to(&self, file: &mut File, write_options: WriteOptions) -> Result<()> {
+		self.inner.save_to(file, write_options)
 	}
 
 	fn properties(&self) -> &Self::Properties {