Created attachment 209525 [details] [review]
Added peas_plugin_info_get_external_data

I like the idea but I'm not very fond of keeping all the GKeyFiles around just for X- key usages as I'm pretty sure not many people will make use of that feature.

Probably the solution would be to have a hashtable. But is it worth? Is it so expensive keeping the KeyFile?

I also don't see the problem keeping the key file around. It's not going to be big, and anyway you don't have thousands of plugins. The other thing is a hash table, but I found it lame that I had to iterate again over all the keys to match each key and construct the hash. Seemed easier (and more efficient) to keep the key file around.

Well, wouldn't it be possible to just keep the keyfile only if there is at least one X- key?

The thing is, if we keep the keyfile around, then there is no need to populate the PluginInfo struct anyhow, we could just fetch the values straight from the keyfile, especially for string-only values like copyright, help uri and so on which are only ever going to be used for about boxes anyway.

I honnestly don't know how gkeyfile stores its data and how big it is in memory.

I don't really care about the implementation so much to be honest. I can just implement whatever you prefer. You would like a hashtable?

Ping

Created attachment 210056 [details] [review]
Added peas_plugin_info_get_external_data

Created attachment 210057 [details] [review]
Added peas_plugin_info_get_external_data

Latest implements external data with a hash table and adds a simple test for external data to the plugin-info test.

Review of attachment 210057 [details] [review]:

Patch looks good to me. See the minor comment though.

::: libpeas/peas-plugin-info.c
@@ +97,3 @@
 
+  if (info->external_data != NULL)
+    g_hash_table_destroy (info->external_data);

use unref instead of destroy

Review of attachment 210057 [details] [review]:

::: libpeas/peas-plugin-info.c
@@ +260,3 @@
+  keys = g_key_file_get_keys (plugin_file, "Plugin", NULL, NULL);
+
+  for (keysptr = keys; keysptr && *keysptr; ++keysptr)

keysptr will never be NULL

@@ +262,3 @@
+  for (keysptr = keys; keysptr && *keysptr; ++keysptr)
+    {
+      if (g_str_has_prefix (*keysptr, "X-"))

Use a continue to avoid the large indent

@@ +268,3 @@
+                                                         g_str_equal,
+                                                         (GDestroyNotify)g_free,
+      if (g_str_has_prefix (*keysptr, "X-"))

Space after casts

@@ +279,3 @@
+    }
+
+                                                         (GDestroyNotify)g_free);

keys will never be NULL

@@ +745,3 @@
+/**
+ * peas_plugin_info_get_external_data:
+ * @info: a #PeasPluginInfo

"A #PeasPluginInfo."

@@ +746,3 @@
+ * peas_plugin_info_get_external_data:
+ * @info: a #PeasPluginInfo
+ * @key: a key

"The key to lookup."

@@ +751,3 @@
+ *
+ * External info is specified in the plugin info file prefixed with X-. For
+ * @key: a key

X-Peas = 1 is invalid GKeyFile syntax, use X-Peas=1 instead

@@ +756,3 @@
+ * in the file.
+ *
+ * Gets external info specified for the plugin.

Use external data in comment

::: tests/libpeas/plugin-info.c
@@ +92,3 @@
   g_assert_cmpstr (authors[0], ==, "Garrett Regier");
+
+  g_assert_cmpstr (peas_plugin_info_get_external_data (info, "External"), ==, "external data");

Also test X-External

Created attachment 210396 [details] [review]
external data

There it goes with the comments fixed. If it is ok please push it and make a new release :) We are already suppose to be in hardcode freeze

Thanks for picking this up Ignacio, looks good to me.

Created attachment 210399 [details] [review]
Added peas_plugin_info_get_external_data()

External info was still mentioned multiple times instead of external data.

Review of attachment 210399 [details] [review]:

Just one comment, feel free to push

::: libpeas/peas-plugin-info.c
@@ +257,3 @@
     info->hidden = b;
 
+  strv = g_key_file_get_keys (plugin_file, "Plugin", NULL, NULL);

why strv? IMHO it is more understandable keys...

This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.