Forget functionality

[rinigus]

This PR brings the changes as described below. Its build is available at http://repo.merproject.org/obs/home:/rinigus:/keyboard/sailfish_latest_armv7hl/armv7hl/ . No changes in dictionaries or otherwise were done, so only plugin needs to be updated to get new functionality.

• forget word API of Presage is used and exposed via long press on a word. So, there is no menu, as suggested in https://github.com/sailfish-keyboard/sailfishos-presage-predictor/issues/6 , but functionality is there. Word of warning for testers. If you write some strange word, like "Tutututu" continue typing the text and then delete this word while the word is still available, Presage will re-learn it using one or more of the predictors (recency and user-writable predictor). If you delete "Tutututu" and ask to forget it, it will all work as expected.

• Cleanup of the code by dropping excessive learning and DBUS. As described in https://github.com/sailfish-keyboard/sailfishos-presage-predictor/issues/25 , fixes #25 . It also fixes #1 through no-implement

• Cleanup of the code, dropping presage spec since we package it separately. This SPEC was used in that packaging.

[martonmiklos]

I have started to install the new packages, and came across a strange thing:

[root@Sailfish tmp]# zypper install maliit-plugin-presage-1.0+forget.20180315200947.2e5b021-10.24.1.jolla.armv7hl.rpm Loading repository data... Reading installed packages... Resolving package dependencies...

Problem: libpresage1-0.9.1+master.20180313195323.66.g40941f3-10.31.1.jolla.armv7hl requires presage-data, but this requirement cannot be provided uninstallable providers: presage-data-0.9.1-7.28.armv7hl[openrepos-martonmiklos] presage-data-0.9.1-7.29.armv7hl[openrepos-martonmiklos] presage-data-0.9.1-7.30.noarch[openrepos-martonmiklos] Solution 1: deinstallation of libpresage1-0.9.1+master.20180313195323.66.g40941f3-10.31.1.jolla.armv7hl Solution 2: do not install maliit-plugin-presage-1.0+forget.20180315200947.2e5b021-10.24.1.jolla.armv7hl Solution 3: break libpresage1-0.9.1+master.20180313195323.66.g40941f3-10.31.1.jolla.armv7hl by ignoring some of its dependencies

Choose from above solutions by number or cancel [1/2/3/c] (c): 1 Resolving dependencies... Resolving package dependencies...

The following NEW package is going to be installed: maliit-plugin-presage

The following packages are going to be REMOVED: libpresage1 presage-data

1 new package to install, 2 to remove. Overall download size: 344.5 KiB. After the operation, additional 551.1 KiB will be used. Continue? [y/n/?] (y): y Retrieving package maliit-plugin-presage-1.0+forget.20180315200947.2e5b021-10.24.1.jolla.armv7hl (1/1), 344.5 KiB (944.7 KiB unpacked) Removing libpresage1-0.9.1+master.20180313195323.66.g40941f3-10.31.1.jolla ........................................................................[done] Retrieving package maliit-plugin-presage-1.0+forget.20180315200947.2e5b021-10.24.1.jolla.armv7hl (1/1), 344.5 KiB (944.7 KiB unpacked) Installing: maliit-plugin-presage-1.0+forget.20180315200947.2e5b021-10.24.1.jolla .................................................................[done] There are some running programs that use files deleted by recent upgrade. You may wish to restart some of them. Run 'zypper ps' to list these programs. [root@Sailfish tmp]# zypper ps The following running processes use deleted files:

PID | PPID | UID | Login | Command | Service | Files
-----+------+--------+-------+----------------+---------+---------------------------------- 6973 | 4131 | 100000 | nemo | droidvdec3:src | | /usr/lib/libhunspell-1.2.so.0.0.0

You may wish to restart these processes. See 'man zypper' for information about the meaning of values in the above table.

I have broke the xulrunner package (since it depends on the old version of hunspell), this might be related to this?

[martonmiklos]

I have this version installed:

[root@Sailfish nemo]# zypper info maliit-plugin-presage
Loading repository data...
Reading installed packages...

Information for package maliit-plugin-presage:

Repository: @System
Name: maliit-plugin-presage
Version: 1.0+forget.20180315200947.2e5b021-10.24.1.jolla
Arch: armv7hl
Vendor: 
Installed: Yes
Status: up-to-date
Installed Size: 944.7 KiB
Summary: Maliit plugin for text predictions using Presage
Description: 
Keyboard prediction plugin based on the Presage prediction engine

