mordak / Term48

50 stars 20 forks source link

Moved preferences into own struct, rewrote keymapping lookups #22

Closed ardangelo closed 6 years ago

ardangelo commented 7 years ago

I have created a pref_t type holding all necessary Term48 preferences; preferences.c now provides functions to read in a libconfig file and convert to pref_t and write a pref_t out to a libconfig file. Keymaps now have the type struct { char from; char *to; } and functions are provided in preferences.c to look these up. As part of this change, I rewrote the symbol menu rendering code. In addition to using the new keymapping mechanisms, the entire symbol menu is rendered at startup into an SDL_Surface and blitted during the render loop instead of constructing it ad-hoc when the symbol menu is shown.

This was spurred on by Term48's behavior of overwriting the .term48rc file upon encountering an invalid setting. The .term48rc is no longer overwritten, however the offending entries are quietly replaced by their default values during initialization. Ideally would have some sort of output that informs about the offending entries but couldn't get any debug output to display before pty initialization.

In the short term, I plan to get keyhold accent menus implemented. I also have some future ideas for a more general framework for keyhold actions, touch targets, and customizable layers beyond meta-mode that would replace the current key-checking loop with a "switchboard" of function pointers.

mordak commented 7 years ago

I like the look of this - moving to a prefs struct is nicer. I've looked over the diff and it looks good (lots of whitespace diff though, which makes it look bigger than it is).

I will load it up on my Passport this weekend to make sure everything still works there. I see you removed the isPassport var, which I remember putting in because the swipe gestures don't work like on the Playbook/Z10/30. This is behind a pref var now, which should be fine, but we might want to force auto_show_vkb=true on Passports, so users can't get themselves into a state where they can't edit the prefs file because the symbol keys are missing.

Anyway, nice work! I assume you have a Q5/10 or Classic? That's great, because I don't have any of those anymore, so symmenu stuff is hard to do properly. :-)

mordak commented 7 years ago

I added some minor comments that should be easy to fix. The only real showstopper is the absence of backwards compat from prefs v8 and prefs v9. Changing types from group to list is fine, but we need to handle upgrading old config files to the new types / format, and populate the upgraded config file with the default values for any new variables (rescreen for symmenu), and also fix the READMEs to reflect any changes.

ardangelo commented 7 years ago

I've fixed all the other things up and stubbed out an upgrade function. I figure the best way would be to operate on the config_t object directly to upgrade any changed fields (e.g. symkey groups to symmenu lists) and leave the pref_t creation code only operating on the latest prefs version.

mordak commented 7 years ago

Looks good. Operating on the config_t object is cool. The switch statement is good - it will allow fallthrough for people jumping > 1 version. Fill in the stub and I think we are good to go. It would be cool if there was a upgrade_config_v6() that handled the previous business of changing the hjkl values. :-)

ardangelo commented 6 years ago

Hey, I've got the upgrade functions working. It'll upgrade the changed fields and store the old config in .term48rc-old. All accents are entered into keyhold_accents.c, however there's not currently a way to bring up uppercase accents (KEYMOD_SHIFT is cleared upon keydown, is there another way to check if it was in "shift mode"?).

mordak commented 6 years ago

This looks awesome! I am going to merge it now. If you have any other stuff coming in it will be easier to eyeball in a diff that is less huge. Great work!