Timestamp tag regression in 0.26.0 about go-astisub HOT 3 CLOSED

We need to add proper handling of inline timestamp tags indeed and I'd be happy to point you to the proper direction for a PR 👍

Here's what needs to be done:

• add a StartAt time.Time attribute to LineItem. Add(), Fragment() and similar methods, as well as their tests, would need to be updated as well to process this new attribute, but that would make this PR way more complicated and I wouldn't mind to do that part after merging your changes. It's up to you whether you want to update those methods as well in this PR.
• in parseTextWebVTT, when processing an html tag, start by determining whether its content is a timestamp. If so, store it in a startAt variable. Later, when creating the LineItem, provide the StartAt value as well.
• finally, in LineItem.webVTTBytes(), if !StartAt.IsZero() append the proper webvtt tag
• add a proper test for inline timestamp tags in webvtt_test.go

Let me know whether you feel like updating Add(), Fragment() and similar methods. Again, I don't mind doing it myself after merging your PR, we'll just have to add the proper TODOs in the code if that's the case 👍

from go-astisub.

• in parseTextWebVTT, when processing an html tag, start by determining whether its content is a timestamp. If so, store it in a startAt variable. Later, when creating the LineItem, provide the StartAt value as well.

The html tokenizer doesn't actually recognise the timestamp tag as a start tag token, presumably because the first character after the < is numeric. I'm guessing we would need to split the resultant text or tokenize it further. I've given it a brief go, but I suspect you might have a better idea.

Let me know whether you feel like updating Add(), Fragment() and similar methods. Again, I don't mind doing it myself after merging your PR, we'll just have to add the proper TODOs in the code if that's the case 👍

Having had a quick look, I think it's probably beyond my current knowledge of the codebase, so I'd prefer to leave that as TODOs.

from go-astisub.

Having had a quick look, I think it's probably beyond my current knowledge of the codebase, so I'd prefer to leave that as TODOs.

👍

The html tokenizer doesn't actually recognise the timestamp tag as a start tag token, presumably because the first character after the < is numeric. I'm guessing we would need to split the resultant text or tokenize it further. I've given it a brief go, but I suspect you might have a better idea.

Splitting the input text at the start of parseTextWebVTT based on inline timestamp tags seems like a good idea. We would then have to loop through split items to run the existing logic 👍

from go-astisub.