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.

Sign up for GitHub

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

Jump to bottom

Revamp the parser #222

Closed
zcorpan opened this issue Oct 22, 2015 · 1 comment
Closed

Revamp the parser #222

zcorpan opened this issue Oct 22, 2015 · 1 comment
Assignees
@zcorpan
Labels
clarification

Comments

@zcorpan
Copy link
Member

zcorpan commented Oct 22, 2015

To fix #219 (comment) I find the flat structure of the parsing algorithm with "jump to step foo" makes things hard to work with. Explore a different way to specify the parser that can e.g. share steps for skipping to the next blank line or "-->", and makes it easier to add new kinds of blocks.

This should be purely editorial.

Also see http://krijnhoetmer.nl/irc-logs/whatwg/20151022#l-301 for some discussion.
@zcorpan zcorpan mentioned this issue Oct 25, 2015
Merged
@zcorpan
Copy link
Member Author

zcorpan commented Oct 27, 2015

See https://output.jsbin.com/guhura/ (in Firefox) for a comparison between an attempted literal implementation of the current spec and a new, hopefully better, implementation. The new implementation also fixes #224 (this is how I discovered it).

http://jsbin.com/guhura/edit?js is the JS for the two implementations (parseWebVTTFile2 is the new one).

@zcorpan zcorpan mentioned this issue Nov 5, 2015
Closed
zcorpan added a commit that referenced this issue Nov 6, 2015
@zcorpan
Fix #222: Revamp the parser
2cdb336 
Introduce a new "consume a WebVTT block" concept that is used
for parsing the header, cues, and discarding of bad cues.

This should be strictly editorial except that it also fixes #224.
It should be easier to add new block types like STYLE, and hopefully
easier to implement without having the algorithm use GOTO everywhere.
@zcorpan zcorpan mentioned this issue Nov 7, 2015
Merged
@zcorpan zcorpan self-assigned this Nov 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zcorpan zcorpan

Labels
clarification
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@zcorpan