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

formatter: add {:sanitize} to return song or song [feat] #2237

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

lasers
Copy link
Contributor

@lasers lasers commented Feb 26, 2024

DO NOT MERGE. An alternative to @dpeukert's #2234.

Pro:

  • Works on all modules. Not restricted to music modules.
  • Doesn't have to add anything to existing music modules.
  • Remove couple of spotify configs.

Con:

  • Non-config metadata patterns. I think users might not care. If they do, they can make PR that will help everybody.
  • Abuse (?) Not what it was originally for. Got :d, :.2f, :g, :ceil, and now :escape and :sanitize... Lol.

Instruction:

  • To use this, just use {title:sanitize}, {album:sanitize} in any modules (music or not).
  • You even can use {title:escape,sanitize} too. My testing is limited and short, but everything seems OK.

Default behavior:

  • This can be set as default behavior for music modules by adding class Meta or self.py3.update_placeholder_formats in post_config_hook... but if you add them on, there will no way for users to turn them off in the format config. This is same for other placeholders not related to this PR. Just a FYI.

@lasers
Copy link
Contributor Author

lasers commented Feb 27, 2024

The issue is hardcoded meta words. It'd be ideal to keep them contained to modules only.

I feel like self.py3.sanitize() / self.py3.replace() might be a better choice, but how should the config look like? I'm guessing it should look something like a thresholds dict... A replace or sanitize dictionary with placeholders (keys) + re.compile keywords including meta words (values).

This PR is just something I tried because of placeholders and doing [\?sanitize {placeholder}] sounds bad.

@lasers
Copy link
Contributor Author

lasers commented Mar 4, 2024

spotify is untested. FYI just in case.

Comment on lines +310 to +313
try:
return self.sanitized_string_cache[string]
except KeyError:
cache_key = string
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this needed? The composites may could be cached. Maybe regex cache only... Something to look into.

@lasers lasers marked this pull request as draft March 26, 2024 17:52
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.

None yet

1 participant