Re: [hatari-devel] Recent symbolic keyboard mapping change

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


 
> Layouts I have not yet researched: FI, NO, DK, SA, NL, CS, HU

At least FI seems to be the same as Swedish, so you can drop that of your
list. I added NL and DK on my own. I never saw a SA TOS, and EmuTOS does
not support it, too, so I think we can ignore that one.

I'm glad that you added them, but I think there is value in independently reviewing them as well.
 
Well, what I did was basically a complete rewrite of your patches. Having a
full switch-case statement with all keys for *each* language looked pretty
much too big and unmaintainable to me, it's way easier if we only encode
the differences to the standard mapping.

So, yes I felt differently about what is maintainable when I wrote it, but that doesn't mean I am unwilling to do this as a compromise.

While working on it I frequently used diffs to compare the languages, which I considered part of the maintenance process. To me this felt easier than maintaining a "manual diff" in the code itself like you have done. However, I'm not making an argument for it, just explaining why my work in progress was done that way. I can work with it either way.

The work was done by:
1. Testing the corresponding host language keyboard, pressing every key available and logging each of them.
2. Make sure every key on the host keyboard is mapped to an ST key.
3. Make sure every ST key is mapped to at least one key on the host keyboard. (This is the reason for the noted 3 "arbitrary" keys on FR, and 1 on SE.)
4. Test this with EmuTOS, pressing every key and verifying the result. Test with official TOS if relevant.
5. Optionally, switch to other host languages and see how well the symbolic mapping works cross-language. This is basically just verification, as I would not prioritize anything from mismatched host/TOS but it is good to review how well the symbolic mapping system actually handles this. (IMO it does a good job!)
 
Requesting such a complete rewrite
from you seemed unfair to me, especially since you seemed to be busy with
other stuff nowadays, that's why I went ahead and did this rewrite on my
own. Sorry, I thought you'd rather be happy if you would not need to do
such a rewrite on your own, but I'll make sure that I ask you the next time
in advance.

When you incorporate my work like this and make changes, I feel a need to review it very carefully to make sure the effort hasn't gone to waste.  In order to review it, I essentially have to do the restructuring myself to be able to compare, and because I did I found many points in it which were neglected. I have done my best to verify every single mapping. I do not expect perfection from myself or others, but if something I verified is being altered or discarded I will make comment so we can figure out what it best.

Can you split up that work into individual, coherent steps, please?

If you would like it split up, I would need some guidance about what you feel should be an individual coherent step here.

If you look at the PR, you should see that there are zero logical code changes. All of this is changes to individual switch statements.

The "steps" for me were to review ~100 cases for each language and compare them against the switch tables I had already prepared. If a mapping was different, I corrected it and noted it. If a comment was missing or incomplete, I restored it or improved it. There are a lot of notes because a lot of these were lost or changed in your restructuring, but each change is an independent case statement.

If it helps I can tell you that as part of testing I wrote code to dump keymaps of each language and compare them against the original version of my work to make sure the result was still the same.


Aside from key case changes, in my version I had written it so that NVRAM is not allowed to affect the keyboard mapping unless the TOS was identified as a multi-language TOS. In your version, you did not include this in your revision. I did not attempt to restore this in my PR, but I do not know why you chose to discard it, perhaps you could comment? The reason I had done this was that I didn't think it made sense to allow NVRAM settings to change the symbolic mapping in conflict with the one the TOS can actually support. To me the case where this would happen would be the Falcon user switching from emuTOS to an original TOS that was not the same language as their NVRAM setting, and I felt that in this case the user would want mappings that work with the TOS in this case. I will put up no contest for this if you disagree, as it is a very minor point to me, but I wanted to point it out in case the omission was a mistake.

-- Brad



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/