Stash Settings: Add double key type

[xiota]

Split from #3000. Adds double key type to stash settings.

Example usage (stash-example.c):

StashGroup *group;
gboolean porcelain_enabled;
gchar *potter_name;
gint stock;
gdouble price;
const gchar filename[] = "/path/data.conf";

/* setup the group */
group = stash_group_new("cup");
stash_group_add_boolean(group, &porcelain_enabled, "porcelain", TRUE);
stash_group_add_string(group, &potter_name, "potter_name", "Miss Clay");
stash_group_add_integer(group, &stock, "stock", 5);
stash_group_add_double(group, &price, "price", 1.50);

/* load the settings from a file */
if (!stash_group_load_from_file(group, filename))
	g_warning(_("Could not load keyfile %s!"), filename);

/* now use settings porcelain_enabled, potter_name, stock, and price */
/* ... */

/* save settings to file */
if (stash_group_save_to_file(group, filename, G_KEY_FILE_NONE) != 0)
	g_error(_("Could not save keyfile %s!"), filename);

/* free memory */
stash_group_free(group);

Generated config:

[cup]
porcelain=true
potter_name=Miss Clay
stock=5
price=1.5

[kugel-]

Do you have a compelling use case for this change?

[xiota]

@kugel- I have a plugin that uses the double type. Without this change, I can't use the stash system with that plugin. If I can't even use the stash system, there's little point making any further changes to it.

After this change, the next change is to add key comments. The reason is the same. I use them in a plugin. Without the change, the stash system is unusable.

If you don't need doubles and comments, never need them in the future, and have no interest in the upcoming PRs to add lookup/override/session features in #3000, there is no compelling reason to add this.

[kugel-]

What does your plugin do? I can understand the desire for doubles but comments (given that keyfiles are typically not meant to be looked at by users)?

[xiota]

given that keyfiles are typically not meant to be looked at by users

Some keyfiles are intended to be read and modified by users. Including comments from the source makes them translatable through the gettext system. If someone later wastes time wrapping the config with a GUI, the same comments can be repurposed as explanatory tooltips.

[xiota]

@kugel-

I don't understand the benefit of using union over casting the pointers. Based on what I'm reading online, they're both considered "bad". It also looks like switching to using unions may require changing existing code, which I've been trying to avoid.

I'm probably doing this wrong... I tried adding the following union to stash.h:

typedef union
{
	gboolean b;
	gint n;
	gdouble d;
	gchar *s;
	gchar **v;
	const gchar *cs;
	const gchar **cv;
} StashPrefType;

The StashPref struct becomes:

struct StashPref
{
	/* ... */
	StashPrefType *setting;
	StashPrefType default_value;
	/* ... */
};

Some functions need to be redefined:

add_pref(StashGroup *group, GType type, StashPrefType *setting,
		const gchar *key_name, StashPrefType default_value);

static StashPref *
add_widget_pref(StashGroup *group, GType setting_type, StashPrefType *setting,
		const gchar *key_name, StashPrefType default_value,
		GType widget_type, StashWidgetID widget_id);

void stash_group_add_widget_property(StashGroup *group, StashPrefType *setting,
		const gchar *key_name, StashPrefType default_value, StashWidgetID widget_id,
		const gchar *property_name, GType type);

To use StashPrefType requires something like:

int *setting = &se->setting->n;
double *setting = &se->setting->d;

Then calls to the above functions need to have casts to StashPrefType, like:

add_pref(group, G_TYPE_BOOLEAN, (StashPrefType *) setting, key_name, (StashPrefType)default_value);

And any external uses of stash_group_add_widget_property() (sidebar.c and maybe plugins) also need to cast StashPrefType.

[kugel-]

See #3005, I said I prepare a change. Why did you implement the union as well?

[xiota]

Why did you implement the union as well?

Wanted to see what the change looks like... and impatient (didn't properly check for a new PR). I also don't understand the use of union, so comparing implementations (or getting your feedback on what I'm doing wrong) may help me understand them better.

[kugel-]

OK. So I did the change in a way that the union is only internally used and not exposed to users of the stash system.

Users call type-specific functions already, which is fine. stash_group_add_widget_property() is an exception because widget properties can be anything as well. I did not want to change that function yet. One could go and make a switch/case on the type in order to fill the union properly (again, the function signature doesn't need changing and it would be hard to do as the API is already exported).

[xiota]

@kugel- Adding the double with your changes definitely looks cleaner.

[elextr]

Just an aside

Based on what I'm reading online, they're both considered "bad".

Indeed, but occasionally essential. The way @kugel- did it in #3005 which encapsulated the badness, away from people using the library, is the right way.

[xiota]

@elextr Code to learn from.

[xiota]

@kugel-

Force push to rebase and change PR for the union. Please let me know if this needs any more changes.