Skip to content

Commit

Permalink
rules: Fix wild card handling
Browse files Browse the repository at this point in the history
The handling of wild card `*` is different in libxkbfile and X server:
wild card matches empty strings for model and option but not for layout
nor variant, while in libxkbcommon wild cards always match empty strings.

See:
- https://gitlab.freedesktop.org/xorg/lib/libxkbfile/-/blob/bf985c68acb1244f51ec91414532a2347fbc1c4c/src/maprules.c#L687
- https://gitlab.freedesktop.org/xorg/lib/libxkbfile/-/blob/bf985c68acb1244f51ec91414532a2347fbc1c4c/src/maprules.c#L712

The difference of handling between the components is unfortunately not
documented, but we should follow the behavior of the original
implementations for consistency.

- Fixed by implementing the same behavior than libxkbfile.
- Added tests and fixed failing tests.
- Improve the documentation of rules to highlight the special behavior.
  • Loading branch information
wismill committed Aug 20, 2024
1 parent ce32dec commit 970a7d5
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 11 deletions.
6 changes: 4 additions & 2 deletions doc/rules-format.md
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,10 @@ rules.
@anchor rules-wildcard-def
Along with matching values by simple string equality and for
membership in a [group] defined previously, rules may also contain
**wildcard** values “\*” which *always match*. These usually appear
near the end of a rule set to set *default* values.
**wildcard** values “\*” with the following behavior:
- For `model` and `options`: *always* match.
- For `layout` and `variant`: match any *non-empty* value.
These usually appear near the end of a rule set to set *default* values.

