ical.js Create Nodes Array

Create an array of nodes, instead of storing the node under UID property or random property.

var cal = ical.parseICS(str);
cal.vevent // <Array>

Mar 06 '14 19:03 tenbits

Hi, This breaks a bunch of the tests - can you take another look?

Thanks!

Mar 06 '14 23:03 peterbraden

@peterbraden sure, if all the features from those 3 prs are acceptable, then I'll close them, merge und fix the tests. Cheers

Mar 07 '14 00:03 tenbits

The features are great - if we can get them in the test suite I'd be happy to pull them! Thanks for the PR's

Mar 11 '14 00:03 peterbraden

Hi Peter, sorry for some delay, all the week I was at CeBIT. May I ask you, if it were possible:

to change all the tests to the utest runner
to break the library into some partial files as, for example, we do this at mask
to change the indention style code - to 4 spaces, or better to tabs.

These things are all not very important, but will make me much easier to work at the project. But is also OK, If only some, or even none, of the proposed changes you accept.

Cheers, Alex

Mar 15 '14 10:03 tenbits

I'd prefer not to make any changes as big as switching the test runner or changing spaces. I'd be happy to consider breaking the module out if it looked more manageable, but it might be better to do this as a separate PR to keep things clean.

Thanks.

Mar 17 '14 22:03 peterbraden

@peterbraden , I have add id feature back, the tests are green know, but for the future it would be better to remove it and to rewrite the tests. As it is much better to store nodes in an array, then under random key names.

... and as we stick to the lowercase convetion of the properties, I transform the type also to lowercase

Mar 23 '14 22:03 tenbits

Can you explain a little bit about what this PR is actually doing?

The reason we are assigning a UID rather than storing in an array is to more closely match the spec: https://tools.ietf.org/html/rfc5545

Apr 04 '14 19:04 peterbraden

It adds array grouping by a type to the calendar instance. So that all, for example, BEGIN:VEVENT ... entries are stored in the array calendar.vevent. Firstly, It is semantically better and secondly, it is easier to perform some work on the array - loop, add events, remove events, etc. Spec is good for storing(serialization/deserialization) rules, but in-memory data structure representation can be selected that, which better suites. And it is definitely the List<Item> structure. Or I miss something? ;) Cheers

Apr 04 '14 19:04 tenbits

I'm kinda torn on this because I want to keep this module completely focused on the serialization/deserialization aspect of this, I don't necessarily want to make assumptions about what will happen with the data. Is there a compelling reason to do this as part of the parsing, or is this something that applications can do afterwards?

Apr 05 '14 21:04 peterbraden

My point was, that in-memory representation structure is not dependent on the spec in this case. I found nothing that says, that child object should be stored(in memory) under the correspondent UUID property in its parent. Or can you point me on this? So I assume, we can handle the calendar object as we "want", then storing Events, Todos etc. in an array structure is the only right way. Always think about the consumers of this module. E.g. how I know that a VTODO component is present, or how many of them are present? Loop over all keys in the calendar object and check the type? Is it not better just to use calendar.vtodo.length? Or what about loops, calendar.vevent.forEach approach is much better. But if you want to stay with random uuid keys, than you should at least provide some calendar wrapper class to perform such simple tasks. Then you will notice, that arrays are good choice.

Apr 05 '14 22:04 tenbits