Closed rkoshak closed 2 years ago
Second attempt at this capability. I believe I've addressed all the comments from the previous PR and probably introduced all sorts of new ones since it's pretty much a complete rewrite.
I removed some functions as they probably are not that useful overall (toYesterday, toTomorrow) and which I no longer needed because I used a different implementation for betweenTimes. I added a new monkey patched isClose()
method that tests the current ZDT and a passed in ZDT to see if they are within a time.Duration apart. I originally wrote it as part of my testing code but decided it's pretty useful in general.
I didn't say it which was a mistake.
This is ready for review.
The requested changes have been made except for adding back in stuff that shouldn't have been in this PR in the first place.
I'm still confused about why it's @memberof time
as opposed to @memberof time.ZonedDateTime
for the monkey patch functions. I've left them as just @memberof time
and removed the tag from the functions that are not exposed.
as opposed to
@memberof time.ZonedDateTime
for the monkey patch functions
Yes, that would make more sense, but I do not know if that works with the JSDoc htmls generated. You can give it a try and run npm run docs
and then have a look at the generated htmls. I think you need to define a new namespace time.ZonedDateTime
.
Interestingly, neither time
nor time.ZonedDateTime
generates anything at all for the monkey patched methods. I'm not sure what to do. Since neither work it seems like I should remove them and we'll just have the docs added to the README.MD for the new functions.
Since neither work it seems like I should remove them and we'll just have the docs added to the README.MD for the new functions.
I would‘t remove them as they are at least helpful when scrolling through the source code, but yes, the README seems to be the only place where the monkey-patched functions can be documented (outside the source code).
In that case I'd say let's just leave them as is and consider this PR good to go.
Some conflicts popped up from an earlier PR that was merged, can you fix those? Thanks!
Fixing the conflict was easy for README.MD but it's marking everything I've added to time.js as a conflict. How to I fix this without basically reversing everything I've added? I don't have write access so I can't tell it to use my version instead of main. I'm not sure what to do.
I don't have write access so I can't tell it to use my version instead of main. I'm not sure what to do.
Your PR is all that needs to be changed. Fetch the main
branch, then in your PR branch do a git merge main
or you can use git rebase main
. This will result in conflicts and give instructions on how to proceed. From here you remove the lines in the file which show the conflicts like <<<<<<< toZDT
, this should be simple as the conflicts are all new inserts, nothing is actually being changed from main (its odd git didn't handle this better), then do a git add time.js
and do a commit and push.
I went ahead and fixed the conflicts , so all is merged now.
Thanks! I thought I wasn't supposed to fetch from main because last time I did that in the toggle PR I ended up with someone else's PR mixed in and it appears to have caused all sorts of problems. Sometimes I miss the simplicity of good old CVS. I feel like a bull in a china shop every time I mess with a PR on Github.
The good news is I think I've figured out Mocha so I'll be able to add some unit tests in a separate PR when I can implement them. I'll look into what it would take to create tests for as much in the library as we can.
[time] Time Utils
Description
Implements a number of utilities for working with date times.
time.toZDT()
: converts almost anything that can reasonably be converted to a date time to a ZonedDateTime. See the docs for the rules used when converting (e.g. a number is added to now as milliseconds, '1:23 PM' returns a date time with today's date and 13:23:00 for the time, etc.).zdt.toToday()
: returns a newtime.ZonedDateTime
withzdt
's time and today's date, accounting for DST changes.zdt.betweenTimes(start, end)
: returnstrue
ifzdt
's time is between the times (ignoring date) represented bystart
andend
. If theend
time is beforestart
, the time range is assumed to span midnight.start
andend
can be anything supported bytime.toZDT()
, for examplenow.betweenTimes(items.getItem('Sunset'), '05:00');
.zdt.isClose(otherZDT, maxDuration)
: returns true if the difference betweenzdt
andotherZDT
is withinmaxDuration
.Testing
I used the following in a MainUI Script to test this library .
The functions used from
testUtils
are:These are a part of openhab_rules_tools.