Adding line folding

[Dynom]

Following the recommendation, I've added line-folding with multi-byte character support.

Feel free to fire away review feedback :-)

Couple of notes:

• The generated ICS validates on: https://icalendar.org/validator.html
• The spec shows N+1 indent per line, but I think the specification is somewhat ambiguous and I've found real-world examples who continue with just 1 indent per subsequent line.
• This PR introduces a dependency on at least Go 1.10

[emersion]

Thanks for sending a PR. Have you noticed that not folding lines breaks some calendaring software?

In my experience, line folding is hard to get right and has been the cause of many bugs. So I'd rather not add it if everything works fine without it.

[Dynom]

Hi, and thanks for your reply.

I haven't had a problem with long line lengths just yet.

However in our case, which might be a niche one, we allow for quite a bit of content in the description field. In this case I've followed a similar approach as to how Google generate's their ICS files and so far it validates and is accepted by the clients I've tested (macOS, Outlook and gmail).

Would you consider making this an opt-in feature? I could change the PR to feature-toggle it.

The approach I'd be considering would be to introduce functional options, to keep BC:

Changing:

func NewEncoder(w io.Writer, options ...Option) *Encoder {
  enc := Encoder{w:w}
  for _, o := range options {
    o(&enc)
  }

  return &enc
}

Introducing:

type Option func(enc *Encoder)
func WithLineFolding() Option {
  return func(enc *Encoder) {
    enc.maxLineLength = 75
  }
}

Using:

// Backwards compatability
enc := ical.NewEncoder(w)

// With line-folding
encWithLineFolding := ical.NewEncoder(w, WithLineFolding())

Changing line folding on the encoder isn't really a problem, since it's marshalling logic, but this might be a clean API to have a clear intent from the start.

[Dynom]

In my experience, line folding is hard to get right and has been the cause of many bugs. So I'd rather not add it if everything works fine without it.

To learn from your experiences, what kind of bugs did you encounter with line folding?

[emersion]

From a quick look, here are a few issues I've hit:

https://github.com/emersion/go-msgauth/issues/18 https://github.com/emersion/go-msgauth/issues/23 https://github.com/emersion/go-msgauth/issues/36 https://github.com/emersion/go-message/issues/54 https://github.com/emersion/go-message/issues/44 https://github.com/emersion/go-message/pull/22

Would you consider making this an opt-in feature?

No.

we allow for quite a bit of content in the description field. In this case I've followed a similar approach as to how Google generate's their ICS files and so far it validates and is accepted by the clients I've tested (macOS, Outlook and gmail).

Why is folding necessary for long description values?

[Dynom]

From a quick look, here are a few issues I've hit:

emersion/go-msgauth#18 emersion/go-msgauth#23 emersion/go-msgauth#36 emersion/go-message#54 emersion/go-message#44 emersion/go-message#22

I understand that a bad experience from line-folding in general can be discouraging, E-mail headers and DKIM in particular can be a bit finicky.

we allow for quite a bit of content in the description field. In this case I've followed a similar approach as to how Google generate's their ICS files and so far it validates and is accepted by the clients I've tested (macOS, Outlook and gmail).

Why is folding necessary for long description values?

It's not. The specification mentions that they should, not that they must be limited to 75 bytes.

I was porting an implementation from a different language and the difference was, besides the key-sorting go-ical does, on the line lengths.

I'll close the PR as you've made your point clear on the matter.