From 7ff8ef94c05ae6386463b63f3ba25add52340d8b Mon Sep 17 00:00:00 2001 From: Vito Caputo Date: Sat, 12 Mar 2022 17:29:26 -0800 Subject: til_settings: always describe relevant settings The existing iterative *_setup() interface only described settings not found, quietly accepting usable settings already present in the til_settings_t. This worked fine for the existing interactive text setup thing, but it's especially problematic for providing a GUI setup frontend. This commit makes it so the *_setup() methods always describe undescribed settings they recognize, leaving the setup frontend loop calling into the *_setup() methods to both apply the description validation if wanted and actually tie the description to respective setting returned by the _setup() methods as being related to the returned description. A new helper called til_settings_get_and_describe_value() has been introduced primarily for use of module setup methods to simplify this nonsense, replacing the til_settings_get_value() calls and surrounding logic, but retaining the til_setting_desc_t definitions largely verbatim. This also results in discarding of some ad-hoc til_setting_desc_check() calls, now that there's a centralized place where settings become "described" (setup_interactively in the case of rototiller). Now a GUI frontend (like glimmer) would just provide its own setup_interactively() equivalent for constructing its widgets for a given *_setup() method's chain of returned descs. Whereas in the past this wasn't really feasible unless there was never going to be pre-supplied settings. I suspect the til_setting_desc_check() integration into setup_interactively() needs more work, but I think this is good enough for now and I'm out of spare time for the moment. --- src/til_settings.c | 136 +++++++++++++++++++++++++++++++++-------------------- 1 file changed, 85 insertions(+), 51 deletions(-) (limited to 'src/til_settings.c') diff --git a/src/til_settings.c b/src/til_settings.c index e3c8c6c..30156d6 100644 --- a/src/til_settings.c +++ b/src/til_settings.c @@ -31,8 +31,7 @@ char * strndup(const char *s, size_t n) /* Split form of key=value[,key=value...] settings string */ typedef struct til_settings_t { unsigned num; - const char **keys; - const char **values; + til_setting_t **settings; } til_settings_t; typedef enum til_settings_fsm_state_t { @@ -43,16 +42,17 @@ typedef enum til_settings_fsm_state_t { } til_settings_fsm_state_t; -static int add_value(til_settings_t *settings, const char *key, const char *value) +static int add_value(til_settings_t *settings, const char *key, const char *value, const til_setting_desc_t *desc) { assert(settings); settings->num++; /* TODO errors */ - settings->keys = realloc(settings->keys, settings->num * sizeof(const char **)); - settings->values = realloc(settings->values, settings->num * sizeof(const char **)); - settings->keys[settings->num - 1] = key; - settings->values[settings->num - 1] = value; + settings->settings = realloc(settings->settings, settings->num * sizeof(til_setting_t *)); + settings->settings[settings->num - 1] = malloc(sizeof(til_setting_t)); + settings->settings[settings->num - 1]->key = key; + settings->settings[settings->num - 1]->value = value; + settings->settings[settings->num - 1]->desc = desc; return 0; } @@ -83,7 +83,7 @@ til_settings_t * til_settings_new(const char *settings_string) case TIL_SETTINGS_FSM_STATE_KEY: if (*p == '=' || *p == ',' || *p == '\0') { - add_value(settings, strndup(token, p - token), NULL); + add_value(settings, strndup(token, p - token), NULL, NULL); if (*p == '=') state = TIL_SETTINGS_FSM_STATE_EQUAL; @@ -99,7 +99,7 @@ til_settings_t * til_settings_new(const char *settings_string) case TIL_SETTINGS_FSM_STATE_VALUE: if (*p == ',' || *p == '\0') { - settings->values[settings->num - 1] = strndup(token, p - token); + settings->settings[settings->num - 1]->value = strndup(token, p - token); state = TIL_SETTINGS_FSM_STATE_COMMA; } break; @@ -124,12 +124,13 @@ til_settings_t * til_settings_free(til_settings_t *settings) if (settings) { for (unsigned i = 0; i < settings->num; i++) { - free((void *)settings->keys[i]); - free((void *)settings->values[i]); + free((void *)settings->settings[i]->key); + free((void *)settings->settings[i]->value); + til_setting_desc_free((void *)settings->settings[i]->desc); + free((void *)settings->settings[i]); } - free((void *)settings->keys); - free((void *)settings->values); + free((void *)settings->settings); free(settings); } @@ -138,14 +139,18 @@ til_settings_t * til_settings_free(til_settings_t *settings) /* find key= in settings, return dup of value side or NULL if missing */ -const char * til_settings_get_value(const til_settings_t *settings, const char *key) +const char * til_settings_get_value(const til_settings_t *settings, const char *key, const til_setting_t **res_setting) { assert(settings); assert(key); for (int i = 0; i < settings->num; i++) { - if (!strcmp(key, settings->keys[i])) - return settings->values[i]; + if (!strcmp(key, settings->settings[i]->key)) { + if (res_setting) + *res_setting = settings->settings[i]; + + return settings->settings[i]->value; + } } return NULL; @@ -153,27 +158,71 @@ const char * til_settings_get_value(const til_settings_t *settings, const char * /* return positional key from settings */ -const char * til_settings_get_key(const til_settings_t *settings, unsigned pos) +const char * til_settings_get_key(const til_settings_t *settings, unsigned pos, const til_setting_t **res_setting) { assert(settings); - if (pos < settings->num) - return settings->keys[pos]; + if (pos < settings->num) { + if (res_setting) + *res_setting = settings->settings[pos]; + + return settings->settings[pos]->key; + } return NULL; } +/* helper for the common setup case of describing a setting when absent or not yet described. + * returns: + * -1 on error, res_* will be untouched in this case. + * 0 when setting is present and described, res_value and res_setting will be populated w/non-NULL, and res_desc NULL in this case. + * 1 when setting is either present but undescribed, or absent (and undescribed), res_* will be populated but res_{value,setting} may be NULL if absent and simply described. + */ +int til_settings_get_and_describe_value(const til_settings_t *settings, const til_setting_desc_t *desc, const char **res_value, const til_setting_t **res_setting, const til_setting_desc_t **res_desc) +{ + const til_setting_t *setting; + const char *value; + + assert(settings); + assert(desc); + assert(res_value); + assert(res_setting); + assert(res_desc); + + value = til_settings_get_value(settings, desc->key, &setting); + if (!value || !setting->desc) { + int r; + + r = til_setting_desc_clone(desc, res_desc); + if (r < 0) + return r; + + *res_value = value; + *res_setting = value ? setting : NULL; + + return 1; + } + + *res_value = value; + *res_setting = setting; + *res_desc = NULL; + + return 0; +} + + /* add key=value to the settings, * or just key if value is NULL. + * desc may be NULL, it's simply passed along as a passenger. */ /* returns < 0 on error */ -int til_settings_add_value(til_settings_t *settings, const char *key, const char *value) +int til_settings_add_value(til_settings_t *settings, const char *key, const char *value, const til_setting_desc_t *desc) { assert(settings); assert(key); - return add_value(settings, strdup(key), value ? strdup(value) : NULL); + return add_value(settings, strdup(key), value ? strdup(value) : NULL, desc); } @@ -181,43 +230,27 @@ int til_settings_add_value(til_settings_t *settings, const char *key, const char /* returns 0 when input settings are complete */ /* returns 1 when input settings are incomplete, storing the next setting's description needed in *next_setting */ /* returns -errno on error */ -int til_settings_apply_desc_generators(const til_settings_t *settings, const til_setting_desc_generator_t generators[], unsigned n_generators, void *setup_context, til_setting_desc_t **next_setting) +int til_settings_apply_desc_generators(const til_settings_t *settings, const til_setting_desc_generator_t generators[], unsigned n_generators, void *setup_context, const til_setting_t **res_setting, const til_setting_desc_t **res_desc) { - til_setting_desc_t *next; - assert(settings); assert(generators); assert(n_generators > 0); - assert(next_setting); + assert(res_setting); + assert(res_desc); for (unsigned i = 0; i < n_generators; i++) { const til_setting_desc_generator_t *g = &generators[i]; - const char *value; - til_setting_desc_t *desc; + const til_setting_desc_t *desc; int r; r = g->func(setup_context, &desc); if (r < 0) return r; - assert(desc); - - value = til_settings_get_value(settings, g->key); - if (value) { - int r; - - r = til_setting_desc_check(desc, value); - til_setting_desc_free(desc); - if (r < 0) - return r; - - if (g->value_ptr) - *g->value_ptr = value; - - continue; - } - - *next_setting = desc; + r = til_settings_get_and_describe_value(settings, desc, g->value_ptr, res_setting, res_desc); + til_setting_desc_free(desc); /* always need to cleanup the desc from g->func(), res_desc gets its own copy */ + if (r) + return r; return 1; } @@ -229,7 +262,7 @@ int til_settings_apply_desc_generators(const til_settings_t *settings, const til /* convenience helper for creating a new setting description */ /* copies of everything supplied are made in newly allocated memory, stored @ res_desc */ /* returns < 0 on error */ -int til_setting_desc_clone(const til_setting_desc_t *desc, til_setting_desc_t **res_desc) +int til_setting_desc_clone(const til_setting_desc_t *desc, const til_setting_desc_t **res_desc) { til_setting_desc_t *d; @@ -280,7 +313,7 @@ int til_setting_desc_clone(const til_setting_desc_t *desc, til_setting_desc_t ** } -til_setting_desc_t * til_setting_desc_free(til_setting_desc_t *desc) +til_setting_desc_t * til_setting_desc_free(const til_setting_desc_t *desc) { if (desc) { free((void *)desc->name); @@ -300,7 +333,7 @@ til_setting_desc_t * til_setting_desc_free(til_setting_desc_t *desc) free((void *)desc->annotations); } - free(desc); + free((void *)desc); } return NULL; @@ -310,6 +343,7 @@ til_setting_desc_t * til_setting_desc_free(til_setting_desc_t *desc) int til_setting_desc_check(const til_setting_desc_t *desc, const char *value) { assert(desc); + assert(value); if (desc->values) { @@ -366,10 +400,10 @@ char * til_settings_as_arg(const til_settings_t *settings) if (i > 0) off += snpf(buf, size, off, ","); - off += snpf(buf, size, off, "%s", settings->keys[i]); + off += snpf(buf, size, off, "%s", settings->settings[i]->key); - if (settings->values[i]) - off += snpf(buf, size, off, "=%s", settings->values[i]); + if (settings->settings[i]->value) + off += snpf(buf, size, off, "=%s", settings->settings[i]->value); } if (!buf) { -- cgit v1.2.1