search

zyrouge / node-genius-lyrics

Simple lyrics fetcher that uses Genius. Also has official API implementations.
https://genius-lyrics.js.org
MIT License
61 stars 12 forks source link

removeChorus regex is not working, regex fix #39

Closed juliendorra closed 1 year ago

juliendorra commented 1 year ago

As per issue #38 Here's a proposed fix for the removeChorus option. This regex matches Chorus, Post-Chorus and Pre-Chorus blocks, allowing to remove them all.

zyrouge commented 1 year ago

Why wasn't the previous working though? Due to trailing whitespaces? Something that is a bit more simple like /\[[^\]]+\]\S*\n?/g would work?

zyrouge commented 1 year ago

image Your solution seems to match the whole chorus lyrics, which is not actually the behavior I was having in mind. That option was implemented only to remove things like [Chorus] and [Pre-Chorus 2]. Anything beyond that is left to the user themselves. Thank you for opening this pull request though, really appreciate it. If you really want, open a pull request containing additional options to lyrics method.

juliendorra commented 1 year ago

Hi, yes indeed, I understood your intention as removing the chorus text themselves, not the various headers (because of the name of the option / function)

Note that there's more to headers than chorus (including Verse, and many others!) so you might want to rename this option/function removeBracketedHeaders to be self-documenting.

Cleaning up duplicate blocks (typically chorus) would be a different option that can be useful in some use cases (like sentiment analysis for example). I have something similar in my code that I could try a port to the lib.

zyrouge commented 1 year ago

Hi, yes indeed, I understood your intention as removing the chorus text themselves, not the various headers (because of the name of the option / function)

Note that there's more to headers than chorus (including Verse, and many others!) so you might want to rename this option/function removeBracketedHeaders to be self-documenting.

Cleaning up duplicate blocks (typically chorus) would be a different option that can be useful in some use cases (like sentiment analysis for example). I have something similar in my code that I could try a port to the lib.

I only realised that recently, I will try to make that option more clear on upcoming releases. Thank you.