Created attachment 289824 [details] [review]
Add custom labels for keys

Some keys do not have a label, so one has to guess what it does.

This tries to find a label for dead keys by looking for the label of the non-dead key.
And it adds the posibility to force a custom label for a key in the layout:
Just add a label attribute to your key.
I.e. <key name="Esc" label="Esc" />

Added custom labels for the us-layout.

Thanks for the report and patch, but wouldn't it be a pain if all such symbols need to have a label="..." attribute?   I agree it is a good idea to use Gdk.keyval_name() as a fallback.

Currently only a small subset of the needed labels is defined. And even the default keyboard has some missing labels. Of course it would be best to have as many labels as possible predefined. But I think it is good to have the option to set a custom label when it is needed.

Personally, I am using the neo layout (http://en.wikipedia.org/wiki/Keyboard_layout#Neo) for which I have written a custom layout for caribou. But It contains many special keys like math-symbols, greek letters, dead-keys and function keys which do not have a label by default. Therefore it is almost unusable.
For some keys this can be forced bey setting a text, but this also prints this text when pressing the key. For function keys and dead-keys this destrois the functionality of the key. In this case a custom label is needed.

(In reply to comment #3)
> Currently only a small subset of the needed labels is defined.

However, you defined labels for F1, ..., F12.  The number doesn't look small to me.  Also, Gdk.keyval_name/Gdk.keyval_from_name roundtrip for those keys:

  gjs> Gdk.keyval_name(Gdk.keyval_from_name('F11'))
  "F11"

so those labels are redundant, if we could use Gdk.keyval_name as a fallback.

> default keyboard has some missing labels. Of course it would be best to have as
> many labels as possible predefined. But I think it is good to have the option
> to set a custom label when it is needed.

I think there are two separate issues.  One is that the default keyboard is missing some labels, and another is a feature to allow custom labels.  I'd prefer to have two separate patches for them.

The patch for the former could be: extend label_map with Esc and Tab, which Gdk.keyval_name doesn't return usable labels, and use Gdk.keyval_name for the rest.  There's no dead keys defined in the default layout, no need to worry about them.

Then, for the latter "feature", you can propose whatever you like.

Created attachment 290074 [details] [review]
Add label for all keys

Some keys do not have a label, so one has to guess what it does.

First, this tries to find a label for dead keys by looking for the label of the non-dead key.
Second, if no label is found the key name is used as a label. Every valid key should have a label now.
And last, keys with an invalid name are labeled with "invalid name"

Review of attachment 290074 [details] [review]:

Thanks, looks basically good to me.

::: libcaribou/key-model.vala
@@ +121,3 @@
+                        if (!uc.isspace () && uc != 0)
+                            label = uc.to_string ();
+                    }

The logic is becoming a bit complex; can you add some comment on each 'if'?

@@ +133,3 @@
+                        else
+                            label = "invalid\nname";
+                    }

Perhaps this block could be simplified as:

if (label == "") {
    if (_keyvals.length > 0)
        label = Gdk.keyval_name (_keyvals[0]);
    else
        label = "invalid\nname";
}

Created attachment 290104 [details] [review]
Add label for all keys

I have split the keyval and label determination into two separate method, because the code got a bit long and complex.

And I have commented the steps in finding a label.

Created attachment 290137 [details] [review]
Add label to all keys

Some keys do not have a label, so one has to guess what it does.

Do the following for keys we do not have a label yet.
First, try to find a label for dead keys by looking for the label
of the non-dead key.
Second, if no label is found use the key name as a label.
--
Sorry, it looks too much to me.  Since some part of init_key_label depends on init_keyval, I think it's not a good idea to split them into separate methods.

I'd like to go with your second patch, with slight modification:
- Just use 'name' as a fallback label.  If the keyval was determined, the 'name' should already be the valid key name.
- Use "" as an invalid key name, since "invalid name" might not fit in a normal key.

Review of attachment 290137 [details] [review]:

Thanks, looks good to me.

Review of attachment 290137 [details] [review]:

Thanks, looks good to me.

Attachment 290137 [details] pushed as a86790a - Add label to all keys