Created attachment 290904 [details] [review]
This patch adds the g_settings_schema_list_keys() method to GSettingsSchema

Review of attachment 290904 [details] [review]:

::: gio/gsettingsschema.c
@@ +1016,3 @@
+ * Since: 2.44
+ **/
+gchar**

coding style: missing space between `gchar` and `**`.

@@ +1027,3 @@
+  table_list = gvdb_table_list (schema->table, "");
+
+  keys = g_array_new (TRUE, FALSE, sizeof(gchar*));

coding style: missing space between `sizeof` and `(`.

also: why are you using a GArray? I would use a GPtrArray instead.

@@ +1030,3 @@
+  g_array_insert_vals (keys, 0, table_list, g_strv_length (table_list));
+
+  for (i = 0; i < g_strv_length(table_list); i++)

don't use `g_strv_length()` inside a `for` loop: you're iterating twice on the strings array.

@@ +1031,3 @@
+
+  for (i = 0; i < g_strv_length(table_list); i++)
+  {

coding style: insufficient indentation.

@@ +1036,3 @@
+    /* Discard all the subdirs */
+    if (g_str_has_suffix(key, "/"))
+    {

coding style: insufficient indentation.

@@ +1038,3 @@
+    {
+      g_free (key);
+      g_array_remove_index (keys, i - offset);

this is a bit meh. you're adding all keys and then removing stuff. wouldn't it be better to simply iterate over each key in the `table_list` and only adding the wanted keys?

@@ +1045,3 @@
+  g_free (table_list);
+
+  return (gchar**)g_array_free (keys, FALSE);

coding style: insufficient indentation.

::: gio/gsettingsschema.h
@@ +76,3 @@
                                                                          const gchar            *name);
+GLIB_AVAILABLE_IN_2_44
+gchar**                 g_settings_schema_list_keys                     (GSettingsSchema *schema);

coding style: alignment of the argument.

Created attachment 290923 [details] [review]
This patch adds the g_settings_schema_list_keys() method to GSettingsSchema

Agreed with all the points, please review the current changes.

Created attachment 290975 [details] [review]
This patch adds the g_settings_schema_list_keys() method to GSettingsSchema and corresponding test

This patch adds a test case to test the method behaviour

Created attachment 291012 [details] [review]
This patch adds the g_settings_schema_list_keys() method to GSettingsSchema and corresponding test

This last revision fixes a style problem and avoids listing keys used for list of tables for relocatable schemas (those using the ":" suffix convention).

Created attachment 291017 [details] [review]
GSettingsSchema: add g_settings_schema_list_keys()

The list of keys in a GSettings object depends entirely on the schema,
so it makes sense to expose this API there.

Move the implementation out of gsettings.c and into gsettingsschema.c,
replacing the earlier with a simple call to the new location.

We don't do the same for children because the children can change.

Created attachment 291018 [details] [review]
gsettings tool: use schema for listing keys

Use the newly added g_settings_schema_list_keys() API instead of
g_settings_list_keys() in order to list keys.

Doing this allows the 'list-keys' command to work without creating a
GSettings object, which is more efficient.  It also means that we don't
have to provide a (meaningless and ignored) path when listing keys on
relocatable schemas.

While we're at it, update the 'range' command not to require creation of
a GSettings object, in a similar way.

Created attachment 291019 [details] [review]
gsettings tests: use g_settings_schema_list_keys()

Stop using g_settings_list_keys() because soon it will be deprecated.

Created attachment 291020 [details] [review]
GSettings: deprecate g_settings_list_keys()

This is now possible with g_settings_schema_list_keys().

Review of attachment 291017 [details] [review]:

::: gio/gsettingsschema.c
@@ +1014,3 @@
+ * You should probably not be calling this function from "normal" code
+ * (since you should already know what keys are in your schema).  This
+ * function is intended for introspection reasons.

not a native speaker but "intended for reasons" sounds odd to me.
"Added for introspection reasons" or "intended for introspection", maybe ?

@@ +1016,3 @@
+ * function is intended for introspection reasons.
+ *
+ * Returns: (transfer full) (element-type utf8): a list of the keys on

more language lawyering: "keys in @schema" instead of "keys on @schema" ?

::: gio/gsettingsschema.h
@@ +76,3 @@
                                                                          const gchar            *name);
+GLIB_AVAILABLE_IN_2_44
+gchar**                 g_settings_schema_list_keys                     (GSettingsSchema        *schema);

Emmanuele asked for a space between gchar and ** here, I think ?

Review of attachment 291018 [details] [review]:

Looks good

Review of attachment 291019 [details] [review]:

Good to add a test for the new function. Not entirely sure I agree with not testing deprecated functions.

Review of attachment 291020 [details] [review]:

ok

Does this mean that the patch is good to go?

Attachment 291017 [details] pushed as 82fcfeb - GSettingsSchema: add g_settings_schema_list_keys()
Attachment 291018 [details] pushed as bb8eea6 - gsettings tool: use schema for listing keys
Attachment 291019 [details] pushed as 6cf867f - gsettings tests: use g_settings_schema_list_keys()
Attachment 291020 [details] pushed as cb7020a - GSettings: deprecate g_settings_list_keys()

Shouldn't the deprecation be documented?