xkbcomp: Some lists are not allow to be empty

[wismill]

(Follow-up of this comment

The following syntax does not parse in xkbcommon, but it does in xkbcomp:

xkb_symbols "x" {
    key <AC12> { };
};

While the usefulness of such statement is debatable, the fact that it does parse in xkbcomp and that tools may generate such keymap entry make it relevant to handle.

There are other places we could allow empty lists as well.

CC @mahkoh who detected this issue

[mahkoh]

Sorry that's not actually what I meant in that comment. I forgot that xkbcommon doesn't parse empty blocks so my example was a bit too simplified. And, in fact, keys are not affected at all by what I meant.

What I meant is the following:

static bool
UseNewInterpField(enum si_field field, SymInterpInfo *old, SymInterpInfo *new,
                  bool report, enum si_field *collide)
{
    if (!(old->defined & field))
        return true;

but, correctly for keys:

static bool
UseNewKeyField(enum key_field field, enum key_field old, enum key_field new,
               bool clobber, bool report, enum key_field *collide)
{
    if (!(old & field))
        return (new & field);

Example:

        interpret A {
            repeat = true;
        };
        interpret A {
            repeat = true;
        };
        augment interpret A {
            action = SetMods(mods=Mod1);
        };

Output:

	interpret A+AnyOfOrNone(all) {
		repeat= True;
		action= NoAction();
	};

Removing the second interpret produces the correct output:

	interpret A+AnyOfOrNone(all) {
		repeat= True;
		action= SetMods(modifiers=Mod1);
	};

[wismill]

I forgot that xkbcommon doesn't parse empty blocks

@mahkoh Do you have a list of such blocks or is it all of them? I think that the key one is the most relevant (fixed in #567), but I doubt it’s worth to fix them all.

[mahkoh]

I believe this can be determined easily by looking at parser.y. I believe that most lists do not allow empty interiors. E.g.

ActionList      :       ActionList COMMA Action
                        { $$ = ExprAppendActionList($1, $3); }
                |       Action COMMA Action
                        {
                            $$ = ExprCreateActionList($1);
                            $$ = ExprAppendActionList($$, $3);
                        }
                ;

requires at least 2 elements.

VarDeclList     :       VarDeclList VarDecl
                        { $$.head = $1.head; $$.last->common.next = &$2->common; $$.last = $2; }
                |       VarDecl
                        { $$.head = $$.last = $1; }
                ;

requires at least 1 element.

[wismill]

ActionList […] requires at least 2 elements.

@mahkoh Correct: We do not allow e.g. [{NoAction()}], only its canonical form [NoAction()]. I do think it makes sense to allow it.

VarDeclList […] requires at least 1 element.

I am not sure if there is a relevant use case for this, other than xkbcomp compatibility. While it would be easy to allow an empty list, relevant tests should be written too.

What list do you see relevant to allow to be empty, apart of #567?

[mahkoh]

I allow all lists to be empty in my implementation. I believe that is simplest both for the implementation and users.

[wismill]

There is no real use case then.

Implementation-wise it is easier in our code base to work with non-empty lists. I think we allow empty lists in enough relevant places. People hacking the remaining places where empty lists are not supported should be considered experts and will know how to fix it.

We may add support for this, but writing the corresponding tests will be most of the effort, so I am postponing this as very low priority.