Skip to content
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.

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

Supported File Types Section Produces Invalid List of FileType References for Tag Formats in Documentation #505

Closed
Lepidopteran opened this issue Jan 24, 2025 · 5 comments · Fixed by #506
Labels
bug Something isn't working

Comments

@Lepidopteran
Copy link
Contributor

Lepidopteran commented Jan 24, 2025

Reproducer

Look at the "Supported file types" section for any tag format.

Some examples include:

Summary

I was looking at the documentation for various tags and I've found every tag format, the Supported file types section produces invalid references to the various file types

I'm guessing it's because of Line 19 to 24 in lofty_tag.rs in the lofty_attr crate:

fn emit_doc_comment(&self) -> String {
	match self {
		SupportedFormat::Full(path) => format!(
			"* [`FileType::{ft}`](crate::FileType::{ft})\n",
			ft = path.get_ident().unwrap()
		),
		SupportedFormat::ReadOnly(path) => format!(
			"* [`FileType::{ft}`](crate::FileType::{ft}) **(READ ONLY)**\n",
			ft = path.get_ident().unwrap()
		),
	}
}

It's generating documentation that points to crate::FileType::<File_Format> which is invalid because, file types are now stored crate::file::FileType::<File_Format>.

I did try to change it myself, but when I tested it locally, it did not produce a different documentation result for me.

I'm not sure why I couldn't get it to work, but I wanted to point this out!

Expected behavior

In the Supported file types section, produce a list that points to crate::file::FileType::<File_Format> instead of crate::FileType::<File_Format>

Side Note

This is a amazing project!

I was previously using id3, metaflac and mp4ameta for a personal project of mine and I wanted to support reading and writing to more tag formats other than id3, Itunes, and a partial of vorbiscomments, and I was daunting the task of writing my own readers and writers.

So when finding your project, I was relieved, and after trying your project out, I'd say you have the best (and to my knowledge only) library that provides a nice API for every popular metadata standard out there!

So to end all, thank you!

Assets

No response

@Lepidopteran Lepidopteran added the bug Something isn't working label Jan 24, 2025
@Serial-ATA
Copy link
Owner

I'm guessing it's because of Line 19 to 24 in lofty_tag.rs in the lofty_attr crate

Yep, that's what generates them. I imagine it wasn't being updated since the lofty crate just depends on the published versions of the workspace crates:

# Proc macros
lofty_attr = "0.11.0"

It really should use path dependencies, I just didn't think to do that when I migrated the repo to a workspace.

If you already made the changes locally, feel free to make a PR. It may be a bit before I can get to it myself.

And thank you for the kind words and doc fixes you've been making! ❤️

@Lepidopteran
Copy link
Contributor Author

Lepidopteran commented Jan 25, 2025

Oh I see, I can definitely make a PR request!

Would you recommend I only change the lofty_atrr to path based on my local or should I commit that in the PR as well?

And you're welcome, you're great to work with!

@Serial-ATA
Copy link
Owner

You can change it to a path in a PR, just do: lofty_attr = { path = "../lofty_attr" }

@Lepidopteran
Copy link
Contributor Author

It looks like the documentation has not updated. Do you have to push out a new version to update said documentation?

@Serial-ATA
Copy link
Owner

Yep, I can release 0.22.2 tomorrow if I have time. If I don't get around to it though, you can always cargo doc --open -p lofty.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants