sailfish-keyboard / sailfishos-presage-predictor

Presage based input predictor for the Sailfish OS
https://openrepos.net/content/sailfishkeyboard/maliit-plugin-presage
GNU General Public License v3.0
7 stars 4 forks source link

Hunspell support #23

Closed rinigus closed 6 years ago

rinigus commented 6 years ago

I am ready to submit the next PR. This one adds support for Hunspell predictor. This has been the latest change I did in Presage and now its hooked to the plugin as well.

Few important changes:

As before, RPMs are available are at https://build.merproject.org/package/show/home:rinigus:keyboard/sailfishos-presage-predictor . All languages that we test with are covered in terms of n-grams (Swedish is using the latest version from OpenRepos RPM) and hunspell dictionaries. Keyboards for SWE and HUN are missing, those you have to hack together yourself.

Enjoy!

rinigus commented 6 years ago

Hi, how is the testing going? :)

martonmiklos commented 6 years ago

Ah, yesterday we have had a shifted workday so I have not managed to do testing.

rinigus commented 6 years ago

I see :). For English, all is available at OBS. For Hungarian, you would just have to modify keyboard conf to have language specified as hu_HU and get hunspell dictionary together with presage language database from OBS.

rinigus commented 6 years ago

PS: but do remove earlier install first to be sure that there are no conflicts

rinigus commented 6 years ago

Hi, any updates on this?

rinigus commented 6 years ago

@martonmiklos: how is the testing going? I would like to get "forget" branch into the release as well, so its becoming time pressing.

Since the configuration is the same for hunspell and forget, if you haven't tested hunspell, you might as well start testing with the forget branch. Its available at http://repo.merproject.org/obs/home:/rinigus:/keyboard:/devel/sailfish_latest_armv7hl/

@ljo: have you tested hunspell? maybe you could look into the code and review?

In general, its quite straightforward code additions - main work was actually to add the predictor to presage (which was also relatively easy due to its modular nature).

rinigus commented 6 years ago

Morning, I would like to merge this PR into the master. It's been sitting here for 13 days (including 2 weekends) without any feedback.

We don't have any timing ground rules. I would think that 24 hours is too short for the required first feedback, but maybe it's sufficient for giving a time estimate on when review will be done, or aimed to be done. I would suggest to target reviews within 3-4 days and warn when such deadline is not possible.

As for this PR, I would suggest that I'll merge it tonight, in about 12 hours, if there is no one saying otherwise. As soon as it's done, I'll submit the next PR which includes forgetting a word functionality and code cleanup.

Sorry for pushing, just want to keep moving within suggested timeline and get release out this weekend

rinigus commented 6 years ago

@martonmiklos - thank you very much!!!