feat(lyrics-plus): add performer tag for the Musixmatch provider#3689
feat(lyrics-plus): add performer tag for the Musixmatch provider#3689ianz56 wants to merge 6 commits intospicetify:mainfrom
Conversation
📝 WalkthroughWalkthroughIntroduces performer tagging functionality to lyrics display. Extracts performer data from Musixmatch API and integrates performer-aware rendering with conditional visibility controls. Adds configuration option to toggle performer label display and applies styling for performer elements. Changes
Sequence Diagram(s)sequenceDiagram
participant API as Musixmatch API
participant Provider as ProviderMusixmatch
participant Container as LyricsContainer
participant Menu as AdjustmentsMenu
participant Page as LyricsPage
API->>Provider: Return track_structure + performer_tagging
Provider->>Provider: parsePerformerData()<br/>(extract snippets)
Provider->>Provider: matchSequential()<br/>(align performers)
Provider->>Container: Return lines with performer field
Container->>Container: Detect hasPerformer<br/>from line data
Container->>Menu: Pass hasPerformer prop
Menu->>Menu: Conditionally show<br/>performers toggle
Container->>Page: Pass show-performers<br/>config + lines
Page->>Page: Render performer label<br/>if enabled + performer exists
Page->>Page: Deduplicate consecutive<br/>performer labels
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CustomApps/lyrics-plus/ProviderMusixmatch.js`:
- Around line 94-157: parsePerformerData currently assumes
meta.track.performer_tagging exists and will throw if meta or meta.track or
performer_tagging is missing; add defensive checks at the top of
parsePerformerData to return an empty snippetQueue when meta, meta.track, or
meta.track.performer_tagging is falsy, ensure miscTags defaults to an empty
object and resourcesList is derived safely from tagging.resources?.artists
(falling back to []), and keep the rest of the logic (performerMap building,
normalizeForMatch, snippetQueue population) unchanged so the function never
operates on undefined fields.
🧹 Nitpick comments (1)
CustomApps/lyrics-plus/ProviderMusixmatch.js (1)
108-121: Consider safer parsing offqidvalues.The
parseInt(fqid.split(":")[2])pattern assumes a specific format. If the format is unexpected, this could returnNaNor cause issues. The same pattern is repeated at line 119.♻️ Suggested improvement
const resolvedPerformers = c.performers.map((p) => { let name = "Unknown"; if (p.type === "artist") { const fqid = p.fqid; - const idFromFqid = fqid ? parseInt(fqid.split(":")[2]) : null; + const idFromFqid = fqid ? parseInt(fqid.split(":")[2], 10) : null; const artist = resourcesList.find((r) => r.artist_id === idFromFqid); if (artist) name = artist.artist_name; } else if (miscTags[p.type]) { name = miscTags[p.type]; } return { fqid: p.fqid, - artist_id: p.fqid ? parseInt(p.fqid.split(":")[2]) : null, + artist_id: p.fqid ? parseInt(p.fqid.split(":")[2], 10) : null, name: name, }; });
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CustomApps/lyrics-plus/ProviderMusixmatch.js`:
- Around line 110-135: resolvedPerformers currently includes entries with name
"Unknown" which end up in the UI; update the logic so only resolved performers
are kept by filtering out entries with name === "Unknown" (either filter the
resolvedPerformers array after the map or build it with a filter/flatMap) before
computing names and before returning the object so the returned performers array
and names.join(", ") contain only resolved performers; ensure the early return
(names.length === 0) stays intact and references
resolvedPerformers/names/snippet when constructing the returned object.
rxri
changed the title
There was a problem hiding this comment.
🧹 Nitpick comments (2)
CustomApps/lyrics-plus/Pages.js (2)
211-227: Performer rendering logic is correct; consider extracting the repeated IIFE.The null-safety on
previousLineis properly handled, and thesynced-compactconditional deduplication is sound. However, this IIFE block is duplicated nearly identically acrossSyncedLyricsPage,SyncedExpandedLyricsPage, andUnsyncedLyricsPage. Extracting it into a shared helper (e.g.,renderPerformerLabel(performer, previousPerformer, options)) would reduce duplication and simplify future changes.♻️ Suggested helper extraction
+const renderPerformerLabel = (performer, previousPerformer, { deduplicate = true } = {}) => { + if (!CONFIG.visual["show-performers"] || !performer) return null; + if (deduplicate && previousPerformer === performer) return null; + return react.createElement( + "span", + { className: "lyrics-lyricsContainer-Performer" }, + performer + ); +};Then in each component, replace the IIFE with a one-liner, e.g. for
SyncedLyricsPage:renderPerformerLabel( performer, lyricWithEmptyLines[lineNumber - 1]?.performer, { deduplicate: !CONFIG.visual["synced-compact"] } ),
506-522: Remove the redundantsynced-compactcheck fromSyncedExpandedLyricsPage.The
synced-compactconfiguration controls which component renders at the top level (lines 1075, 1084 in index.js): when true,SyncedLyricsPagerenders; when false,SyncedExpandedLyricsPagerenders. This means the check at line 509 (if (!CONFIG.visual["synced-compact"])) always evaluates to true, making it unnecessary. SinceSyncedExpandedLyricsPageis only instantiated whensynced-compactis false, deduplication should always happen here. Simplify by removing the condition and unconditionally performing deduplication in this view.
2 participants
Display performer tags for each line showing who the singer is. This can be enabled/disabled via the Option Menu.
output1.mp4
Summary by CodeRabbit
New Features
Style