sorlok / waitzar

Automatically exported from code.google.com/p/waitzar
Other
0 stars 0 forks source link

Input Method Code Cleanup #190

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
The ConfigManager is a lot cleaner now, so I'd like to briefly sketch some 
cleanup ideas for Input Methods. Whereas ConfigManager was one big lump of 
messy code, InputMethods span multiple files and have strange, obscure 
interdependencies. I'll have to review the code individually, but off the bat 
here's a few ideas:
   * I made RomanInputMethod a templated class because I feared slowing down WaitZar vis. Burglish. However, a virtual function call (in single-inheritance mode) is phenomenally fast in C++. I'd like to turn RomanInputMethod into its own class, and extend it to WaitZar and Burglish input methods. This will enable a whole lot more changes.
   * Currently, "symbols" are handles in InputMethod.... but it's almost guaranteed that LetterInputMethods and Keyboards won't revert to symbols. 
   * An InputMethod should be able to specify what "category" of keys it needs: Regular, Shifted, Ctrl+Regular, Ctrl+Alt+Shift+Regular, etc. We should have a way to dyanmically enable/disable entire key sets when switching InputMethods (this can come later; for now, just define key sets and build it in to the KeyMagic binary format).
   * HelpKeyboards are waaaay too complex; the RomanInputMethod has to maintain one copy, and MainFile another... and then they build up different input strings, for no clear reason.
   * Virtual Keys should be unified; there's already an Issue for this.

Original issue reported on code.google.com by seth.h...@gmail.com on 2 Feb 2011 at 3:18

GoogleCodeExporter commented 9 years ago
* In an attempt to unify Virtual Keys, I made the code a lot more manageable; 
for example, the original (scancode) and locale-dependent key values are saved 
when the key is created, and a pointer is used to retrieve the "current" value 
(with the means provided for retrieving either one at any time).

 This will allow us to deal with keys as directly as possible, and sets the stage for full unification.

* RomanInputMethod's templates have been removed.

* InputMethod now handles key dispatch, so LetterInputMethod doesn't need to 
declare handleFullStop()/handleNumber(), etc. (It was forwarding them to 
handleKeyPress() anyway).

Original comment by seth.h...@gmail.com on 17 Feb 2011 at 2:06

GoogleCodeExporter commented 9 years ago
The more I look at options to clean up this code, the more I am convinced that 
its troubles could be mostly eliminated through a better application of MVC:
   * The WordBuilder is the model. It shouldn't handle things like "press left" or "press right", but should have (essentially) "match(string roman)" and "reset()" ...and calling match multiple times should "add to" the match.
  * The InputMethod and its subclasses should be the controller. They should have a way of triggering repainting or SendInput() in MainFile, and should handle all key dispatches. They should be less concerned with maintaining strings (that was because I didn't trust char* in WordBuilder when I started coding WZ).
  * The MainFile's recalc() method is the View.

There are other things to consider:

  * WordBuilder will still need "getDefaultEntryIndex()", since Pat-Sint is part of the model. This is not a problem. Also, InputMethod should probably construct the string of hotkey shortcuts (~, 1, 2, 3..  or just 1,2,3..  depending on PS) and pass them off to MainFile for painting.

  * HelpInput is mildly unclear. I feel that the InputMethod should just build up the string it "expects", with no knowledge of whether or not it's in Help mode, and then MainFile can "getCandidate()" or whatever (instead of the horrible ternary operator mess we use now). Perhaps, if WordBuilder is promoted to an actual model, we might promote some kind of model for LetterInputMethods? This would definitely simplify KeyMagic keyboards. 

  * The SentenceList is equally unclear. It has a lot of nice functionality, but I'm beginning to think that it might be better implemented as a model ONLY for the sentence window. Then, each of the 2 primary windows will have its own model, and we can avoid all this "if helpWindow.isVisible()" nonsense. Likewise, we might be able to use this kind of model as a basis for the KeyMagicKeyboards too, since they will have similar features (like "generating" a word).

Based on our work with the config loaders, I'll confirm that building up a new 
infrastructure side-by-side to the old one is a little bit constraining, but in 
the end it has 2 key benefits:
  * It avoids the tendency to design poorly-thought-out systems, which happens when one designs from scratch.
  * It means that tiny non-obvious fixes (like fixing 4 or 5 words that convert wrongly using Ko Soe Min's code) are not forgotten. 

Unfortunately, it's also a bit maddening. But I already "fixed" this code once; 
I have no desire to touch it again after this next fix.

Original comment by seth.h...@gmail.com on 17 Feb 2011 at 2:18