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

Simple support for WebVTT style #95

Merged

Conversation

NhanNguyen700
Copy link
Contributor

Hi all,

Now, we will lose all of WebVTT style after reading it, process somethings and write to WebVTT format again. This PR provide a simple WebVTT style handling, since CSS support many properties, I will not parsing them all, we will just blindly store all the things in STYLE and rewrite them when writing WebVTT again. We will love some spaces and new lines, but it's not a big duel. Besides, when merging subtitles2 into subtitles1, if there are duplicated CSS selectors, only the CSS style of subtitles1 will be kept.

@NhanNguyen700
Copy link
Contributor Author

Hi @asticode , I added simple support for WebVTT style, could you please take a look?

subtitles.go Outdated Show resolved Hide resolved
webvtt.go Outdated Show resolved Hide resolved
subtitles.go Outdated Show resolved Hide resolved
webvtt.go Outdated Show resolved Hide resolved
webvtt.go Outdated Show resolved Hide resolved
webvtt.go Outdated Show resolved Hide resolved
@asticode
Copy link
Owner

Let me know when I can review your last changes 👍

webvtt.go Outdated Show resolved Hide resolved
webvtt.go Outdated Show resolved Hide resolved
@asticode
Copy link
Owner

asticode commented Nov 3, 2023

Tests seem to be failing, could you fix them? 🤔

Also could you remove cssEndToken and use "}" directly in the condition? Also based on your last comment, that could be a good idea to add a comment in the condition:

if blockName != webvttBlockNameStyle || webVTTStyles == nil ||
    // CSS has ended
    len(webVTTStyles.WebVTTStyles) > 0 || webVTTStyles.WebVTTStyles[len(webVTTStyles.WebVTTStyles)-1] == "}" {

@NhanNguyen700
Copy link
Contributor Author

Comments added and cssEndToken was removed

@asticode asticode merged commit 2c5a2cc into asticode:master Nov 3, 2023
1 check passed
@asticode
Copy link
Owner

asticode commented Nov 3, 2023

Thanks for the PR! ❤️

Let me know whether you need a tag 👍

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.

2 participants