Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add default alarm configuration option #1048

Merged
merged 8 commits into from
Nov 14, 2023
Merged

Conversation

sdx23
Copy link
Contributor

@sdx23 sdx23 commented May 21, 2021

fixes #698

Not sure where I put it is the best place to, please comment on that.

Also: I think in the existing options "default_(day)event_duration" the terms "dayevent" and "event" are mixed up. I'd expect "dayevent" to refer to an day-long event and in contrast to that "event" an event of e.g. 1h duration, but the implementation is the other way round. My commit, nevertheless, uses this same (possibly reverse) meaning.

@geier
Copy link
Member

geier commented Oct 30, 2023

Thanks and sorry for the 2.5 year delay in taking a look at this.

I have rebased this on the latest master. I also think it's a good idea to have this in ikhal and while it's simple to add a default alarm for a new event, I'm not yet sure on how to convert the alarm if one changes the event from daytime to allday. Perhaps check if the alarm is equal to the default and change it then to the other default?

@geier geier force-pushed the defaultalarm branch 2 times, most recently from a066567 to e0185a4 Compare October 30, 2023 23:04
@geier
Copy link
Member

geier commented Oct 30, 2023

Also: I think in the existing options "default_(day)event_duration" the terms "dayevent" and "event" are mixed up. I'd expect "dayevent" to refer to an day-long event and in contrast to that "event" an event of e.g. 1h duration, but the implementation is the other way round. My commit, nevertheless, uses this same (possibly reverse) meaning.

Good catch and I have no idea how that could be that wrong for so many years. Hopefully fixed.

@geier
Copy link
Member

geier commented Oct 30, 2023

I'm not yet sure on how to convert the alarm if one changes the event from daytime to allday. Perhaps check if the alarm is equal to the default and change it then to the other default?

implemented that way.

@geier geier requested a review from WhyNotHugo October 30, 2023 23:19
sdx23 and others added 8 commits November 1, 2023 16:12
`default_event_duration` and `default_dayevent_duration` where mixed up. The
default for `default_event_duration` was 1 day and it was used as the
default lengths for new all-day events, now it is used for datetime
events and the default is 1 hour. The opposite was true for
`default_dayevent_duration`.
@geier geier merged commit 92f5b88 into pimutils:master Nov 14, 2023
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Default alarm
2 participants