~whynothugo/public-inbox

1

valarmd development

Details
Message ID
<20221216132356.mhb75o777q3udmjw@navi>
DKIM signature
missing
Download raw message
Hi,

as I have no way of showing notifications with calendar events (and
their reminders) I was looking for a way to have this and stumbled upon
valarmd. It's not done yet but I thought I could build on your
foundation a little and see that it gets a few more of the features it
needs, or have some bugs solved until it's at least "usable".

While trying it out, I noticed that events that repeat and have
notifications are skipped in valarmd. If I understand the standard
correctly, repeating events only have the first date saved and the
following dates are then constructed from properties in the iCalendar
file. So valarmd seems to only check for the first event (the one
explictly specified), but disregards the repeating ones, making it only
show notifications for the first event.
Is this correct?

Do keep in mind that I'm not familiar with the iCalendar format at all
and only started reading a little about it when looking at valarmd, so
some things I'm saying might be false.

I'd love to work on implementing this, although it will take some time
due to not being familiar with the format.
Do you have any started (but not pushed) work on valarmd I need to
consider?

Another thing: While implementing proper handling of repeating events, I
might also throw in a command line parser with clap and a TOML
configuration file that would be in
`$XDG_CONFIG_HOME/valarmd/config.toml`. Is this the desired way of doing
it?
This would be implemented in a similar way to how I implemented it in
rss-email[0], but trying to circumvent potential `unwrap`s of "optional"
fields (optional on the command line, but necessary overall) by
introducing a struct that combines command line and configuration file
options, overriding configuration file options if they have been given
on the command line.

Let me know what you think of this!

Cheers.

[0]: https://git.sr.ht/~witcher/rss-email/tree/61279cd551cd6646abdfe7ba83baee1b3c812318/item/src/cli.rs

-- 
witcher
Details
Message ID
<7a6be7a0-4791-433d-8ad1-7ffc84bcae13@app.fastmail.com>
In-Reply-To
<20221216132356.mhb75o777q3udmjw@navi> (view parent)
DKIM signature
missing
Download raw message
On Fri, 16 Dec 2022, at 14:23, witcher wrote:
> Hi,
>
> as I have no way of showing notifications with calendar events (and
> their reminders) I was looking for a way to have this and stumbled upon
> valarmd. It's not done yet but I thought I could build on your
> foundation a little and see that it gets a few more of the features it
> needs, or have some bugs solved until it's at least "usable".
>

There's one big thing that's broken: timezones. And anything icalendar-related
in Rust is somewhat blocked by this whole situation.

Basically, to use the chrono crate, we'd need an implementation of the
chrono::offset::TimeZone trait, that holds data from a VTIMEZONE component
inside of it. Let's call it VTimeZone. This timezone can then be used for all
sorts of things. Including RRULEs.

We can think of RRULEs in two groups: 

1. The recurrence rule for an event that happens repeats every Monday at
   midday (i.e.: the rule inside a VTODO, VEVENT, etc).
2. The recurrence rule that defines DST transitions and alike inside a
   VTIMEZONE component. These in particular can only use local time or UTC
   time, but never another actual timezone.

The first group can have RRULEs with any timezone (e.g.: including one defined
as a VTIMEZONE component). The second group can only have UTC or local
timezone.

So, in order to properly parse all RRULEs (in particular, those in events and
todos) we need the TimeZone implementation which I mentioned above.

Now, you'll note that a VTimeZone struct will need to have an RRule, but RRule
have DateTimes which can have a VTimeZone. So there's a circular dependency,
implying that both types need to be defined in the same crate.

Some background here, where I initially attempted an approach that failed:
https://github.com/fmeringdal/rust-rrule/pull/85

This change in chrono would make things a lot cleaner, but we have a (somewhat
hacky) workarond for now:
https://github.com/chronotope/chrono/issues/822

So it seems that right now, what needs to happen is a fork of the rrule crate
with vtimezones defined in it. I think I have some WIP code stashed, but
nothing close to working. I say a fork because a lot of work needs to be done
before it all even makes sense, but it would be great to upstream it if the
maintainer is okay with the rrule create suddenly becoming rrule-and-vtimezone.

All this said, the rrule crate is not totally correct in how it parses
timezones now either. There exist counter-examples that don't work properly. I
mention this mostly because it's possible that some notification might come in
1hour late or something like that. This cannot be fixed without addressing the
above.

> While trying it out, I noticed that events that repeat and have
> notifications are skipped in valarmd. If I understand the standard
> correctly, repeating events only have the first date saved and the
> following dates are then constructed from properties in the iCalendar
> file. So valarmd seems to only check for the first event (the one
> explictly specified), but disregards the repeating ones, making it only
> show notifications for the first event.
> Is this correct?

See the README, there's a "Pending still" section.

>
> Do keep in mind that I'm not familiar with the iCalendar format at all
> and only started reading a little about it when looking at valarmd, so
> some things I'm saying might be false.
>
> I'd love to work on implementing this, although it will take some time
> due to not being familiar with the format.
> Do you have any started (but not pushed) work on valarmd I need to
> consider?
>

There's a `timezone` branch, to which I've just pushed an additional commit
with everything in my working copy.

> Another thing: While implementing proper handling of repeating events, I
> might also throw in a command line parser with clap and a TOML
> configuration file that would be in
> `$XDG_CONFIG_HOME/valarmd/config.toml`. Is this the desired way of doing
> it?
> This would be implemented in a similar way to how I implemented it in
> rss-email[0], but trying to circumvent potential `unwrap`s of "optional"
> fields (optional on the command line, but necessary overall) by
> introducing a struct that combines command line and configuration file
> options, overriding configuration file options if they have been given
> on the command line.
>

Sounds reasonable. Tip: For config parameters that are paths, the struct should
hold a PathBuf, not a String. Creating a Config instance with input that's not
confirmed valid should not be allowed (e.g.: a String that is not a valid
PathBuf).

> Let me know what you think of this!
>
> Cheers.
>
> [0]: 
> https://git.sr.ht/~witcher/rss-email/tree/61279cd551cd6646abdfe7ba83baee1b3c812318/item/src/cli.rs
>
> -- 
> witcher
>
> Attachments:
> * signature.asc

Let me know if you have more questions. As you can see, the whole thing is
blocked by nothing that's not minor at all.

That aside, I do want to use vstorage for reading items instead of walking a
dir myself. But that's mostly a nice-to-have.

-- 
Hugo
Reply to thread Export thread (mbox)