#19 Sorts keys before saving options.xml

[Damien Cassou]

This patch proposes to sort the keys before saving to avoid changing the configuration file if only the order changes. This is because I version control my configuration files and currently, each time I quit ario, I get a different configuration file even if nothing changed ; just because the order of keys changed in the hash table.

This patch fixes the problem.

[Damien Cassou]

Patch to sort keys before saving

[Marc Pavot]

Hello,

The content of your patch seems to be incorrect (HTML garbage) or sourceforge has a problem with it. Can you please check your patch or re-upload it?

Thanks
Marc

[Damien Cassou]

If I click on 'download' in the list of attached files, I get a valid patch file as far as I can tell. Here it is in case it still does not work for you:

Index: ario-conf.c

--- ario-conf.c (revision 770)
+++ ario-conf.c (working copy)
@@ -243,6 +243,15 @@
xmlNodeAddContent (cur, (const xmlChar *) value);
}

+static gint
+ario_conf_compare_keys (gconstpointer a,
+ gconstpointer b)
+{
+ const xmlChar * str1 = (const xmlChar *) a;
+ const xmlChar * str2 = (const xmlChar *) b;
+ return xmlStrcmp (str1, str2);
+}
+
static gboolean
ario_conf_save (G_GNUC_UNUSED gpointer data)
{
@@ -250,6 +259,8 @@
xmlNodePtr cur;
xmlDocPtr doc;
char *xml_filename;
+ guint index;
+ GList* sorted_keys;

if (!modified)
return TRUE;
@@ -259,10 +270,17 @@
cur = xmlNewNode (NULL, (const xmlChar *) XML_ROOT_NAME);
xmlDocSetRootElement (doc, cur);

- g_hash_table_foreach (hash,
- (GHFunc) ario_conf_save_foreach,
- cur);
+ /* We sort the keys before saving to avoid changing the
+ configuration file if only the order changes */
+ sorted_keys = g_list_sort (g_hash_table_get_keys (hash),
+ ario_conf_compare_keys);

+ for (index = 0; index < g_list_length (sorted_keys); ++index) {
+ gpointer key = g_list_nth_data (sorted_keys, index);
+ gpointer value = g_hash_table_lookup (hash, key);
+ ario_conf_save_foreach ((gchar *) key, (gchar *) value, cur);
+ }
+
xml_filename = g_build_filename (ario_util_config_dir (), "options.xml", NULL);

/* We save the xml file */

[Marc Pavot]

Sorry for the very late reply...

There are some issues with this patch:
- Memory leak: value retruned by g_hash_table_get_keys must be freed
- The loop on the list of keys + retrieve of value in the hash table is not very efficient. Best way would be to go throught the hash table only once and to build a sorted list of key/values couples.

Can you please change that?

Thanks
Marc

[Damien Cassou]

--- ario-conf-orig.c 2012-04-18 13:53:29.532028246 +0200
+++ ario-conf.c 2012-04-18 13:51:33.716029547 +0200
@@ -229,7 +229,7 @@
return ret;
}

-static void
+static gboolean
ario_conf_save_foreach (gchar *key,
gchar *value,
xmlNodePtr root)
@@ -241,6 +241,24 @@
cur = xmlNewChild (root, NULL, (const xmlChar *) "option", NULL);
xmlSetProp (cur, (const xmlChar *) "key", (const xmlChar *) key);
xmlNodeAddContent (cur, (const xmlChar *) value);
+ return FALSE;
+}
+
+static gint
+ario_conf_compare_keys (gconstpointer a,
+ gconstpointer b)
+{
+ const xmlChar * str1 = (const xmlChar *) a;
+ const xmlChar * str2 = (const xmlChar *) b;
+ return xmlStrcmp (str1, str2);
+}
+
+static void
+ario_conf_sorted_save_foreach (gchar *key,
+ gchar *value,
+ GTree *sorted_pairs)
+{
+ g_tree_insert(sorted_pairs, key, value);
}

static gboolean
@@ -250,6 +268,7 @@
xmlNodePtr cur;
xmlDocPtr doc;
char *xml_filename;
+ GTree *sorted_pairs;

if (!modified)
return TRUE;
@@ -259,9 +278,16 @@
cur = xmlNewNode (NULL, (const xmlChar *) XML_ROOT_NAME);
xmlDocSetRootElement (doc, cur);

- g_hash_table_foreach (hash,
- (GHFunc) ario_conf_save_foreach,
- cur);
+ /* We sort the keys before saving to avoid changing the
+ configuration file if only the order changes */
+ sorted_pairs=g_tree_new(ario_conf_compare_keys);
+ g_hash_table_foreach (hash,
+ (GHFunc) ario_conf_sorted_save_foreach,
+ sorted_pairs);
+ g_tree_foreach(sorted_pairs,
+ (GTraverseFunc) ario_conf_save_foreach,
+ cur);
+ g_tree_destroy(sorted_pairs);

xml_filename = g_build_filename (ario_util_config_dir (), "options.xml", NULL);

[Marc Pavot]

Pushed to Ario svn.

Thank you
Marc