From 8190e3a535a242a3055f0fbecc035d1e0e4e27b9 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 19 Jun 2026 16:17:31 -0400 Subject: [PATCH] refactor(lyrics): trim verbose comments to essential why --- model/lyrics.go | 4 +--- model/lyrics_parse.go | 28 +++++++++------------------- model/lyrics_ttml.go | 6 ++---- plugins/testdata/test-lyrics/main.go | 3 +-- 4 files changed, 13 insertions(+), 28 deletions(-) diff --git a/model/lyrics.go b/model/lyrics.go index 9712ca30a..26ecc1579 100644 --- a/model/lyrics.go +++ b/model/lyrics.go @@ -414,9 +414,7 @@ func parseTime(line string, match []int) (int64, error) { type LyricList []Lyrics -// MarshalJSON owns the lyrics column invariant: an empty or nil list serializes -// to [], never null. This keeps every writer consistent without each one having -// to pre-initialize or nil-guard the slice. +// MarshalJSON keeps the lyrics column invariant: empty/nil serializes to [], never null. func (ll LyricList) MarshalJSON() ([]byte, error) { if len(ll) == 0 { return []byte("[]"), nil diff --git a/model/lyrics_parse.go b/model/lyrics_parse.go index 95cebbfa2..fc7016fbb 100644 --- a/model/lyrics_parse.go +++ b/model/lyrics_parse.go @@ -9,21 +9,17 @@ import ( "github.com/navidrome/navidrome/log" ) -// lyricParser parses a single lyrics format. It returns an empty list (not an -// error) when the input is well-formed but not its format, so candidates can be -// tried in order. The lang argument is the default language for formats that do -// not carry their own. +// lyricParser returns an empty list (not an error) when the input is not its +// format, so parsers can be tried in order. lang is the default for formats that +// do not carry their own. type lyricParser func(lang string, contents []byte) (LyricList, error) func parseLyricsfileBytes(_ string, contents []byte) (LyricList, error) { return parseLyricsfile(string(contents)) } -// lyricFormats lists the structured formats. Each parser is self-skipping: it -// returns an empty list (not an error) when the content is not its format, so it -// is safe to try in order. Slice order is the content-sniff probe order -// (TTML → SRT → YAML); each row's suffixes drive sidecar dispatch. LRC/plain is -// not listed — it is the fallback floor for both paths. +// lyricFormats is the structured formats in content-sniff probe order; each +// row's suffixes drive sidecar dispatch. LRC/plain is the unlisted fallback floor. var lyricFormats = []struct { suffixes []string parse lyricParser @@ -33,14 +29,10 @@ var lyricFormats = []struct { {[]string{".yaml", ".yml"}, parseLyricsfileBytes}, } -// ParseLyrics is the single entry point for parsing lyrics. A known suffix -// (.ttml/.srt/.yaml/.yml/.lrc) routes to that format's parser; an empty or -// "auto" suffix content-sniffs by trying each format in order. In both modes a -// structured parser that does not match falls back to the LRC/plain-text floor — -// never to another structured format. +// ParseLyrics is the single entry point for parsing lyrics. A known suffix routes +// to that format's parser; an empty or "auto" suffix content-sniffs. Either way, +// a structured parser that does not match falls back to the LRC/plain-text floor. func ParseLyrics(suffix, lang string, contents []byte) (LyricList, error) { - // Strip a leading BOM once here so every parser sees clean bytes, regardless - // of which caller (file read, embedded tag, plugin, DB string) supplied them. contents = stripBOM(contents) if suffix == "" || strings.EqualFold(suffix, "auto") { @@ -55,11 +47,9 @@ func ParseLyrics(suffix, lang string, contents []byte) (LyricList, error) { return parseFirstMatch(lang, contents, f.parse) } } - return plainLRC(lang, contents) // .lrc and any unknown suffix + return plainLRC(lang, contents) } -// parseFirstMatch tries each candidate in order, returning the first non-empty -// result. When every candidate misses, it falls to the LRC/plain-text floor. func parseFirstMatch(lang string, contents []byte, candidates ...lyricParser) (LyricList, error) { for _, parse := range candidates { list, err := parse(lang, contents) diff --git a/model/lyrics_ttml.go b/model/lyrics_ttml.go index 59686ce16..aa07dd9ef 100644 --- a/model/lyrics_ttml.go +++ b/model/lyrics_ttml.go @@ -109,10 +109,8 @@ func parseTTML(contents []byte) (LyricList, error) { func parseTTMLWithDefaultLang(defaultLang string, contents []byte) (LyricList, error) { contents = xmlEncodingRegex.ReplaceAll(contents, []byte(``)) - // Self-skip non-TTML content (returns nil, nil) so content-sniffing does not - // run the XML decoder on plain text or LRC. Checked after the encoding fixup - // above so a UTF-16-declared document is still recognized. Content that is a - // document but malformed still parses below and surfaces its error. + // Skip non-TTML content so sniffing doesn't run the XML decoder on plain text. + // Checked after the encoding fixup so UTF-16-declared documents are recognized. if !isTTMLDocument(string(contents)) { return nil, nil } diff --git a/plugins/testdata/test-lyrics/main.go b/plugins/testdata/test-lyrics/main.go index 988490515..2ee2dabbf 100644 --- a/plugins/testdata/test-lyrics/main.go +++ b/plugins/testdata/test-lyrics/main.go @@ -20,8 +20,7 @@ func (t *testLyrics) GetLyrics(input lyrics.GetLyricsRequest) (lyrics.GetLyricsR return lyrics.GetLyricsResponse{}, fmt.Errorf("%s", errMsg) } - // Config-selected formats let integration tests prove the adapter's content-sniffing - // detects each format; plain-text parsing would silently mangle the rich ones. + // Config-selected format lets tests exercise the adapter's content-sniffing per format. format, hasFormat := pdk.GetConfig("format") if hasFormat { var text string