GNOME Bugzilla – Bug 561812
Support time_t
Last modified: 2008-12-01 18:20:59 UTC
This patch assumes that the host system uses "long" for time_t, which is true for GNU systems. Right now gobject-introspection also assumes this.
Created attachment 123175 [details] [review] Support time_t
Comment on attachment 123175 [details] [review] Support time_t >@@ -333,6 +333,8 @@ gjs_value_to_g_argument(JSContext *context, > #if (GLIB_SIZEOF_LONG == 8) > case GI_TYPE_TAG_LONG: > case GI_TYPE_TAG_SSIZE: >+ /* TODO - support systems where time_t is a double, etc. */ >+ case GI_TYPE_TAG_TIME_T: > #endif Isn't this missing GLIB_SIZEOF_LONG == 4 case? Can't see if it would support Date -> time_t conversion either, not entirely sure it should even. >+jsval >+gjs_date_from_long (JSContext *context, long time) >+{ >+ return js_NewDateObjectMsec (context, (double) time); >+} Hmm, this looks like private JS API. I don't think we're supposed to call it. >+ let now = new Date(); >+ let retDate = Everything.test_timet(now); >+ /* Just test year/month since JS represents times as >+ * doubles internally (whereas GNU libc uses long), and >+ * I am skeptical about this test not randomly failing >+ * later if we try to be completely precise. This >+ * conversion though is likely to be good enough for most >+ * purposes. >+ */ >+ assertEquals(now.getFullYear(), retDate.getFullYear()); >+ assertEquals(now.getMonth(), retDate.getMonth()); I think this should test up to the second. At least that's pretty much the expected precision usually for time_t.
+jsval +gjs_date_from_long (JSContext *context, long time) +{ + return js_NewDateObjectMsec (context, (double) time); +} I don't understand how that works ... as far as I can see js_NewDateObjectMsec() wants a Java style milliseconds since the epoch, not a Unix style seconds-since epoch. (And as Tommi said, js_* stuff is generally private API, AFAIK only the JS_* stuff in jsapi.h is supposed to be public.) IMO you do want bidirectional conversion to Date. What's unclear to me is if passing a number in instead should be supported. If you do support it, is it a seconds or a milliseconds value? Is a timet something like a whippet?
(In reply to comment #3) > IMO you do want bidirectional conversion to Date. What's unclear to > me is if passing a number in instead should be supported. If you do > support it, is it a seconds or a milliseconds value? I'd think we'd use doubles for seconds. time_t as long doesn't support higher precision anyway, and when it's represented as double there'd be direct mapping anyway.
(In reply to comment #4) > I'd think we'd use doubles for seconds. time_t as long doesn't support higher > precision anyway, and when it's represented as double there'd be direct mapping > anyway. That is almost certainly right; the only reason I was hesitating at all was the plan to make widget.set_rectangle({ x: 2, y: 2, width: 10, height: 10 }); The same as: widget.set_rectangle(new Gdk.Rectangle({ x: 2, y: 2, width: 10, height: 10 })); Then it's a little weird to me if: widget.set_time(342123412); works but isn't the same as: widget.set_time(new Date(342123412));
Right, it is a little weird, though the cases are subtly different. set_rectangle() expects a Rectangle which we can implicitly construct from {..} -- or you can explicitly create a Rectangle yourself set_time() expects time_t which we can implicitly construct from 342123412 (numbers) or Date objects -- if you explicitly create Date objects we expect you know how to do that properly I guess what I'm trying to say is that set_time() expects time_t, not Date, which makes the difference. (If it expected Date the inequality between the two would be a big mistake IMO.)
Created attachment 123185 [details] [review] Support time_t This patch assumes that the host system uses "long" for time_t, which is true for GNU systems. Right now gobject-introspection also assumes this.
The above patch does not work - I can't figure out how to get a reference to the JS Date "class". All the code I can find around on the web is mozilla internals which are using the internal API, and the public API points around this stuff are undocumented and obscure. It does address some of the above comments though.
A hacky way to get the JSClass* for Date would be to eval "new Date()" and then call JS_GetClass() on the result. But surely there is some more sane alternative to that ;-)
Date is a global JS class / function, so you should be able to just call it with the right arguments. See https://developer.mozilla.org/En/SpiderMonkey/JSAPI_Phrasebook#Calling_a_global_JS_function and https://developer.mozilla.org/En/SpiderMonkey/JSAPI_Phrasebook#throw -- look for JS_CallFunctionName
I believe if you call "Date()" you'll get a string back, as if you did it in JS. There doesn't seem to be any way to invoke a function as a constructor in the jsapi.h other than JS_ConstructObject. A guess for getting the class Use JS_GetProperty to get the "prototype" property of the Date function Call JS_GetClass on that
Created attachment 123189 [details] [review] Support time_t This patch assumes that the host system uses "long" for time_t, which is true for GNU systems. Right now gobject-introspection also assumes this.
+#if (GLIB_SIZEOF_LONG == 8) + (time_t) arg->v_long); +#else + (time_t) arg->v_int); +#endif If GLIB_SIZEOF_LONG == 4, isn't arg->v_long the same as arg->v_int? If not, this will break with gfield.c - since that does: value->v_long = G_STRUCT_MEMBER(time_t, mem, offset); To read a time_t field. + /* FIXME - using floating point for this is broken */ + if (!JS_ValueToNumber(context, value, &v)) + wrong = TRUE; Is not :-) + if (!localtime_r (&time, &broken)) + gjs_fatal("Failed to convert time"); + + args[0] = INT_TO_JSVAL(1900 + broken.tm_year); + args[1] = INT_TO_JSVAL(broken.tm_mon); + args[2] = INT_TO_JSVAL(broken.tm_mday); + args[3] = INT_TO_JSVAL(broken.tm_hour); + args[4] = INT_TO_JSVAL(broken.tm_min); + args[5] = INT_TO_JSVAL(broken.tm_sec); + args[6] = INT_TO_JSVAL(0); OTOH, that... Looking at jsdate.c: JS_FRIEND_API(JSObject *) js_NewDateObject(JSContext* cx, int year, int mon, int mday, int hour, int min, int sec) { JSObject *obj; jsdouble msec_time; JS_ASSERT(mon < 12); msec_time = date_msecFromDate(year, mon, mday, hour, min, sec, 0); obj = js_NewDateObjectMsec(cx, UTC(msec_time)); return obj; } So instead of just converting to a double and multiplying by 1000, you looked up timezone data, and did two complex calender computations (involving leap seconds and who knows what else...) to convert it to a different representation and back again.
Created attachment 123193 [details] [review] Support time_t This patch assumes that the host system uses "long" for time_t, which is true for GNU systems. Right now gobject-introspection also assumes this.
Yeah, I guess it makes sense to just give up and code to SpiderMonkey since that's what gjs is. Fixed the first sizeof issue, thanks.
(In reply to comment #14) > Created an attachment (id=123193) [edit] > Support time_t > > This patch assumes that the host system uses "long" for time_t, which > is true for GNU systems. Right now gobject-introspection also assumes > this. > This looks good to me. Did you leak check it? (make valgrind-check iirc)
Sorry for the delay - to verify I hadn't introduced any new leaks I had to do some valgrind cleanup (bug 562892). This patch is now committed. Committed r123 M test/js/testEverythingBasic.js M gjs/jsapi-util.c M gjs/jsapi-util.h M gi/arg.c r123 = 6cdf0495eb4980c3e471b65a6bc2b6c148ba34bf (git-svn)