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

webvtt,srt: Tolerate edge whitespace (and print line numbers on error) #14

Merged
merged 4 commits into from
Sep 17, 2019
Merged

Conversation

mholt
Copy link
Contributor

@mholt mholt commented Aug 25, 2019

Hi! I'm by no means an expert of WebVTT format but this fixed a parsing problem for me. Feel free to reject if it's a bad change, or improve it or whatever you'd like.

PS. It'd be really helpful to output the line number when an error is encountered. Would you be open to a PR for that?

@coveralls
Copy link

coveralls commented Aug 25, 2019

Coverage Status

Coverage increased (+0.04%) to 76.391% when pulling 3bbcc41 on mholt:patch-1 into c0ed792 on asticode:master.

@asticode
Copy link
Owner

Sounds good. Actually could you add the same change to .srt parsing?

And I'm open for a PR displaying line numbers in errors.

Cheers

@asticode asticode self-assigned this Aug 26, 2019
@mholt
Copy link
Contributor Author

mholt commented Aug 26, 2019

Okay, cool! This is kind of a weekend/side project for me, so it might be a few days, but I'll get around to it (unless someone beats me to it). Thanks for the helpful lib.

@mholt
Copy link
Contributor Author

mholt commented Sep 14, 2019

@asticode I just put them both in this one, since they're small changes :P Hope this is good to go, thanks!

@mholt mholt changed the title webvtt: Tolerate leading and trailing whitespace webvtt,srt: Tolerate edge whitespace (and print line numbers on error) Sep 14, 2019
@asticode asticode merged commit 7fe562f into asticode:master Sep 17, 2019
@asticode
Copy link
Owner

Cheers

@mholt mholt deleted the patch-1 branch September 17, 2019 15:29
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

3 participants