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

Extras: Clean filename and process NFOs #9403

Merged
merged 5 commits into from
Mar 20, 2023

Conversation

SenorSmartyPants
Copy link
Contributor

Changes

  • Filenames are cleaned using CleanStrings regex
  • new regex to remove extra suffixes
  • NFOs for extras are now read

A file named Interview with director-interview.mkv will now be named Interview with director in JF.

NFO files named the same as extra file will be read for metadata.

@SenorSmartyPants
Copy link
Contributor Author

Any feedback on the unit test I changed? I'm not aware of which issues that might be related to, or if my change will affect them.

@jellyfin-bot jellyfin-bot added the merge conflict Merge conflicts should be resolved before a merge label Mar 9, 2023
SenorSmartyPants and others added 4 commits March 9, 2023 17:44
- Adds regular expression to CleanStrings to remove suffix style extra naming from the name presented in JF.
- Override Resolve for Extras to enable parsename
- remove exclusion on parsename of extratypes
- Change test to prevent owned items from using parent NFO. Test is now in MovieNFOSaver, only movie type will use movie.nfo.
I need more information about the need for this test, to make sure I am not introducing an issue.
Apply code review

Co-authored-by: Cody Robibero <cody@robibe.ro>
@jellyfin-bot jellyfin-bot removed the merge conflict Merge conflicts should be resolved before a merge label Mar 9, 2023
@1337joe
Copy link
Member

1337joe commented Mar 16, 2023

Any feedback on the unit test I changed? I'm not aware of which issues that might be related to, or if my change will affect them.

I added that test when I did some work on ProviderManager to ensure that behavior didn't change during my refactor. The check itself is from Emby, I just made the comment more relevant.

I've never used NFOs so I can't attest to the necessity of that check. I expect that if you're changing the behavior you've done some testing with various NFO and naming arrangements.

Copy link
Member

@crobibero crobibero left a comment

Choose a reason for hiding this comment

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

I think some tests for the new naming would be helpful, without doing much checking I think you can plug directly into FindExtrasTests

@SenorSmartyPants
Copy link
Contributor Author

@crobibero Added one test of cleaning 3 extra names.

@crobibero crobibero merged commit dd491ce into jellyfin:master Mar 20, 2023
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.

6 participants