Srt html styling #121
Srt html styling #121
Conversation
justin-taylor
force-pushed
the
srt-html-styling
branch
from
d7242fd
to
1cf96aa
Compare
justin-taylor
force-pushed
the
srt-html-styling
branch
from
1cf96aa
to
e677551
Compare
testdata/example-styled-in.srt
Outdated
There was a problem hiding this comment.
Can you rename this file to example-in-styled.srt?
testdata/example-styled-out.srt
Outdated
There was a problem hiding this comment.
Can you rename this file to example-out-styled.srt?
testdata/example-styled-out.vtt
Outdated
There was a problem hiding this comment.
Can you rename this file to example-out-styled.vtt?
srt_test.go
Outdated
| assert.Equal(t, string(c), w.String()) | ||
| } | ||
|
|
||
| func TestReadSRTWriteWebVTTStyled(t *testing.T) { |
There was a problem hiding this comment.
Can you not create a separate test for that and instead add writing to vtt at the end of TestSRTStyled?
srt_test.go
Outdated
| s, err := astisub.OpenFile("./testdata/example-styled-in.srt") | ||
| assert.NoError(t, err) | ||
| assertStyledSubtitleItems(t, s) | ||
| assertSRTSubtitleStyles(t, s) |
There was a problem hiding this comment.
Can you not make it a function and instead test the style directly in this function? It shouldn't be used elsewhere
subtitles_test.go
Outdated
| assert.Equal(t, "'The time is 7:35.'", i.Items[5].Lines[1].String()) | ||
| } | ||
|
|
||
| func assertSRTSubtitleStyles(t *testing.T, i *astisub.Subtitles) { |
There was a problem hiding this comment.
This function should be removed based on my previous comment
All comments have been addressed. |
|
Thanks for the PR! ❤️ Let me know whether you need a tag 👍 |
A tag would be great! Thank you. |
|
I've created a |
This PR resurrects #96 The changes from the PR have been re-applied and made to work with the mast recent changes. For our use-case we often run into an srt file being uploaded with stylings that are then broken when converted to webvtt.
I believe all the comments that were apart of that PR have been addressed, but please let me know if other changes are needed.