Created attachment 149330 [details] [review]
Install context-jsapi.h

Consumers might want to access the JSAPI directly for quite
a lot of reasons; don't include it in <gjs/gjs.h>, but require
that header to be included first.

Review of attachment 149330 [details] [review]:

I think it should be included by default in gjs.h,
and perhaps a private define should be set before including gjs.h so the caller
knows that it's actually an unstable/unsupported/evil api.

(In reply to comment #2)
> Review of attachment 149330 [details] [review]:
> 
> I think it should be included by default in gjs.h,
> and perhaps a private define should be set before including gjs.h so the caller
> knows that it's actually an unstable/unsupported/evil api.

Hm, is it unstable?  I thought the issue was more that it pulled in all of the Spidermonkey headers.

Honestly...I'd actually rather delete it and just have:

gpointer gjs_content_get_native_context (GjsContext *js_content);

in gjs.h, and consumers can cast this to JSContext *.

I worked around this for now in gnome-shell by just prototyping the function internally, but I'd still like to land some API for this upstream.  Any opinions?  Should we do the #define GJS_USE_LOWLEVEL_API or something?

*** Bug 607592 has been marked as a duplicate of this bug. ***

What is the concern with bringing in the spidermonkey headers?  Just compile speed or what?  It doesn't seem like a big deal to me.

It's a nice modularity I think that if someone is just creating an interpreter and not defining bindings they don't need jsapi.h

I like the gpointer gjs_content_get_native_context (GjsContext *js_context)
idea, that seems very simple.
except use "void *" not that gpointer BS ;-)

Created attachment 152581 [details] [review]
Add gjs_context_get_native_context which allows access to the JSContext

Review of attachment 152581 [details] [review]:

::: gjs/context.c
@@ +660,3 @@
+ *
+ * Returns a pointer to the underlying native context.  Currently this is
+ * a SpiderMonkey JSContext*

I think this would be better said as "For SpiderMonkey, this is a JSContext *". It's not like we'd change it to some other SpiderMonkey object at an undefined point in the future.

@@ +662,3 @@
+ * a SpiderMonkey JSContext*
+ *
+ */

Stray blank line

@@ +666,3 @@
+gjs_context_get_native_context (GjsContext *js_context)
+{
+    g_return_val_if_fail(js_context != NULL, NULL);

GjsContext is a GObject, so replace this with GJS_IS_CONTEXT(context)

@@ +667,3 @@
+{
+    g_return_val_if_fail(js_context != NULL, NULL);
+    return gjs_context_get_context(js_context);

Having 

 JS_Context *gjs_context_get_context();
 void *      gjs_context_get_native_context()

even if they are in different header files strikes me as unattractive; I think you should just get rid of the separate gjs_context_get_context() and accept reduced type safety. (There aren't a lot of places where gjs_context_get_context() is used anyways.)

::: gjs/context.h
@@ +65,2 @@
 GList*          gjs_context_get_all              (void);
+void           *gjs_context_get_native_context   (GjsContext *js_context);

The inconsistency in two adjacent lines in the placement of the '*' really makes this impossible to read.

Created attachment 152588 [details] [review]
Add gjs_context_get_native_context which allows access to the JSContext

Of course, I should amend the commit message to mention that I've also removed gjs_context_get_context()

Sigh.  This is all wrong, standby...

Created attachment 152589 [details] [review]
Add gjs_context_get_native_context which allows access to the JSContext

Also remove gjs_context_get_context() since it is obsoleted

Created attachment 152590 [details] [review]
Add gjs_context_get_native_context which allows access to the JSContext

Also remove gjs_context_get_context() since it is obsoleted

Review of attachment 152590 [details] [review]:

This looks good to me.

James' commit is in master since Feb 23, close bug?

commit bcbe62340bb30d755c66479e0a3c0a7103b59aa4
Author:     James Willcox <jwillcox@litl.com>
AuthorDate: Fri Jan 29 10:04:21 2010 -0500
Commit:     James Willcox <jwillcox@litl.com>
CommitDate: Tue Feb 23 11:16:03 2010 -0500

    Add gjs_context_get_native_context which allows access to the JSContext
    
    Also remove gjs_context_get_context() since it is obsoleted
    
    https://bugzilla.gnome.org/show_bug.cgi?id=604075

Yup, closing.