Parsing of calendars

[ModProg]

While this crate supports creating icals I would need a parser as well. I was thinking about using https://github.com/Peltoche/ical-rs to add parsing to this crate.

Is this something you would consider merging?

[hoodie]

Nice that you point this out, I considered that too at some point. I think I wrote a whole parser in nom some time ago. maybe we could finally make that a thing.

Structurally both approaches would have the same effect: neither the nom-parser nor ical-rs produce the same types as icalendar-rs, so we need a bunch of From<T> implementations

[ModProg]

Yes, I would also then like to move away from storing everything in Properties and only do that for serialization. That way, we can use the same data structure for parsing from string or building with the builder, and only in the serialization step they get converted to a list of Properties that can be easily converted to a string.

What is your opinion on this approach?

I would be open to use whatever ical parser you prefer, I thought we could just implement From to convert from ical's data type to icalendar's and during that transformation also convert some useful properties like start and end to types usable in Rust.

[ModProg]

The other option would be to parse properties in the get methods, but that could be expensive when getter get called mutiple times.

[ModProg]

One interesting design decision is what to do with the shared properties of components, I think we could either make Event/Todo hold a Component struct or create a Component struct that is generic over the EventType.

Easiest is probably put Component in Event and implement Deref and MutDeref. So that the interface stays mostly the same.

[hoodie]

I'm not sure I followed your proposal to "move away from storing everything in Properties". Do you mean the builder API should produce different types?

[ModProg]

Yes, because if I parse a calendar event, I probably want to read its values, so I thought we could e.g. have a field start of type CalendarDateTime that way someone using icalendar could more easily work with it.

To only have one type for the Components, I would then also just set this field when building a Component, and only convert it to a property when calling to_string.

I hope this makes a little more sense.

[hoodie]

I think I let the builder pattern and the parser both produce relatively generic types because the spec allows for a huge amount of properties. If you make all of them a struct field you would have to make all of them optional and still miss a few they you weren't aware of. This would not be very pretty. The spec is kinda large.

[ModProg]

I think I let the builder pattern and the parser both produce relatively generic types because the spec allows for a huge amount of properties. If you make all of them a struct field you would have to make all of them optional and still miss a few they you weren't aware of. This would not be very pretty. The spec is kinda large.

That makes sense, I would not remove the properties storage for unknown fields, but I think it would be ok to add the most commonly parameters as fields, like start, end, summary etc.

[ModProg]

Especially those that have non string types like start.

[hoodie]

I don't know, there are still events that do not have those. How about we just have a bunch of getters for the most common fields? Oh and I just noticed, you probably only want this for the parsed types right? because the builder types already have methods like Component::starts() because they're sorta write only.

What's your usecase BTW?

[ModProg]

I build a terminal calendar comparable to calcurse that will support syncing via webcal.

Therefor, I need to both parse and write icalendar data.

Oh and I just noticed, you probably only want this for the parsed types right? because the builder types already have methods like Component::starts() because they're sorta write only.

Well I thought it would make sense to use the same type for serialization and deserialization.

Because then I could use that type in my application for example if the user edits an entry and I need to push it.

[hoodie]

Well I thought it would make sense to use the same type for serialization and deserialization.

It makes sense to not have to use two distinct types on the public API. But I think the builder-pattern usecase and the parsing/reserialization usecase have a bit different requirements on the underlying API. So I wouldn't try to force two things into the same type but maybe have a nicer abstraction on top.

[hoodie]

wanna give the current main branch a try? @ModProg I haven't released it yet, but I'll probably do so this week

[ModProg]

I will have a look probably today

[ModProg]

One thing I noticed Calender parsing only supports components and no properties right?

[hoodie]

Actually it should parse everything. If something doesn’t work I’d appreciate a reproducer.

[ModProg]

I meant calendar properties:

pub fn read_calendar(input: &str) -> Result<Calendar<'_>, String> {
    components(input)
        .finish()
        .map(|(_, components)| Calendar { components })
        .map_err(|e: VerboseError<&str>| format!("error: {}", convert_error(input, e.clone())))
}

that is creating a Calendar with only components or did I misunderstand that?

[hoodie]

ah, right that is a helper type, but actually a calendar is a component itself they can be nested. so that Calendar should only have one component, which is the whole calendar. If that is parsed incorrectly then maybe I should check again. Technically you should not see this type, I'm not sure if I'll make it public at all. You can use .parse::<icalendar::Calendar>(), that is a different Calendar struct.

[ModProg]

oh ok, my bad I'll have a look, I accidentaly imported the parse::Calendar :D

[hoodie]

that was good feedback, I'll make sure to not expose it, it's confusing