```c
! layout = keycodes
Expand Down
21 changes: 13 additions & 8 deletions src/xkbcomp/rules.c
Original file line number Diff line number Diff line change
Expand Up @@ -694,20 +694,22 @@ match_group(struct matcher *m, struct sval group_name, struct sval to)

static bool
match_value(struct matcher *m, struct sval val, struct sval to,
enum mlvo_match_type match_type)
enum mlvo_match_type match_type, bool wildcard_matches_empty)
{
if (match_type == MLVO_MATCH_WILDCARD)
return true;
/* Wild card match empty values only if explicitly required */
return wildcard_matches_empty || !!to.len;
if (match_type == MLVO_MATCH_GROUP)
return match_group(m, val, to);
return svaleq(val, to);
}

static bool
match_value_and_mark(struct matcher *m, struct sval val,
struct matched_sval *to, enum mlvo_match_type match_type)
struct matched_sval *to, enum mlvo_match_type match_type,
bool wildcard_matches_empty)
{
bool matched = match_value(m, val, to->sval, match_type);
bool matched = match_value(m, val, to->sval, match_type, wildcard_matches_empty);
if (matched)
to->matched = true;
return matched;
Expand Down Expand Up @@ -868,25 +870,28 @@ matcher_rule_apply_if_matches(struct matcher *m, struct scanner *s)
struct matched_sval *to;
bool matched = false;

/* NOTE: Wild card matches empty values only for model and options, as
* implemented in libxkbfile and xserver. The reason for such different
* treatment is not documented. */
if (mlvo == MLVO_MODEL) {
to = &m->rmlvo.model;
matched = match_value_and_mark(m, value, to, match_type);
matched = match_value_and_mark(m, value, to, match_type, true);
}
else if (mlvo == MLVO_LAYOUT) {
xkb_layout_index_t idx = m->mapping.layout_idx;
idx = (idx == XKB_LAYOUT_INVALID ? 0 : idx);
to = &darray_item(m->rmlvo.layouts, idx);
matched = match_value_and_mark(m, value, to, match_type);
matched = match_value_and_mark(m, value, to, match_type, false);
}
else if (mlvo == MLVO_VARIANT) {
xkb_layout_index_t idx = m->mapping.variant_idx;
idx = (idx == XKB_LAYOUT_INVALID ? 0 : idx);
to = &darray_item(m->rmlvo.variants, idx);
matched = match_value_and_mark(m, value, to, match_type);
matched = match_value_and_mark(m, value, to, match_type, false);
}
else if (mlvo == MLVO_OPTION) {
darray_foreach(to, m->rmlvo.options) {
matched = match_value_and_mark(m, value, to, match_type);
matched = match_value_and_mark(m, value, to, match_type, true);
if (matched)
break;
}
Expand Down
4 changes: 4 additions & 0 deletions test/data/rules/groups
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
$layout_group * = my_symbols+%(v)
* * = default_symbols

! layout = symbols
$layout_group = my_symbols+%(v)
* = default_symbols

! model = types
* = default_types

Expand Down
32 changes: 32 additions & 0 deletions test/data/rules/wildcard
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
! model = keycodes
* = evdev

! model = geometry
* = pc(pc104)

! layout variant = symbols
* * = pc+%l%(v)

! layout[1] variant[1] = symbols
* * = pc+%l[1]%(v[1])

! layout[2] variant[2] = symbols
* * = +%l[2]%(v[2]):2

! layout[3] variant[3] = symbols
* * = +%l[3]%(v[3]):3

! layout[4] variant[3] = symbols
* * = +%l[4]%(v[4]):4

! model layout = compat
* * = complete

! model layout[1] = compat
* * = complete

! model layout[2] = compat
* * = complete

! model = types
* = complete
33 changes: 32 additions & 1 deletion test/rules-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ main(int argc, char *argv[])
struct test_data test2 = {
.rules = "simple",

.model = "", .layout = "", .variant = "", .options = "",
.model = "", .layout = "foo", .variant = "", .options = "",

.keycodes = "default_keycodes", .types = "default_types",
.compat = "default_compat", .symbols = "default_symbols",
Expand Down Expand Up @@ -220,6 +220,37 @@ main(int argc, char *argv[])
};
assert(test_rules(ctx, &test7));

/* Wild card does not match empty entries for layouts and variants */
#define ENTRY(_model, _layout, _variant, _options, _symbols, _should_fail) \
{ .rules = "wildcard", .model = _model, \
.layout = _layout, .variant = _variant, .options = _options, \
.keycodes = "evdev", .types = "complete", .compat = "complete", \
.symbols = _symbols , .should_fail = _should_fail }
struct test_data wildcard_data[] = {
/* OK: empty model and options and at least one layout+variant combo */
ENTRY(NULL, "a" , "1" , NULL, "pc+a(1)", false),
ENTRY("" , "a" , "1" , "" , "pc+a(1)", false),
ENTRY("" , "a," , "1," , "" , "pc+a(1)", false),
ENTRY("" , ",b" , ",2" , "" , "+b(2):2", false),
ENTRY("" , "a,b", "1," , "" , "pc+a(1)", false),
ENTRY("" , "a,b", ",2" , "" , "+b(2):2", false),
/* Fails: empty layout or variant */
ENTRY(NULL, NULL , NULL , NULL, "", true),
ENTRY(NULL, "" , "" , NULL, "", true),
ENTRY(NULL, NULL , "1" , NULL, "", true),
ENTRY(NULL, "" , "1" , NULL, "", true),
ENTRY(NULL, "," , "1,2", NULL, "", true),
ENTRY(NULL, "a" , NULL , NULL, "", true),
ENTRY(NULL, "a" , "" , NULL, "", true),
ENTRY(NULL, "a,b", NULL , NULL, "", true),
ENTRY(NULL, "a,b", "" , NULL, "", true),
ENTRY(NULL, "a,b", "," , NULL, "", true)
};
#undef ENTRY
for (size_t k = 0; k < ARRAY_SIZE(wildcard_data); k++) {
assert(test_rules(ctx, &wildcard_data[k]));
}

xkb_context_unref(ctx);
return 0;
}

0 comments on commit 970a7d5

Please sign in to comment.