On 20-08-02 06:42, Hiltjo Posthuma wrote: > Thanks for improving the patches. There are many changes so bear with me to > review them. It may take some time.
No problem, thanks for looking at it. > Some notes: > - The man page should list and document all the (new) options: > -D, -O, -l, -s, -H, etc. > The environment variables should also be documented: > SVKBD_LAYER, SVKBD_HEIGHTFACTOR, SVKBD_ENABLEOVERLAYS Done! (Patch will follow seperately in next mail) > - `debug` could be a function debug(fmt, ...) and print to stderr + fflush. Good idea, implemented. I still kept the debug check out of the function as I'm not sure if the compiler optimizes it enough otherwise. > - The default layout was changed, maybe it should stay english, svkbd is not > only for mobile. svkbd is not specificly for sxmo/mobile, but I'd like to > support this project. Yes, that was a very deliberate choice. The svkbd-mobile-intl layout is the most feature-rich layout so I felt it made sense to have that as the default. It offers at least all input abilities the traditional layout did, plus a lot more, so it makes a good default virtual keyboard experience, I'd argue. As it's smaller, it works on mobile devices (but that shouldn't hinder anyone from using it on larger displays of course). The reverse doesn't hold true: the traditional layout would work very poorly on mobile devices. I'd actually also want to propose to have make install symlink an svkbd symlink to the actually compiled layout (which by default would be svkbd-mobile-intl then), assuming there is no symlink already. > - Theres still a buffer overflow in SVKBD_LAYERS. You could use strdup() > there. Oops, I see what you mean now yes. I fixed it. > - Code-style should be similar to st, comment style should be /*, > TABs should be used consistently. */ Fixed > Below is a patch to improve the Makefile: > - fix makefile to copy config.h rule. > - config.mk should use pkg-config (PKG_CONFIG), like st config.mk. Right! Thanks! I wasn't sure how to get that right. In a moment I'll be sending a small patch to be applied on top of the 14 already submitted (and top of your Makefile patch). Regards, -- Maarten van Gompel [email protected] https://proycon.anaproy.nl https://github.com/proycon GnuPG key: 0x39FE11201A31555C XMPP: [email protected] Matrix: @proycon:matrix.anaproy.nl Telegram: proycon IRC: proycon (freenode) Discord: proycon#8272 Mastodon: https://social.anaproy.nl/@proycon (@[email protected]) Twitter: https://twitter.com/proycon Bitcoin: 1BRptZsKQtqRGSZ5qKbX2azbfiygHxJPsd
