Created attachment 201813 [details] [review]
gsettings: Add API so that extensions can use gsettings without crashing

Review of attachment 201813 [details] [review]:

Overall, pretty nice, and would surely help!

::: js/ui/extensionSystem.js
@@ +300,3 @@
+function _compileSchema(dir) {
+    let argv = ['glib-compile-schemas', '--targetdir', dir.get_path(), dir.get_path()];
+    GLib.spawn_sync(null, argv, null, GLib.SpawnFlags.SEARCH_PATH, null, null);

You should probably catch exceptions here.

@@ +307,3 @@
+        let schema = meta.schemaSource.lookup(schemaId, true);
+        if (!schema) {
+            throw new Error(meta.uuid, 'Settings schema \'%s\' cannot be found'.format(schemaId));

Error constructor accepts only one argument. In any case, meta.uuid would be inferred by the try/catch handling the exception and logging the error.

@@ +413,3 @@
+        _compileSchema(dir);
+
+    let parentSchemaSource = Gio.SettingsSchemaSource.get_default();

Unused variable

Review of attachment 201813 [details] [review]:

Overall, pretty nice, and would surely help!

::: js/ui/extensionSystem.js
@@ +300,3 @@
+function _compileSchema(dir) {
+    let argv = ['glib-compile-schemas', '--targetdir', dir.get_path(), dir.get_path()];
+    GLib.spawn_sync(null, argv, null, GLib.SpawnFlags.SEARCH_PATH, null, null);

You should probably catch exceptions here.

@@ +307,3 @@
+        let schema = meta.schemaSource.lookup(schemaId, true);
+        if (!schema) {
+            throw new Error(meta.uuid, 'Settings schema \'%s\' cannot be found'.format(schemaId));

Error constructor accepts only one argument. In any case, meta.uuid would be inferred by the try/catch handling the exception and logging the error.

@@ +413,3 @@
+        _compileSchema(dir);
+
+    let parentSchemaSource = Gio.SettingsSchemaSource.get_default();

Unused variable

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

(In reply to comment #3)
> Review of attachment 201813 [details] [review]:
> 
> Overall, pretty nice, and would surely help!
> 
> ::: js/ui/extensionSystem.js
> @@ +300,3 @@
> +function _compileSchema(dir) {
> +    let argv = ['glib-compile-schemas', '--targetdir', dir.get_path(),
> dir.get_path()];
> +    GLib.spawn_sync(null, argv, null, GLib.SpawnFlags.SEARCH_PATH, null,
> null);
> 
> You should probably catch exceptions here.

What exceptions could there be?

Is this still relevant (considering the new extension preferences stuff)?

In any case, exceptions could arise from not finding the executable in path or not being able to run it. Both are unlikely cases, but worth at least a warning anyway.

(In reply to comment #6)
> Is this still relevant (considering the new extension preferences stuff)?

It probably is. We could just have people copy your convenience.js, but that isn't very friendly. I'll attach a new patch rebased on top of the (yet unlanded) prefs stuff.

> In any case, exceptions could arise from not finding the executable in path or
> not being able to run it. Both are unlikely cases, but worth at least a warning
> anyway.

If glib-compile-schemas isn't in PATH, we really should be crashing.

Not relevant any more.