Skip to content

time: check if a day is a valid day of its month - #19232

Merged
medvednikov merged 2 commits into
vlang:masterfrom
ttytm:time.parser/check-leap-year
Aug 29, 2023
Merged

medvednikov merged 2 commits into
vlang:masterfrom
ttytm:time.parser/check-leap-year

Conversation

@ttytm

@ttytm ttytm commented Aug 28, 2023 •

Copy link
Copy Markdown
Member

Closes #19230

πŸ€– Generated by Copilot at 4b56aa7

Improved date validation in time module by adding leap year check in date_time_parser.v.

πŸ€– Generated by Copilot at 4b56aa7

  • Add a new variable needs_leap_year to track if the parsed date requires a leap year or not (link)
  • Assign needs_leap_year to true if the parsed month is February and the parsed day is 0 or 29, otherwise false, for different date formats (link, link, link)
  • Return an invalid time error if needs_leap_year is true but the parsed year is not a leap year, to prevent invalid dates (link)

@ttytm
ttytm marked this pull request as draft August 28, 2023 23:24
@ttytm
ttytm force-pushed the time.parser/check-leap-year branch 3 times, most recently from 6846f60 to c1d1710 Compare August 28, 2023 23:47
@ttytm
ttytm force-pushed the time.parser/check-leap-year branch from c1d1710 to 20fb7b2 Compare August 28, 2023 23:53
@ttytm
ttytm marked this pull request as ready for review August 28, 2023 23:54
@islonely

islonely commented Aug 29, 2023 •

Copy link
Copy Markdown
Contributor

We're probably just gonna have to do a check for most months. The modified code needs to check for day_in_month == 30 as well or '2023-02-30' will still be allowed. Months that should only have 30 days are allowed to have 31 also. June only has 30 days but time.parse_format returns a Time object.

import time
fn main() {
    if t := time.parse_format('June 31, 2023', 'MMMM DD, YYYY') {
        println(t)
    } else {
        println('parse failed')
    }
}

@ttytm
ttytm marked this pull request as draft August 29, 2023 03:35
@ttytm
ttytm marked this pull request as ready for review August 29, 2023 04:04
@ttytm
ttytm force-pushed the time.parser/check-leap-year branch from f339d33 to fe22b0d Compare August 29, 2023 04:06
@ttytm
ttytm force-pushed the time.parser/check-leap-year branch from fe22b0d to 5375779 Compare August 29, 2023 04:08
@ttytm ttytm changed the title time: check if date is valid without leap year time: check if a day is a valid day of it's month Aug 29, 2023
@ttytm ttytm changed the title time: check if a day is a valid day of it's month time: check if a day is a valid day of its month Aug 29, 2023
@spytheman

Copy link
Copy Markdown
Contributor

Needs tests too, to prevent regressions.

@spytheman spytheman left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good work.

@medvednikov
medvednikov merged commit 80db364 into vlang:master Aug 29, 2023
@ttytm
ttytm deleted the time.parser/check-leap-year branch September 1, 2023 00:54
Wertzui123 pushed a commit to Wertzui123/v that referenced this pull request Oct 8, 2023
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.

time.parse and time.parse_format doesn't validate day properly

4 participants