search

vipzhicheng / logseq-plugin-block-calendar

A block calendar for Logseq.
MIT License
39 stars 13 forks source link

Fixed issue with leap years + added some tests #35

Closed markscamilleri closed 12 months ago

markscamilleri commented 1 year ago

I noticed that the leap years were being calculated incorrectly, by using the divisible by 4 method. Unfortunately, this method does not take into account that leap years that are divisible by 100 are not leap years unless they are also divisible by 400 (source)

As a result, and becuase I noticed that dayjs also provides a helper function to get the days in a month, I thought it would make sense (and make the code cleaner) to use that helper function.

I also took the liberty of writing a few (imperfect) tests for this including some snapshot tests to help test the output of the setCal function. For the tests, although I wanted to use node:test, I had to install jest as my environment was not able to locate note:test. I'm happy to change to another framework or even node:test and try get it working on my environment.

markscamilleri commented 1 year ago

@vipzhicheng sorry for tagging but I haven't received any feedback for 2 weeks 😅

wensonsmith commented 1 year ago

Dose this pr can resolve #34 ? Hope release soon

markscamilleri commented 1 year ago

Not sure - but I can have a look later this week. Though I suspect that this might be a separate issue, as I replaced the parts of obtaining the last day in the month 😅

vipzhicheng commented 12 months ago

Sorry for the delay and thanks for your PR. I have reproduced the issue and tested your code. It works perfectly. Will add it to the next release.