But I cannot get the long tap working. It works as it would accept the solution. Do I need to tweak something on the keyboard QML side too?

[rinigus]

@martonmiklos - thank you very much for looking into it! I will address the issues that you have seen one-by-one below

hunspell

we don't need to install newer version, its just used for builds. However, as you experienced, its even dangerous - not that I was expecting it. So, to avoid users installing newer hunspell, I moved the package in OBS to my toolbox (https://build.merproject.org/project/show/home:rinigus:toolbox). That way, its used for building but is not a part of keyboard repository. So, users can install, as you did, via zypper and not get any problems with hunspell.

old presage uninstall

This is an expected behavior and its great that it works. Old presage-data packages are not compatible with the new presage-lang ones and its stated in https://github.com/sailfish-keyboard/sailfishos-presage-predictor/blob/master/rpm/maliit-plugin-presage.spec#L34 . So, its expected to uninstall the whole bunch of old packages.

long tap not working

That's supposed to work, no additional code is needed in QML. Its a part of this commit already (https://github.com/sailfish-keyboard/sailfishos-presage-predictor/pull/26/files#diff-f422a4d32c42d7b2544ea3e17c63f762R93 )

If it doesn't work, try:

• make an empty note
• choose english keyboard
• type Tutututu
• type space
• start typing tutu and see that "tutututu" is suggested
• delete the note text
• start typing "Tutu" and press and hold on suggested "Tutututu" until it disappears

Why it could not work: the few reasons I can come up with is

• word started with a char with accent. then lowercasing normalization doesn't work, we need unicode support
• word is still present in the text in the buffer and presage is re-learning it
• word is in read-only dictionary (legitimate word learned as a part of n-gram training and is in presage-lang package; or legitimate word suggested by hunspell)

If forget is still doesn't work, please describe how to reproduce it in English keyboard

[rinigus]

PS: please reinstall system hunspell on your device and remove my hunspell - don't want things to break

[rinigus]

@martonmiklos: are you still getting issues with the long tap? Or everything is OK?

@ljo: did you had a chance to test?

[martonmiklos]

Nope I have not get to the point to remove the new hunspell (which actually broke some things like the browser). I have forgotten the fact (again) that the hunspell is linked statically.

[rinigus]

@martonmiklos : you can use

zypper in -f hunspell-_versioninfo_

to get proper hunspell back. For 2.1.3.7, its

zypper in -f hunspell-1.2.8.5-1.1.7

[martonmiklos]

Managed to get back the hunspell, and the forgot functionality works just fine (the words what I wanted to clear was in the ngram database).

[rinigus]

Great to hear! Its confusing at first that you cannot get out the words from n-grams, but I think its OK as for design. Maybe we can later add some kind of blacklisted words as an additional way of forgetting them

[rinigus]

PS: So, its "just" the code review left before merge :)

[martonmiklos]

Great to hear! Its confusing at first that you cannot get out the words from n-grams, but I think its OK as for design. Maybe we can later add some kind of blacklisted words as an additional way of forgetting them

Yeah I have been thinking about the same that we should provide some solution for editing the ngram databases as well. We will have no resources for properly screening the input corpuses, so there will be a customer need to remove from those databases as well. Anyway what you made so far is great to ship, and we can think about improving it later.

[rinigus]

I will make the changes that you suggest (doc and default value) in 2 hours or a touch later. I should have added the default value, but just forgot. At first, I wanted to be sure that I knew where predict was called when I changed API and used compiler errors to keep me in control. Later, just forgot to add default :)

As for blocking/editing n-grams, we have to figure out the solution. Maybe its OK to clean it through user feedback via separate projects (one per language?) issues. That way we will strive towards better quality in long run.

[rinigus]

The force parameter is documented and the member variable removed - thanks for suggesting it! I had to make predict_impl as an implementation of predict method, since the slot was not very happy with accepting default value using the connection mechanism that I used (there were compiler errors).

Please review and, if all is fine, merge. :)

[rinigus]

Thank you very much! I haven't registered account on OpenRepos yet, will be able to work on it in 2-3 hours. So you, if you wish, you can beat me to that :)

We probably now need to write install docs and make a release. If we make proper release by changing RPM spec and tag it on github, I'll be able to compile official version on OBS with the correct version info.