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

Don't overwrite events unnecessarily when parsing #482

Merged
merged 7 commits into from
Mar 26, 2020

Conversation

Thalexander
Copy link
Collaborator

Fixes #453

Adds a date_parsed and time_parsed property to determine whether to overwrite existing data. This may not handle the phrase "for x minutes/hours" (and equivalent German) correctly, however this was already not correct and so I would consider it a separate issue, we will need to determine if the date changes when saying a length that goes over the current day, I kept the existing behaviour of wiping data for these cases. Does not implement guest parsing.

@jeremypw
Copy link
Collaborator

It seems a bit bizarre that there has to be special code for particular languages hard-coded in just to edit the title of an event. I haven't looked at the code in detail but is this really necessary?

@Thalexander
Copy link
Collaborator Author

It is necessary to convey whether a time or a date has been detected, if we were to handle this in ParsedEvent I believe the only real way of doing so would be to have two separate DateTimes and then we would need to change the languages to not assign a DateTime by default but only if parsed, and then this in turn might complicate interactions when multiple expressions are parsed.

@jeremypw
Copy link
Collaborator

jeremypw commented Mar 13, 2020

@Thalexander I cannot seem to reproduce the issue in master so it is difficult to check this fixes it. Do you have a way of reproducing the issue? I have tried with both US and UK english locales.

Update - dont worry I have found a way of reproducing now.

Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

I can confirm this fixes the issue.

Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

You should make a suitable entry in the appdata for the next release.

@jeremypw jeremypw merged commit af7a66b into master Mar 26, 2020
@jeremypw jeremypw deleted the no-overwrite-parsing branch March 26, 2020 16:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editing the title of an event reschedules it to today's next whole hour
2 participants