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

Unused Variable 'seen cue' #324

Closed
BenjaminSchaaf opened this issue Feb 1, 2017 · 4 comments
Closed

Unused Variable 'seen cue' #324

BenjaminSchaaf opened this issue Feb 1, 2017 · 4 comments
Assignees
@zcorpan

Comments

@BenjaminSchaaf
Copy link
Contributor

Under 'WebVTT parser algorithm' (3.) seen cue is set to false, and is then only checked.

Should this variable be removed? (can make a pr if so)
@silviapfeiffer
Copy link
Member

Seems pretty pointless now - I think I used to be used for collecting metadata headers before that got removed.

@silviapfeiffer
Copy link
Member

Nope, I'm wrong, it came from a4bcfc1

@zcorpan
Copy link
Member

zcorpan commented Feb 1, 2017

Hmm, good catch. I think it should be set to true here

Otherwise, let buffer be the empty string.

->

Otherwise, let buffer be the empty string and let seen cue be true.

The intent is to drop STYLE etc blocks on the floor if a cue has been seen.

@BenjaminSchaaf
Copy link
Contributor Author

@zcorpan Confirmed that setting seen cue to true there will fix style blocks getting parsed after the first cue.

zcorpan added a commit that referenced this issue Feb 21, 2017
@zcorpan
Set |seen cue| to true when seeing a cue
f553c3b 
This variable was intended to ignore STYLE and REGION blocks after
having seen a cue, but it was never set to true.

Fixes #324.
@zcorpan zcorpan mentioned this issue Feb 21, 2017
Merged
@zcorpan zcorpan self-assigned this Feb 21, 2017
zcorpan added a commit that referenced this issue Feb 21, 2017
@zcorpan
Set |seen cue| to true when seeing a cue
8a620e0 
This variable was intended to ignore STYLE and REGION blocks after
having seen a cue, but it was never set to true.

Fixes #324.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zcorpan zcorpan

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@silviapfeiffer @zcorpan @BenjaminSchaaf