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

Added Time class #81

Closed
wants to merge 1 commit into from
Closed

Conversation

clsource
Copy link

A simple way of getting the current unix timestamp

import "os" for Time

System.print(Time.time) // 1619555605

Related:

@ChayimFriedman2
Copy link

Quoting the C99 standard:

The time function determines the current calendar time. The encoding of the value is unspecified.

Instead, we should rely on time() + localtime(). I think the best will be to create a new class DateTime:

foreign class DateTime {
  foreign static now

  foreign dayInYear
  foreign dayInWeek
  foreign year
  foreign month
  foreign day
  foreign hour
  foreign minute
  foreign second
  // Helper functions...
}
MODULE(os)
  // ...
  CLASS(DateTime)
    STATIC_METHOD("<allocate>", dateTimeAllocate)
    STATIC_METHOD("now", dateTimeNow)
    METHOD("dayInYear", dateTimeDayInYear)
    // ...
  END_CLASS
END_MODULE

// ...

static WrenHandle* dateTimeClass = NULL;
// Get handle to `DateTime`...

void dateTimeAllocate(WrenVM* vm) {
  ASSERT(false, "Unreachable");
}

void dateTimeNow(WrenVM* vm) {
  time_t rawTime = time(NULL);
  struct tm* timeInfo = localtime(&rawTime);
  wrenEnsureSlots(vm, 1);
  wrenSetSlotHandle(vm, 0, dateTimeClass);
  struct tm* dateTime = wrenSetSlotNewForeign(vm, 0, 0, sizeof(struct tm));
  memcpy(dateTime, timeInfo, sizeof(struct tm));
}

void dateTimeDayInYear(WrenVM* vm) {
  struct tm* timeInfo = wrenGetSlotForeign(vm, 0);
  wrenSetSlotDouble(timeInfo->tm_yday);
}
// ...

@clsource
Copy link
Author

Well if we need a dedicated date class an option is using the date module from PureFox https://github.com/NinjasCL-labs/wren-rosetta/blob/main/src/modules/date.wren

But the minimum needed is just the unix timestamp for this to work

I dont understand quite why localtime would be needed in addition to time.
I though that just using time was required for getting the unix timestamp.

Is there a corner case that I am missing?

@ChayimFriedman2
Copy link

The problem is that Unix timestamp is not standard (I think time() does return it in the POSIX standard, but as my quote stated, the C standard does not).

We can convert struct tm to a Unix timestamp fairly easily, I just though that if we already have a more extensive struct, maybe we should use it.

@joshgoebel
Copy link
Contributor

joshgoebel commented Apr 28, 2021

Is time even portable to all systems we support? So far I thought we were sticking with mostly/only uvlib things because it provides us portability across multiple systems. All UV seems to offer (that I see is) uv_hrtime and uv_uptime though.

But the minimum needed is just the unix timestamp for this to work

I do think just unix timestamp would be a great start - provided it's cross-platform.

an option is using the date module from PureFox

That module is huge - almost 800 lines. I'm meaning to type up an issue for discussion on the philosophy of the CLI. If it tries to adhere somewhat to the minimalism of Wren Core then I'd think that would rule a large class like that out. After all one can always just include that Wren code in their project easily - there is no need that it must be compiled into the CLI.

The CLI library by necessity needs to do things that are impossible to do from Wren alone - such as load a file, get the system time, open a network socket... but timezone math, date parsing, lunar calendar vs solar calendar vs gregorian calendar are things that could be all easily be added by an external library (like Purefox's). Minimally we'd just need to provide tools to get the system time, so the rough idea proposed @ChayimFriedman2 along those lines looks about right. (I dunno about the time() + localtime() stuff though) Again, making sure it's cross-platform.

I've love to see more Wren code and fewer foreign classes, but that's partly because of the current limitations of foreign classes. I think Wren code is accessible to far more people than C so unless performance is critical I think the more Wren code the better. That said this isn't so bad just wrapping a struct.

The alternative I'd love to see:

class DateTime {
  year { _year } 
  dayInYear { _dayInYear }
  // etc
  construct new(year, month, day, hour, minute, second, etc)
  foreign localtime() 
}

Calling localtime would drop to C, get the time struct, and then call DateTime.new passing it all the data that would then live purely on the Wren side... This also allows setters to be easily added in Wren... (if we decided that was in-scope)

@clsource
Copy link
Author

I think this can be inside a new module called "date" or "calendar" so we can have more alternatives like

Now.unix // returns the current unix timestamp

Now.iso // returns current timestamp in iso8601 format

Now.utc(-4) // returns the current timestamp in iso8601 format with -4 hours

Datetime // would have utility methods for manipulating dates and times ?

@clsource
Copy link
Author

I will close this until futher discusssion about a time class is defined.

I think wren_cli should at least give the current timestamp to allow creating new higher level classes on top :)

@clsource clsource closed this Apr 28, 2021
@joshgoebel
Copy link
Contributor

If we added hrtimer (for benchmarking) would the help you any?

@clsource
Copy link
Author

There is System.clock for benchmarking i think 👍 but that does not counts dates

@joshgoebel
Copy link
Contributor

And it only counts CPU burn not actual clock time... so it's questionable... though I can see why it's mostly ok for benchmarking since ideally you're in CPU burn 100% of the time.

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.

None yet

3 participants