search

cozy / cozy-ical

ICal Parser and Calendar models for Cozy Cloud
http://cozy.io
Other
29 stars 11 forks source link

Add X-WR-CALNAME field support, change readline algorithme in parse. #22

Closed jacquarg closed 9 years ago

jacquarg commented 9 years ago

Add standard but non-normative X-WR-CALNAME field support to export and import calendar name. Change readline algorithme for file parse. Use a custom line by line in order stream reader, instead of lazy.

jsilvestre commented 9 years ago

I am kinda afraid to merge that. Have you made more manual testing? Does it solve your bug on you bigger file?

jacquarg commented 9 years ago

I agree with your feeling ! I did some manual tests (sadly, it didn't work on first tests : ), but now it works with my big files and solves the bug.

The integration of the new line parser were effortlessly, and is ugly. I will take a look if it can be cleaned.

jsilvestre commented 9 years ago

@frankrousseau since you have written the first version of the parser, can you have a look at the code too?

frankrousseau commented 9 years ago

If you want to change it, I think we should make it fully based on stream. Here is a nice lib that will help you to achieve that: https://www.npmjs.com/package/byline

You can make the parser inherit from:

 http://nodejs.org/api/stream.html#stream_class_stream_readable

And use standard functions like pause.

jacquarg commented 9 years ago

Ok, I will look at it. There is indeed no need to reinvent the wheel, as there isn't preemption in nodejs.

frankrousseau commented 9 years ago

It's not about reinventing the wheel, it's about matching the usual Node API. Currently, you named a class as a ReadStream, so it should match the Node API. And I prefer to make things the Node way instead of renaming the class.