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

proposal: Support no indentation option in func (Subtitles) WriteToTTML #116

Closed
min0625 opened this issue Oct 5, 2024 · 3 comments
Closed

Comments

@min0625
Copy link
Contributor

min0625 commented Oct 5, 2024

Background

I noticed that the WriteToTTML function always indents the output. This is not a problem in most cases, but in some cases, storage space is limited, you may not want indentation.

Proposal

I propose that:

  • New method WriteToTTMLWithOptions in Subtitles.
  • New type TTMLWriteOptions for WriteToTTMLWithOptions.

Define the following:

// WriteToTTMLWithOptions writes subtitles in .ttml format with options.
func (s Subtitles) WriteToTTMLWithOptions(o io.Writer, opts TTMLWriteOptions) (err error) {
    // ...

	// Marshal XML
	var e = xml.NewEncoder(o)

    // Indent
    if opts.Indent != nil {
        e.Indent("", *opts.Indent)
    } else {
        e.Indent("", "    ")
    }

    // ...
}

// TTMLWriteOptions represents TTML write options.
type TTMLWriteOptions struct {
    Indent *string // Default is 4 spaces.
}
min0625 pushed a commit to min0625/asticode-go-astisub that referenced this issue Oct 5, 2024
@asticode
Copy link
Owner

asticode commented Oct 6, 2024

Good idea! 👍

However I'd rather not have an additional WriteToTTMLWithOptions() method but instead:

  • rename TTMLWriteOptions into WriteToTTMLOptions
  • make Indent a string
  • add a type WriteToTTMLOption func(o *WriteToTTMLOptions)
  • transform WriteToTTML into WriteToTTML(o io.Writer, opts ...WriteToTTMLOption)
  • at the beginning of WriteToTTML do something like
// Create write options
wo := &WriteToTTMLOptions{Indent: "    "}
for _, opt := range opts {
  opt(wo)
}
  • add
func WriteToTTMLWithIndentOption(indent string) WriteToTTMLOption {
  return func(o *WriteToTTMLOptions) {
    o.Indent = indent
  }
}

and use WriteToTTML(r, WriteToTTMLWithIndentOption("")) to remove indentation

@min0625
Copy link
Contributor Author

min0625 commented Oct 6, 2024

LGTM. I will adjust the PR later.

min0625 pushed a commit to min0625/asticode-go-astisub that referenced this issue Oct 6, 2024
@min0625
Copy link
Contributor Author

min0625 commented Oct 6, 2024

Hi @asticode,

The PR is ready for review. Thanks.

asticode pushed a commit that referenced this issue Oct 7, 2024
* feat: Issues-116 support options for WriteToTTML

Issue: #116

* chore: rename TestTTMLWithOptionsNoIndent to TestWriteToTTMLWithIndentOption

link: #117 (comment)

* chore: use ./testdata/example-in.ttml

link: #117 (comment)

---------

Co-authored-by: Min <[email protected]>
@asticode asticode closed this as completed Oct 8, 2024
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

No branches or pull requests

2 participants