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

Solved https://github.com/asticode/go-astisub/issues/59 #60

Merged
merged 2 commits into from
Nov 17, 2021
Merged

Solved https://github.com/asticode/go-astisub/issues/59 #60

merged 2 commits into from
Nov 17, 2021

Conversation

caryyu
Copy link
Contributor

@caryyu caryyu commented Nov 17, 2021

Originated from #59

@coveralls
Copy link

coveralls commented Nov 17, 2021

Coverage Status

Coverage increased (+0.1%) to 76.596% when pulling 1803e51 on caryyu:master into aa9412f on asticode:master.

ssa.go Outdated
@@ -1268,3 +1274,14 @@ func (s Subtitles) WriteToSSA(o io.Writer) (err error) {
}
return
}

// SSAOptions
type SSAOptions struct{}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OnUnknownSectionName and OnInvalidLine need to be fields of SSAOptions.

You also need to create a defaultSSAOptions func() SSAOptions that would return SSAOptions with callbacks properly set for logging

ssa.go Outdated
@@ -175,7 +181,7 @@ func ReadFromSSA(i io.Reader) (o *Subtitles, err error) {
format = make(map[int]string)
continue
default:
log.Printf("astisub: unknown section: %s", line)
opts.OnUnknownSectionName(line)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once OnUnknownSectionName is a field, you'll need to wrap this with if opts.OnUnknownSectionName != nil

ssa.go Outdated
@@ -195,7 +201,7 @@ func ReadFromSSA(i io.Reader) (o *Subtitles, err error) {
// Split on ":"
var split = strings.Split(line, ":")
if len(split) < 2 || split[0] == "" {
log.Printf("astisub: not understood: '%s', ignoring", line)
opts.OnInvalidLine(line)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once OnInvalidLine is a field, you'll need to wrap this with if opts.OnInvalidLine != nil

ssa.go Outdated
@@ -134,6 +134,12 @@ var ssaRegexpEffect = regexp.MustCompile(`\{[^\{]+\}`)

// ReadFromSSA parses an .ssa content
func ReadFromSSA(i io.Reader) (o *Subtitles, err error) {
o, err = ReadFromSSAWithOptions(i, SSAOptions{})
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to replace SSAOptions{} with defaultSSAOptions()

Copy link
Owner

@asticode asticode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@asticode asticode merged commit 9f2ecac into asticode:master Nov 17, 2021
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.

None yet

3 participants