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

Add cleanup pattern from track names in scrobbling operations #237

Merged
merged 7 commits into from
Sep 15, 2024

Conversation

silversonicaxel
Copy link
Contributor

@silversonicaxel silversonicaxel commented Jul 1, 2024

open todos

  • prevent scrobbling of empty song
  • add a visual preview in the tracklist of the part of the song affected by the cleanup, something as a strikethrough, for instance, to help see potential user manual issue:
    • song name: live forever - live
    • cleanup pattern: live
    • song displayed: live forever - live
  • remove just last occurrence of the cleanup pattern, and not all occurrences
  • create <TracklistCleanupForm />
  • maybe anticipate, and fix, user manual mistake if - or / are missing from the cleanup pattern

description

This feature helps to clean up some album track names before the scrobbling operations.
The cleanup is intended to help those lastfm users who prefer not to see among their scrobbles patterns like - remastered, demo, live ....

So the PR add an extra field to highlight a pattern to remove from track names in scrobbling operations.

Screenshot 2024-07-16 at 12 50 14

I hope I covered all scrobbling possibilities, in case, let me know where to fix.

@silversonicaxel
Copy link
Contributor Author

Or, if you have a different or better ideas, let's discuss about it.

@silversonicaxel
Copy link
Contributor Author

I've just a question that came to my mind now, shall I add the new translation ids for all locales? Or just for the one I know and the other I do not know, simply add them in English?

Copy link
Owner

@elamperti elamperti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of users will like this feature. Thanks for your contribution!

A few UX concerns:

  • What happens if the user decides to filter things like the whole title of a song? (the application would probably crash or the request to lastfm will fail)

  • Given a title like She's Electric - Live at Knebworth, the user may want to use just Live at Knebworth, which would end up as She's electric -. Maybe there should be a preview below of how a random track from the list would look like. ScrobbleItem is already a huge component that I don't want to make more complex, but ideally I'd love to see the matching text striked-through and grayed in the tracklist. Like:

    She's Electric - Live

    Meanwhile, we can show a preview below the filter so the users don't have to scrobble to check if their pattern is correct.

  • Another situation: what if the track is Live Forever - Live and the user wants to remove Live? (yes, they should add the dash, but people usually don't read instructions 😅 )

Overall, I like this feature! Let's try to add it, keeping it as simple as possible, then we can improve it from there.

src/components/ScrobbleItem.tsx Outdated Show resolved Hide resolved
src/components/ScrobbleItem.tsx Outdated Show resolved Hide resolved
src/domains/scrobbleAlbum/partials/Tracklist.tsx Outdated Show resolved Hide resolved
src/domains/scrobbleAlbum/partials/Tracklist.tsx Outdated Show resolved Hide resolved
@silversonicaxel silversonicaxel force-pushed the feat/album-removal-pattern branch from b24ad47 to 3dd5c28 Compare July 14, 2024 10:45
@silversonicaxel
Copy link
Contributor Author

And I'll check your remarks.

Repository owner deleted a comment from codecov bot Jul 14, 2024
@silversonicaxel silversonicaxel force-pushed the feat/album-removal-pattern branch 2 times, most recently from c80027a to 41c7a66 Compare July 16, 2024 09:42
@silversonicaxel silversonicaxel force-pushed the feat/album-removal-pattern branch from 41c7a66 to 8a194b6 Compare July 16, 2024 10:47
@silversonicaxel silversonicaxel changed the title Add removal text pattern from tracks name in scrobbling operations Add cleanup pattern from track names in scrobbling operations Jul 16, 2024
@silversonicaxel silversonicaxel force-pushed the feat/album-removal-pattern branch 6 times, most recently from 4a71826 to eae346c Compare July 17, 2024 08:28
@silversonicaxel silversonicaxel force-pushed the feat/album-removal-pattern branch 2 times, most recently from 9d645c3 to c93766f Compare July 17, 2024 13:55
@silversonicaxel silversonicaxel force-pushed the feat/album-removal-pattern branch from c93766f to 14bdb99 Compare July 31, 2024 09:22
@silversonicaxel silversonicaxel force-pushed the feat/album-removal-pattern branch from 14bdb99 to f02a5ab Compare August 27, 2024 06:25
@elamperti
Copy link
Owner

Thanks for your work!

I took the opportunity to make a number of adjustments after my first attempts to use it as user and what I saw fit for the codebase. Let me know if you have any questions or thoughts on them! Always happy to discuss improvements together :)

Screenshot 2024-09-15 at 18-12-05 Open Scrobbler

Seeing that it was becoming difficult to catch edge cases such as empty parenthesis, astray dashes and/or extra spaces, and considering that it could be expanded to accept wildcards, I moved from using text-based matching to crafting a regular expression. Starting with the initial user string (e.g. live), and based on a series of criteria, it's then expanded into a regexp that accounts from partial words and other nuances (check the test file).

The resulting pattern is then used to uniformly break down the track title into pieces ("crumbles") which generate an uniform output to be used at two different places: in the tracklist seen by the user (where the full text is shown, with the parts to be removed highlighted in red), and also at the moment the user scrobbles (through the function cleanTitleWithPattern()), which results in a consistent result between what the user sees and what is finally scrobbled.

This filter allows you to just enter live and filter tracks with (Live) (including parenthesis, disregarding capitalization, and removing extra spaces).

At the moment the filter will remove more than one occurrence in the text, which may be a problem with songs like Live Forever (live), but it can be circumvented by explicitly filtering the parenthesis (live). If this becomes an issue, breakStringUsingPattern() could be modified to return only one matching crumb (the last one).

From the UI perspective, I moved the filter to the top-right of the list, hidden under a "filter" icon. This keeps it easy to access while being out of the way for users who don't need it, and hopefully the icon + label are descriptive enough. This simplifies translations as well: I think that if someone wants this function, they know how it works. If they don't want it, they won't look for it. And curious users can experiment what it does without risk as well.

I'll be pushing a few more changes to track the used patterns server-side. Based on user reception and experience it can be improved and expanded (e.g. to support wildcard characters, which is something I didn't add yet because I want to approach it more carefully). Let's see how it goes! I'll include this in the next release of the scrobbler (2.9.0), as soon as I merge and stabilize the rest of the pending changes (over the following days, if everything stays on track).

@elamperti elamperti merged commit 51681d4 into elamperti:main Sep 15, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants