Skip to content

Fix Date Parser Accidentally Parsing Two-Digit Number as Year #4242

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bytrangle
Copy link
Contributor

Description

When user creates a new task which includes short syntax for date, then following by a two-digit number, the number will be interpreted as year.

For example: "Task @14/4 90m". Implication: User schedules the task for 14/4 going forward and estimates the task as taking 90 minutes. Two problems:

  • This task will be scheduled for 14/4/1990 because Chrono understands 90 as 1990.
  • The task title will include the word "m" when it should have been escaped.

Changes I've made:

  • Write a custom parser that checks if the task title contains string in the format of DD/MM YY. The regex pattern is more complicated than that, because there are so many ways a user can write date informally. It doesn't take into account syntax like "August 8 90m".
  • The parser will set the year for the task to the current year or a year in the future
  • The parseScheduledDate is changed so that the word "m" won't be a part of the final task title.

Issues Resolved

Potentially fixes #4194

Check List

  • New functionality includes testing.
  • New functionality has been documented in the README if applicable.

@johannesjo johannesjo requested review from Copilot and johannesjo and removed request for Copilot April 10, 2025 21:05
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

src/app/features/tasks/short-syntax.ts:321

  • [nitpick] The hacky check for a time estimate appended to the date might not handle all edge cases. A clearer separation of date and time estimate parsing would improve robustness.
if (inputDate.match(/ [0-9]{1,}m/g)) {

if (matched) {
if (matched[matched.length - 1]) {
const twoDigits = matched[matched.length - 1];
result.text = text.replace(twoDigits, '');
Copy link
Preview

Copilot AI Apr 10, 2025

Choose a reason for hiding this comment

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

Replacing the two-digit year indiscriminately may remove matching digits elsewhere in the string. Consider targeting the exact location of the match or using a more precise replacement strategy.

Suggested change
result.text = text.replace(twoDigits, '');
const startIndex = matched.index + matched[0].lastIndexOf(twoDigits);
const endIndex = startIndex + twoDigits.length;
result.text = text.slice(0, startIndex) + text.slice(endIndex);

Copilot is powered by AI, so mistakes are possible. Review output carefully before use.

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.

Date and time syntax not working as expected in quick-add (might be me)
2 participants