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 episode parsing #2429

Merged
merged 3 commits into from
Feb 21, 2020
Merged

Fix episode parsing #2429

merged 3 commits into from
Feb 21, 2020

Conversation

Bond-009
Copy link
Member

Issues
Fixes #2171

@Bond-009 Bond-009 added this to In progress in Release 10.5.0 via automation Feb 19, 2020
@SenorSmartyPants
Copy link
Contributor

Please add a season number test with the same Daily show filename. Currently JF ids that episode as 1x25 and not the correct 25x22.

@SenorSmartyPants
Copy link
Contributor

SenorSmartyPants commented Feb 19, 2020

new EpisodeExpression(@".*(\\|\/)(?<seriesname>((?![sS]?\d{1,4}[xX]\d{1,3})[^\\\/])*)?([sS]?(?<seasonnumber>\d{1,4})[xX](?<epnumber>\d+))[^\\\/]*$")
{
IsNamed = true
},
new EpisodeExpression(@".*(\\|\/)(?<seriesname>[^\\\/]*)[sS](?<seasonnumber>\d{1,4})[xX\.]?[eE](?<epnumber>\d+)[^\\\/]*$")
{
IsNamed = true
},

These expressions look like the correct ones to fix the tests. They are KODI naming standard.
https://kodi.wiki/view/Naming_video_files/TV_shows
I think maybe they just need to be moved before the EpisodeExpression that is currently matching these test cases.

Maybe just the first expression I linked will match the specific test cases you've added.

@Bond-009
Copy link
Member Author

We moved that expression before the Kodi expressions because of false positives.

@Artiume
Copy link
Contributor

Artiume commented Feb 19, 2020

Not sure if we should modify it further but I do receive false positives when the name is within the name of another movie. Thomas and Friends: The Great Race was matched as The Making of Thomas and Friends: The Great Race

This is under the current regex, not this proposed regex.

Copy link
Contributor

@JustAMan JustAMan left a comment

Choose a reason for hiding this comment

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

Mostly LGTM, but I'd like to see others' questions resolved before stamping a green mark.

@Bond-009
Copy link
Member Author

@Artiume Create an issue with the full filename

@dkanada dkanada merged commit 4355b45 into jellyfin:master Feb 21, 2020
Release 10.5.0 automation moved this from In progress to Done Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Tv Episodes identified incorrectly in 10.4.2 and 10.4.3
5 participants