From 7caf57f0131bc963a100dd1d6a5aa738fb334c3c Mon Sep 17 00:00:00 2001 From: Pierre Le Marre Date: Thu, 16 Nov 2023 09:29:31 +0100 Subject: [PATCH] =?UTF-8?q?registry:=20Parse=20=E2=80=9Cpopularity?= =?UTF-8?q?=E2=80=9D=20attribute?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously the attribute “popularity” was completely ignored. It also did not respect the modified DTD, because its default value depends if we are currently parsing an “extras” rules file. Fixed: - Always parse the popularity attribute. - Change the DTD to reflect that the default value is implied. --- src/registry.c | 171 ++++++++++++++++++++++++++---------------------- test/registry.c | 127 +++++++++++++++++++++++++---------- 2 files changed, 188 insertions(+), 110 deletions(-) diff --git a/src/registry.c b/src/registry.c index d6181e14b..cd1560e7e 100644 --- a/src/registry.c +++ b/src/registry.c @@ -708,44 +708,73 @@ extract_text(xmlNode *node) return NULL; } +/* Data from “configItem” node */ +struct config_item { + char *name; + char *description; + char *brief; + char *vendor; + enum rxkb_popularity popularity; +}; + +#define config_item_new(popularity_) { \ + .name = NULL, \ + .description = NULL, \ + .brief = NULL, \ + .vendor = NULL, \ + .popularity = popularity_ \ +} + +static void +config_item_free(struct config_item *config) { + free(config->name); + free(config->description); + free(config->brief); + free(config->vendor); +} + static bool -parse_config_item(struct rxkb_context *ctx, - xmlNode *parent, - char **name, - char **description, - char **brief, - char **vendor) +parse_config_item(struct rxkb_context *ctx, xmlNode *parent, + struct config_item *config) { xmlNode *node = NULL; xmlNode *ci = NULL; for (ci = parent->children; ci; ci = ci->next) { if (is_node(ci, "configItem")) { - *name = NULL; - *description = NULL; - *brief = NULL; - *vendor = NULL; + /* Process attributes */ + xmlChar *raw_popularity = xmlGetProp(ci, (const xmlChar*)"popularity"); + if (raw_popularity) { + if (xmlStrEqual(raw_popularity, (const xmlChar*)"standard")) + config->popularity = RXKB_POPULARITY_STANDARD; + else if (xmlStrEqual(raw_popularity, (const xmlChar*)"exotic")) + config->popularity = RXKB_POPULARITY_EXOTIC; + else + log_err(ctx, + "xml:%d: invalid popularity attribute: expected " + "'standard' or 'exotic', got: '%s'\n", + ci->line, raw_popularity); + } + xmlFree(raw_popularity); + /* Process children */ for (node = ci->children; node; node = node->next) { if (is_node(node, "name")) - *name = extract_text(node); + config->name = extract_text(node); else if (is_node(node, "description")) - *description = extract_text(node); + config->description = extract_text(node); else if (is_node(node, "shortDescription")) - *brief = extract_text(node); + config->brief = extract_text(node); else if (is_node(node, "vendor")) - *vendor = extract_text(node); + config->vendor = extract_text(node); /* Note: the DTD allows for vendor + brief but models only use * vendor and everything else only uses shortDescription */ } - if (!*name || !strlen(*name)) { + if (!config->name || !strlen(config->name)) { log_err(ctx, "xml:%d: missing required element 'name'\n", ci->line); - free(*name); - free(*description); - free(*brief); - free(*vendor); + config_item_free(config); return false; } @@ -760,34 +789,31 @@ static void parse_model(struct rxkb_context *ctx, xmlNode *model, enum rxkb_popularity popularity) { - char *name, *description, *brief, *vendor; + struct config_item config = config_item_new(popularity); - if (parse_config_item(ctx, model, &name, &description, &brief, &vendor)) { + if (parse_config_item(ctx, model, &config)) { struct rxkb_model *m; list_for_each(m, &ctx->models, base.link) { - if (streq(m->name, name)) { - free(name); - free(description); - free(brief); - free(vendor); + if (streq(m->name, config.name)) { + config_item_free(&config); return; } } /* new model */ m = rxkb_model_create(&ctx->base); - m->name = name; - m->description = description; - m->vendor = vendor; - m->popularity = popularity; + m->name = config.name; + m->description = config.description; + m->vendor = config.vendor; + m->popularity = config.popularity; list_append(&ctx->models, &m->base.link); } } static void parse_model_list(struct rxkb_context *ctx, xmlNode *model_list, - enum rxkb_popularity popularity) + enum rxkb_popularity popularity) { xmlNode *node = NULL; @@ -850,14 +876,14 @@ parse_variant(struct rxkb_context *ctx, struct rxkb_layout *l, xmlNode *variant, enum rxkb_popularity popularity) { xmlNode *ci; - char *name, *description, *brief, *vendor; + struct config_item config = config_item_new(popularity); - if (parse_config_item(ctx, variant, &name, &description, &brief, &vendor)) { + if (parse_config_item(ctx, variant, &config)) { struct rxkb_layout *v; bool exists = false; list_for_each(v, &ctx->layouts, base.link) { - if (streq(v->name, name) && streq(v->name, l->name)) { + if (streq(v->name, config.name) && streq(v->name, l->name)) { exists = true; break; } @@ -868,11 +894,11 @@ parse_variant(struct rxkb_context *ctx, struct rxkb_layout *l, list_init(&v->iso639s); list_init(&v->iso3166s); v->name = strdup(l->name); - v->variant = name; - v->description = description; + v->variant = config.name; + v->description = config.description; // if variant omits brief, inherit from parent layout. - v->brief = brief == NULL ? strdup_safe(l->brief) : brief; - v->popularity = popularity; + v->brief = config.brief == NULL ? strdup_safe(l->brief) : config.brief; + v->popularity = config.popularity; list_append(&ctx->layouts, &v->base.link); for (ci = variant->children; ci; ci = ci->next) { @@ -913,10 +939,7 @@ parse_variant(struct rxkb_context *ctx, struct rxkb_layout *l, } } } else { - free(name); - free(description); - free(brief); - free(vendor); + config_item_free(&config); } } } @@ -937,16 +960,16 @@ static void parse_layout(struct rxkb_context *ctx, xmlNode *layout, enum rxkb_popularity popularity) { - char *name, *description, *brief, *vendor; + struct config_item config = config_item_new(popularity); struct rxkb_layout *l; xmlNode *node = NULL; bool exists = false; - if (!parse_config_item(ctx, layout, &name, &description, &brief, &vendor)) + if (!parse_config_item(ctx, layout, &config)) return; list_for_each(l, &ctx->layouts, base.link) { - if (streq(l->name, name) && l->variant == NULL) { + if (streq(l->name, config.name) && l->variant == NULL) { exists = true; break; } @@ -956,17 +979,14 @@ parse_layout(struct rxkb_context *ctx, xmlNode *layout, l = rxkb_layout_create(&ctx->base); list_init(&l->iso639s); list_init(&l->iso3166s); - l->name = name; + l->name = config.name; l->variant = NULL; - l->description = description; - l->brief = brief; - l->popularity = popularity; + l->description = config.description; + l->brief = config.brief; + l->popularity = config.popularity; list_append(&ctx->layouts, &l->base.link); } else { - free(name); - free(description); - free(brief); - free(vendor); + config_item_free(&config); } for (node = layout->children; node; node = node->next) { @@ -1001,25 +1021,22 @@ static void parse_option(struct rxkb_context *ctx, struct rxkb_option_group *group, xmlNode *option, enum rxkb_popularity popularity) { - char *name, *description, *brief, *vendor; + struct config_item config = config_item_new(popularity); - if (parse_config_item(ctx, option, &name, &description, &brief, &vendor)) { + if (parse_config_item(ctx, option, &config)) { struct rxkb_option *o; list_for_each(o, &group->options, base.link) { - if (streq(o->name, name)) { - free(name); - free(description); - free(brief); - free(vendor); + if (streq(o->name, config.name)) { + config_item_free(&config); return; } } o = rxkb_option_create(&group->base); - o->name = name; - o->description = description; - o->popularity = popularity; + o->name = config.name; + o->description = config.description; + o->popularity = config.popularity; list_append(&group->options, &o->base.link); } } @@ -1028,17 +1045,17 @@ static void parse_group(struct rxkb_context *ctx, xmlNode *group, enum rxkb_popularity popularity) { - char *name, *description, *brief, *vendor; + struct config_item config = config_item_new(popularity); struct rxkb_option_group *g; xmlNode *node = NULL; xmlChar *multiple; bool exists = false; - if (!parse_config_item(ctx, group, &name, &description, &brief, &vendor)) + if (!parse_config_item(ctx, group, &config)) return; list_for_each(g, &ctx->option_groups, base.link) { - if (streq(g->name, name)) { + if (streq(g->name, config.name)) { exists = true; break; } @@ -1046,9 +1063,9 @@ parse_group(struct rxkb_context *ctx, xmlNode *group, if (!exists) { g = rxkb_option_group_create(&ctx->base); - g->name = name; - g->description = description; - g->popularity = popularity; + g->name = config.name; + g->description = config.description; + g->popularity = config.popularity; multiple = xmlGetProp(group, (const xmlChar*)"allowMultipleSelection"); if (multiple && xmlStrEqual(multiple, (const xmlChar*)"true")) @@ -1058,10 +1075,7 @@ parse_group(struct rxkb_context *ctx, xmlNode *group, list_init(&g->options); list_append(&ctx->option_groups, &g->base.link); } else { - free(name); - free(description); - free(brief); - free(vendor); + config_item_free(&config); } for (node = group->children; node; node = node->next) { @@ -1146,9 +1160,12 @@ validate(struct rxkb_context *ctx, xmlDoc *doc) xmlValidCtxt *dtdvalid = NULL; xmlDtd *dtd = NULL; xmlParserInputBufferPtr buf = NULL; - /* This is a modified version of the xkeyboard-config xkb.dtd. That one - * requires modelList, layoutList and optionList, we - * allow for any of those to be missing. + /* This is a modified version of the xkeyboard-config xkb.dtd: + * • xkeyboard-config requires modelList, layoutList and optionList, + * but we allow for any of those to be missing. + * • xkeyboard-config sets default value of “popularity” to “standard”, + * but we set this value depending if we are currently parsing an + * “extras” rule file. */ const char dtdstr[] = "\n" @@ -1165,7 +1182,7 @@ validate(struct rxkb_context *ctx, xmlDoc *doc) "\n" "\n" "\n" - "\n" + "\n" "\n" "\n" "\n" diff --git a/test/registry.c b/test/registry.c index 141281857..a3993e8d5 100644 --- a/test/registry.c +++ b/test/registry.c @@ -46,6 +46,12 @@ enum { OPTION, }; +enum popularity { + POPULARITY_UNDEFINED = 0, /* Not member of enum rxkb_popularity */ + POPULARITY_STANDARD = RXKB_POPULARITY_STANDARD, + POPULARITY_EXOTIC = RXKB_POPULARITY_EXOTIC, +}; + struct test_model { const char *name; /* required */ const char *vendor; @@ -59,6 +65,7 @@ struct test_layout { const char *description; const char *iso639[3]; /* language list (iso639 three letter codes), 3 is enough for our test */ const char *iso3166[3]; /* country list (iso3166 two letter codes), 3 is enough for our tests */ + enum popularity popularity; }; struct test_option { @@ -74,6 +81,22 @@ struct test_option_group { struct test_option options[10]; }; +static const char * +popularity_attr(enum popularity popularity) +{ + switch (popularity) { + case POPULARITY_UNDEFINED: + return ""; + case RXKB_POPULARITY_STANDARD: + return " popularity=\"standard\""; + case RXKB_POPULARITY_EXOTIC: + return " popularity=\"exotic\""; + default: + fprintf(stderr, "ERROR: unsupported popularity: %d\n", popularity); + assert(false); + } +} + static void fprint_config_item(FILE *fp, const char *name, @@ -81,10 +104,11 @@ fprint_config_item(FILE *fp, const char *brief, const char *description, const char * const iso639[3], - const char * const iso3166[3]) + const char * const iso3166[3], + enum popularity popularity) { - fprintf(fp, " \n" - " %s\n", name); + fprintf(fp, " \n" + " %s\n", popularity_attr(popularity), name); if (brief) fprintf(fp, " %s\n", brief); if (description) @@ -156,7 +180,8 @@ test_create_rules(const char *ruleset, for (const struct test_model *m = test_models; m->name; m++) { fprintf(fp, "\n"); - fprint_config_item(fp, m->name, m->vendor, NULL, m->description, NULL, NULL); + fprint_config_item(fp, m->name, m->vendor, NULL, m->description, + NULL, NULL, POPULARITY_UNDEFINED); fprintf(fp, "\n"); } fprintf(fp, "\n"); @@ -174,13 +199,13 @@ test_create_rules(const char *ruleset, while (l->name) { fprintf(fp, "\n"); - fprint_config_item(fp, l->name, NULL, l->brief, l->description, l->iso639, l->iso3166); + fprint_config_item(fp, l->name, NULL, l->brief, l->description, l->iso639, l->iso3166, l->popularity); if (next->name && streq(next->name, l->name)) { fprintf(fp, "\n"); do { fprintf(fp, "\n"); - fprint_config_item(fp, next->variant, NULL, next->brief, next->description, next->iso639, next->iso3166); + fprint_config_item(fp, next->variant, NULL, next->brief, next->description, next->iso639, next->iso3166, next->popularity); fprintf(fp, "\n"); l = next; next++; @@ -199,10 +224,12 @@ test_create_rules(const char *ruleset, for (const struct test_option_group *g = test_groups; g->name; g++) { fprintf(fp, "\n", g->allow_multiple_selection ? "true" : "false"); - fprint_config_item(fp, g->name, NULL, NULL, g->description, NULL, NULL); + fprint_config_item(fp, g->name, NULL, NULL, g->description, + NULL, NULL, POPULARITY_UNDEFINED); for (const struct test_option *o = g->options; o->name; o++) { fprintf(fp, " \n"); } fprintf(fp, "\n"); @@ -775,35 +802,69 @@ test_load_invalid_languages(void) static void test_popularity(void) { - struct test_layout system_layouts[] = { - {"l1", NO_VARIANT }, - {"l1", "v1" }, + assert(POPULARITY_UNDEFINED != POPULARITY_STANDARD); + assert(POPULARITY_UNDEFINED != POPULARITY_EXOTIC); + + struct test_layout system_layouts[] = { + {.name = "l1", .variant = NO_VARIANT }, /* Default popularity */ + {.name = "l1", .variant = "v1" }, /* Default popularity */ + {.name = "l2", .popularity = POPULARITY_STANDARD }, + {.name = "l3", .popularity = POPULARITY_EXOTIC }, {NULL}, }; struct rxkb_context *ctx; struct rxkb_layout *l; - const char *ruleset = "xkbtests.extras"; - char *dir = NULL; - - dir = test_create_rules(ruleset, NULL, system_layouts, NULL); - ctx = rxkb_context_new(RXKB_CONTEXT_NO_DEFAULT_INCLUDES | - RXKB_CONTEXT_LOAD_EXOTIC_RULES); - assert(ctx); - assert(rxkb_context_include_path_append(ctx, dir)); - /* Hack: rulest above generates xkbtests.extras.xml, loading "xkbtests" - * means the extras file counts as exotic */ - assert(rxkb_context_parse(ctx, "xkbtests")); - - l = fetch_layout(ctx, "l1", NO_VARIANT); - assert(rxkb_layout_get_popularity(l) == RXKB_POPULARITY_EXOTIC); - rxkb_layout_unref(l); - - l = fetch_layout(ctx, "l1", "v1"); - assert(rxkb_layout_get_popularity(l) == RXKB_POPULARITY_EXOTIC); - rxkb_layout_unref(l); - - test_remove_rules(dir, ruleset); - rxkb_context_unref(ctx); + struct ruleset_conf { + const char *ruleset; + enum rxkb_context_flags flags; + enum rxkb_popularity popularity; /* Default popularity */ + }; + struct ruleset_conf rulesets[] = { + { /* Rules with “standard” popularity */ + .ruleset="xkbtests", + .flags=RXKB_CONTEXT_NO_DEFAULT_INCLUDES, + .popularity=RXKB_POPULARITY_STANDARD + }, + { /* Rules with “exotic” popularity (hack, see below) */ + .ruleset="xkbtests.extras", + .flags=RXKB_CONTEXT_NO_DEFAULT_INCLUDES | + RXKB_CONTEXT_LOAD_EXOTIC_RULES, + .popularity=RXKB_POPULARITY_EXOTIC + } + }; + for (size_t k = 0; k < ARRAY_SIZE(rulesets); k++) { + struct ruleset_conf *conf = &rulesets[k]; + char *dir = NULL; + + dir = test_create_rules(conf->ruleset, NULL, system_layouts, NULL); + ctx = rxkb_context_new(conf->flags); + assert(ctx); + assert(rxkb_context_include_path_append(ctx, dir)); + /* Hack: ruleset "xkbtests.extras" above generates xkbtests.extras.xml, + * loading "xkbtests" means the extras file counts as exotic */ + assert(rxkb_context_parse(ctx, "xkbtests")); + + /* Test implicit popularity */ + l = fetch_layout(ctx, "l1", NO_VARIANT); + assert(rxkb_layout_get_popularity(l) == conf->popularity); + rxkb_layout_unref(l); + + l = fetch_layout(ctx, "l1", "v1"); + assert(rxkb_layout_get_popularity(l) == conf->popularity); + rxkb_layout_unref(l); + + /* Test explicit popularity */ + l = fetch_layout(ctx, "l2", NO_VARIANT); + assert(rxkb_layout_get_popularity(l) == RXKB_POPULARITY_STANDARD); + rxkb_layout_unref(l); + + l = fetch_layout(ctx, "l3", NO_VARIANT); + assert(rxkb_layout_get_popularity(l) == RXKB_POPULARITY_EXOTIC); + rxkb_layout_unref(l); + + test_remove_rules(dir, conf->ruleset); + rxkb_context_unref(ctx); + } }