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

All day event process correctly with --notstarted, fix #1004 #1005

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ephase
Copy link
Contributor

@ephase ephase commented Jan 18, 2021

When you do khal list --notstarted, the "today" all-day event is displayed. This MR avoid this problem.

With `khal list` and `khal calendar`
@ephase ephase linked an issue Jan 18, 2021 that may be closed by this pull request
Copy link
Collaborator

@d7415 d7415 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see a problem with this :)

@d7415
Copy link
Collaborator

d7415 commented Jan 24, 2021

Hmm, actually...

Copy link
Collaborator

@d7415 d7415 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took a while, but (like I said in #1004) this would require us to change the documented behaviour. This would also prevent excluding multi-day events (that have started before the chosen day) but showing new events on that day. It's also confusing because in this form khal list --notstarted will show events that have started today (or even ended) unless they're all day events.

@d7415
Copy link
Collaborator

d7415 commented Jan 24, 2021

Fixing the bug (that khal list now --notstarted still shows all-days) without breaking this looks to be a pain.

@ephase
Copy link
Contributor Author

ephase commented Jan 24, 2021

Hi @d7415, so if I understand, I need to create a exception to display multi-day events? For example event begins yesterday and ends in 3 day, khal list now --notstarted should display my event for tomorrow and next days but not for today?

@d7415
Copy link
Collaborator

d7415 commented Jan 24, 2021

Hi @d7415, so if I understand, I need to create a exception to display multi-day events? For example event begins yesterday and ends in 3 day, khal list now --notstarted should display my event for tomorrow and next days but not for today?

No. In that example the event has started so shouldn't be shown (and the current code won't show it)

My problems with this PR are:

  • There should be a way to hide events like your example, while showing all-day events today (e.g. hide that I'm on holiday all week, show that it's my friend's birthday today). The current code does this, but the PR does not.
  • This PR results in an inconsistency where running khal list --nostarted at midday will hide an all-day event for today, but show an event that happened this morning.

There is still a bug from #1004 where if a time after midnight is specified (e.g. with now) all day events show in --notstarted. But that looks like we'd have to special-case a time (other than midnight) being specified, which is probably why it wasn't done in the first place. (Well, that and showing today's all-day events in --notstarted isn't likely to cause a major inconvenience)

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.

Day events and --notstarted
3 participants