Code review comment for lp://qastaging/~popey/ubuntu-terminal-app/add-control

Hi Bart,

In my opinion the patch looks way better than a lot of first-time QML patches. Congrats on that!
Regarding your questions: I guess we can give you a helping hand here. ;)

I'll try to answer the translation question right now.

Translations are handled by JsonTranslator.qml. It contains a getTranslatedNamyById() method which is already capable of translating the string "CTRL". You can find an example call to that method in line 84 of KeyboardLayout.qml. You only need to tell it that you are trying to translate a modifier (arg #1) and then pass the modifier string from the json file (arg #2) and it will provide you with the translated string.

To become more specific, I would suggest to modify the json file to contain the following code:

"main_action" : {
    "type": "modifier",
    "mod": "Control"
}

(The shorter form "mod" to keep it in line with the rest of the syntax if it doesn't break anything else.)

Then add another case to the createKeyText() method of KeyboardLayout.qml to enable translations of modifier keys and use that method in createNextModifierActionString() to determine the text of the action.

That should do the trick. ;)

By the way, while we're at it, would you mind adjusting the error message in line 36 of jsonParser.js to include the new modifier keyword? :)

If you have any further questions, feel free to ask. I agree that the highlighting thing is not easy and it will require some time for me to look into it as well. I'll try to do that when I have some spare time but maybe someone else is capable of doing that earlier.

Thanks again for the great patch!