Fix newlines

[westy92]

Per the spec, lines should end with \r\n, not just \n.

4.1 Content Lines

The iCalendar object is organized into individual lines of text,
called content lines. Content lines are delimited by a line break,
which is a CRLF sequence (US-ASCII decimal 13, followed by US-ASCII
decimal 10).

https://www.ietf.org/rfc/rfc2445.txt

[Peltoche]

Hello @westy92, thanks for this PR.

It seems that you are right about the specs but, as you can see with the tests, you change is not retro-compatible. I have only supported the "classic newline" (\n) because all the vcard/vcal I have found was using it.

Could you change your PR to make it able to handle both \n and \r\n please?

[westy92]

@Peltoche thanks for the feedback, I'll try to get this fixed ASAP. Everything initially passed locally - it looks like I need to pass --all-features to run all tests and see these failures.

[westy92]

@Peltoche all tests are now passing!

[Peltoche]

Hello @westy92, happy new year and sorry for the delay. I have a lot of stuff going on in my private life and this lib is really not the first of my priorities. But we will succeed to merge this PR soon I hope!

I only have one issue with your PR, you change the previous tests and this indicate that you are creating a change in behaviour. I don't think we need or should create a breaking change to solve you issue. If I handled only the \n and nobody have raised and issue before is because the parser is based on a lot of real case scenarios which use \n as newline indicator.

So could you change the code to handle both \n and \r\n` as a newline indicator please? This should avoid to change the current behaviour and allows us to have to correct behaviour.

Thanks again for you contribution.

[westy92]

Hi @Peltoche.

Thank you for the additional PR feedback.

The parsing code is unchanged, along with its unit tests. The only code that is different is the generator.

I added a unit test with a spec-compliant ICS file to prove this.

If anything, this is a bug fix to bring ICS generated by the library into compliance.

[Peltoche]

Indeed, I have done the review too quickly. I merge it. Thanks for the contrib 👍

[westy92]

Thank you for the approval, merge, and release! 🎉