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

fix: swap trailing determiners at the end of titles #9820

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

tempcvrohch
Copy link

Database Migration

NO

Description

Some files have their , A or , The at the end of the file to prevent alphabetical sorting clustering these titles together. This stops working when searching for files inside Radarr as a cleaned title will have 'prestigethe' instead of 'theprestige'.

Seeing as the and a in the middle of a title are already filtered out and covered in a different test case I didn't write a negative test case.

This is just covering English, I assume to support more languages the regex will need to be expanded.

Screenshot (if UI related)

Todos

  • Tests
  • Translation Keys (./src/NzbDrone.Core/Localization/Core/en.json)
  • Wiki Updates

Issues Fixed or Closed by this PR

fixes Radarr#9815.\n\n Some files have their , A or , The at the end of the file to prevent alphabetical sorting clustering these titles together. This stops working when searching for files inside radarr as a cleaned title will have 'prestigethe' instead of 'theprestige'.
@github-actions github-actions bot added the Area: Parser Issue is related to parsing infrastructure. label Mar 5, 2024
@@ -12,6 +12,7 @@ namespace NzbDrone.Common.Extensions
public static class StringExtensions
{
private static readonly Regex CamelCaseRegex = new Regex("(?<!^)[A-Z]", RegexOptions.Compiled);
private static readonly Regex TrailingDeterminerRegex = new Regex("(, The)$|(, A)$", RegexOptions.Compiled);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private static readonly Regex TrailingDeterminerRegex = new Regex("(, The)$|(, A)$", RegexOptions.Compiled);
private static readonly Regex TrailingArticleRegex = new Regex("(, The)$|(, A)$", RegexOptions.Compiled);

Maybe? Food for thought

Copy link
Author

Choose a reason for hiding this comment

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

I'm pretty impartial to the naming, but it should be consistent. Want me to change all of the occurrences from determiner to article then?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Parser Issue is related to parsing infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MovieTitle, The naming convention breaks API calls
2 participants