Split from #3000. Adds double key type to stash settings.
Example usage (`stash-example.c`): ```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 ``` You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/3004
-- Commit Summary --
* Stash Settings: Add double key type
-- File Changes --
M doc/stash-example.c (12) M src/plugindata.h (2) M src/stash.c (35) M src/stash.h (3) M src/utils.c (55) M src/utils.h (2)
-- Patch Links --
https://github.com/geany/geany/pull/3004.patch https://github.com/geany/geany/pull/3004.diff
Do you have a compelling use case for this change?
I have a plugin that uses the double type. Without this change, I can't use the stash system with that plugin.
@kugel- requested changes on this pull request.
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)?
@@ -795,11 +795,11 @@ gchar *utils_get_initials(const gchar *name)
* @param config A GKeyFile object. * @param section The group name to look in for the key. * @param key The key to find. - * @param default_value The default value which will be returned when @a section or @a key - * don't exist. + * @param default_value The default value which will be returned when @a section + * or @a key don't exist.
No unrelated changes, please.
@@ -476,6 +497,20 @@ void stash_group_add_boolean(StashGroup *group, gboolean *setting,
}
+/** Adds double setting. + * @param group . + * @param setting Address of setting variable. + * @param key_name Name for key in a @c GKeyFile. + * @param default_value Value to use if the key doesn't exist when loading. */ +GEANY_API_SYMBOL +void stash_group_add_double(StashGroup *group, gdouble *setting, + const gchar *key_name, gdouble default_value) +{ + gulong *default_long = (gulong*) &default_value; + add_pref(group, G_TYPE_DOUBLE, setting, key_name, (gpointer) *default_long);
add_pref should use a union. I'll prepare a change, then you can rebase this.
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 commented on this pull request.
@@ -795,11 +795,11 @@ gchar *utils_get_initials(const gchar *name)
* @param config A GKeyFile object. * @param section The group name to look in for the key. * @param key The key to find. - * @param default_value The default value which will be returned when @a section or @a key - * don't exist. + * @param default_value The default value which will be returned when @a section + * or @a key don't exist.
Ok. Will revert this when rebasing on the union code you will provide.
@xiota commented on this pull request.
@@ -476,6 +497,20 @@ void stash_group_add_boolean(StashGroup *group, gboolean *setting,
}
+/** Adds double setting. + * @param group . + * @param setting Address of setting variable. + * @param key_name Name for key in a @c GKeyFile. + * @param default_value Value to use if the key doesn't exist when loading. */ +GEANY_API_SYMBOL +void stash_group_add_double(StashGroup *group, gdouble *setting, + const gchar *key_name, gdouble default_value) +{ + gulong *default_long = (gulong*) &default_value; + add_pref(group, G_TYPE_DOUBLE, setting, key_name, (gpointer) *default_long);
Will rebase when it's available. Thank you.
@xiota pushed 1 commit.
520c126d3b8c757f28ea5cc3c3077f30ecdf50d7 Revert comment changes; use gint64 instead of gulong
@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`: ```C typedef union { gboolean b; gint n; gdouble d; gchar *s; gchar **v; const gchar *cs; const gchar **cv; } StashPrefType; ```
The StashPref struct becomes: ```C struct StashPref { /* ... */ StashPrefType *setting; StashPrefType default_value; /* ... */ }; ```
Some functions need to be redefined: ```C 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: ```C int *setting = &se->setting->n; double *setting = &se->setting->d; ```
Then calls to the above functions need to have casts to `StashPrefType`, like: ```C 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.
See #3005, I said I prepare a change. Why did you implement the union as well?
Why did you implement the union as well?
Wanted to see what the change looks like... and impatient. 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.
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 pushed 2 commits.
3672af2e84134763b8b478479c68605541cd7307 Use union to store StashPref default values 87d93d582be3e2e48a97949a92f696694aad850c Update for union
@kugel- Adding the double with your changes definitely looks cleaner.
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.
@elextr Code to learn from.
@xiota pushed 3 commits.
5fb281d42313b3ca60cb848e8a36517e5ddc50a1 Stash Settings: Add double key type 7defa67623ebfc5eb71b7eb77182c40ef3dfbb60 Revert comment changes; use gint64 instead of gulong 547b818ba2b8b4f357606d10a7fb7f484c1e8b83 Update for union; update comment
@kugel-
Force push to rebase and change PR for the union. Please let me know if this needs any more changes.
Merged #3004 into master.
github-comments@lists.geany.org