[ID3v2] Standardize on fromFields#142
Open
benrr101 wants to merge 16 commits into
Open
Conversation
…lib-sharp into feat/id3v2/from-fields
…nownFrame, UnsynchronizedLyricsFrame, UrlLinkFrame and some fixes to previous ones.
…. Rename text in Url frame to url
There was a problem hiding this comment.
Pull request overview
Standardizes ID3v2 frame construction around fromFields, including defaults and direct field initialization.
Changes:
- Replaces specialized frame factories with
fromFields. - Renames URL frame
textaccessors tourl. - Updates tag logic and unit tests for the new APIs.
Reviewed changes
Copilot reviewed 40 out of 40 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
src/utils.ts |
Adds optional bigint validation. |
src/id3v2/id3v2TagHeader.ts |
Adds field-based header construction. |
src/id3v2/id3v2Tag.ts |
Migrates frame creation and URL access. |
src/id3v2/frames/userUrlLinkFrame.ts |
Standardizes WXXX construction. |
src/id3v2/frames/userTextInformationFrame.ts |
Standardizes TXXX construction. |
src/id3v2/frames/urlLinkFrame.ts |
Adds fromFields and url. |
src/id3v2/frames/unsynchronizedLyricsFrame.ts |
Adds field-based construction. |
src/id3v2/frames/unknownFrame.ts |
Renames constructor to fromFields. |
src/id3v2/frames/uniqueFileIdentifierFrame.ts |
Adds defaults to field construction. |
src/id3v2/frames/textInformationFrame.ts |
Adds text-aware construction. |
src/id3v2/frames/termsOfUseFrame.ts |
Expands constructor fields. |
src/id3v2/frames/synchronizedLyricsFrame.ts |
Consolidates lyrics construction. |
src/id3v2/frames/relativeVolumeFrame.ts |
Adds channel construction and cloning. |
src/id3v2/frames/privateFrame.ts |
Adds owner/data construction. |
src/id3v2/frames/popularimeterFrame.ts |
Adds rating/count construction. |
src/id3v2/frames/playCountFrame.ts |
Adds count-based construction. |
src/id3v2/frames/musicCdIdentifierFrame.ts |
Standardizes data construction. |
src/id3v2/frames/genreFrame.ts |
Adds text and encoding construction. |
src/id3v2/frames/eventTimeCodeFrame.ts |
Consolidates event construction. |
src/id3v2/frames/commentsFrame.ts |
Consolidates comment construction. |
test-unit/id3v2/userUrlLinkFrameTests.ts |
Updates WXXX tests. |
test-unit/id3v2/userTextInformationFrameTests.ts |
Updates TXXX tests. |
test-unit/id3v2/urlLinkFrameTests.ts |
Tests URL field construction. |
test-unit/id3v2/unsynchronizedLyricsFrameTests.ts |
Tests lyrics defaults and fields. |
test-unit/id3v2/unknownFrameTests.ts |
Tests unknown-frame construction. |
test-unit/id3v2/uniqueFileIdentifierFrameTests.ts |
Tests UFID defaults and validation. |
test-unit/id3v2/textInformationFrameTests.ts |
Tests text field construction. |
test-unit/id3v2/termsOfUseFrameTests.ts |
Tests terms fields and defaults. |
test-unit/id3v2/synchronizedLyricsFrameTests.ts |
Tests synchronized lyric fields. |
test-unit/id3v2/relativeVolumeFrameTests.ts |
Tests channel field construction. |
test-unit/id3v2/privateFrameTests.ts |
Tests private owner/data fields. |
test-unit/id3v2/popularimeterFrameTests.ts |
Tests rating and play count fields. |
test-unit/id3v2/playCountFrameTests.ts |
Tests play-count construction. |
test-unit/id3v2/musicCdIdentifierFrameTests.ts |
Tests MCDI field construction. |
test-unit/id3v2/id3v2TagTests.ts |
Migrates tag integration tests. |
test-unit/id3v2/genreFrameTests.ts |
Refactors and expands genre tests. |
test-unit/id3v2/frameFactoryTests.ts |
Migrates factory fixtures. |
test-unit/id3v2/eventTimeCodeFrameTests.ts |
Tests event field construction. |
test-unit/id3v2/commentsFrameTests.ts |
Tests comment field construction. |
test-unit/id3v2/attachmentsFrameTests.ts |
Migrates unknown-frame fixtures. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+51
to
+58
| public static fromFields(playCount: bigint = BigInt(0)): PlayCountFrame { | ||
| Guards.ulong(playCount, "playCount"); | ||
|
|
||
| const frame = new PlayCountFrame(new Id3v2FrameHeader(FrameIdentifiers.PCNT)); | ||
| frame._playCount = playCount; | ||
|
|
||
| return frame; | ||
| } |
|
|
||
| const frame = new PopularimeterFrame(new Id3v2FrameHeader(FrameIdentifiers.POPM)); | ||
| frame._user = user; | ||
| frame._playCount = playCount; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The goal of this PR is to migrate the various fromXyz static constructors in the ID3v2 frames to a standard fromFields static constructor. The parameters will vary between frame types, but they will all behave the same - construct a new frame, store the fields in the new frame, and have useful defaults for the parameters.
As I got deeper into this one I just kinda felt like what the heck am I doing. But, I'm happy that each frame type has two static constructors (well, except for attachments).