guessit-io / guessit

GuessIt is a python library that extracts as much information as possible from a video filename.

https://guessit-io.github.io/guessit

GNU Lesser General Public License v3.0

824 stars 92 forks source link

fix(other): detect "Open Matte" with period separator #714

Closed plotski closed 3 years ago

plotski commented 3 years ago

This fixes detection of "Open Matte" when "." is used instead of " " in the release name.

codecov-commenter commented 3 years ago

Codecov Report

Merging #714 (3630a47) into develop (de85403) will not change coverage. The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop     #714   +/-   ##
========================================
  Coverage    98.49%   98.49%           
========================================
  Files           52       52           
  Lines         3387     3387           
========================================
  Hits          3336     3336           
  Misses          51       51

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 7044af3...3630a47. Read the comment docs.

plotski commented 3 years ago

"-+" also works, but it's more obscure. Regular expression syntax is widely known and understood. Explicit is better than implicit.

Just my opinion. Let me know if you insist on "-+" and I'll change it.

Toilal commented 3 years ago

I understand, but most rebulk objects inside guessit register a dash abbreviation to support separators in a consistent way and improve readability of patterns. It's registered globally for the other property like many other ones.

https://github.com/guessit-io/guessit/blob/1e7b0008232e306478d15f7a78d093804d56df3a/guessit/rules/properties/other.py#L30

dash abbrevation is declared here

https://github.com/guessit-io/guessit/blob/066a9dd448d594e1bdd67a3534c632050a484bd6/guessit/rules/common/__init__.py#L8-L15

So here, r'-' is preprocessed in the regex declaration and replaced with r'(?:[\ \[\]\(\)\{\}\+\*\|=\-_\~\#\.,;:])'. I'm pretty sure you don't want to copy/paste this everywhere in guessit.

plotski commented 3 years ago

Thank you for the explanation!

I've changed it and the tests seem to run fine.

Toilal commented 3 years ago

Thanks, finally I have used "-?" to match no or one separator only.