Skip to content

Remove text prediction feature for now#146

Merged
dobey merged 1 commit into
maliit:masterfrom
dobey:drop-presage
Mar 13, 2024
Merged

Remove text prediction feature for now#146
dobey merged 1 commit into
maliit:masterfrom
dobey:drop-presage

Conversation

@dobey

@dobey dobey commented Jul 12, 2022

Copy link
Copy Markdown
Contributor

As presage has been unmaintained since 2018, among various other concerns with
the feature and how the ngram databases are created, remove text prediction for
now until a better implementation can be done, after keyboard layouts support
has been refactored to be much simpler.

@dobey

dobey commented Jul 26, 2022

Copy link
Copy Markdown
Contributor Author

Also, maybe @sunweaver might have an opinion on this, as he's maintaining the packages in Debian for maliit now. Would be good for some feedback there.

@sunweaver

Copy link
Copy Markdown
Contributor

As we want to replace lomiri-keyboard by maliit-keyboard in Ubuntu Touch 22.04, this feature will probably dearly be missed. For Debian, it is not so relevant, because people will never have used the presage feature in a Debian stable release.

Also, presage is in maintenance-only mode in Debian, as well: https://metadata.ftp-master.debian.org/changelogs//main/p/presage/presage_0.9.1-2.2_changelog

@sunweaver

Copy link
Copy Markdown
Contributor

@mariogrip @peat-psuwit ^^^

@dobey

dobey commented Jul 26, 2022

Copy link
Copy Markdown
Contributor Author

As we want to replace lomiri-keyboard by maliit-keyboard in Ubuntu Touch 22.04, this feature will probably dearly be missed. For Debian, it is not so relevant, because people will never have used the presage feature in a Debian stable release.

It worked quite poorly and it is something people commonly turn off as soon as they start using Ubuntu Touch. However, word completion with spell checking via hunspell is still working fine, if the dictionary for the language is installed.

@evelikov

Copy link
Copy Markdown

Out of curiosity: are there any blockers for removing the presage/prediction databases?

Glancing at my native tongue - Bulgarian - the prediction is very poor or useless even ☹️ Skimming around - Bulgarian and a few other languages have the database_$(lang).db in-tree...

Which as a whole make me wonder if it's actually being maintained?

Thanks o/

@evelikov

Copy link
Copy Markdown

Even though we ship the SteamDeck with maliit-keyboard, I had to remove the presage integration due to the size and poor UX.

@dobey what's your take on this? Are you split on the topic or waiting for feedback from Debian/Ubuntu folks?

@mikhas

mikhas commented Jan 17, 2024

Copy link
Copy Markdown
Member

When we first implemented word candidates it was always assumed that better (free!) prediction engines would materialize eventually. I guess that didn't happen so then the whole feature should be dropped...

@evelikov

Copy link
Copy Markdown

@dobey any chance we can have this merged and a few release please? The latest release was out 1.5 years ago and you've merged a bunch of cool fixes since then.

Thanks in advance, also hope you're OK 🤞

KAMiKAZOW
KAMiKAZOW previously approved these changes Feb 29, 2024
@KAMiKAZOW

Copy link
Copy Markdown
Member

I do have a merge button now but I these changes are above my skill level to actually judge. I would need confirmation of eg. @evelikov that this still builds and breaks nothing else. I'm also not an owner of this organization, so I cannot elevate eg. someone from the SteamOS developers to (hopefully) maintain Maliit.

@evelikov

evelikov commented Mar 2, 2024

Copy link
Copy Markdown

Hey @KAMiKAZOW thanks for chipping in.

At a glance, the PR needs a small update to drop some now unused files - I can do that alongside some testing.

That said, I'm not sure what you mean with "to elevate"? If the project is no longer maintained, the README should really be updated to reflect that.

I cannot comment on things from SteamDeck POV, but as an Arch user this PR would be appreciated.

@sunweaver

Copy link
Copy Markdown
Contributor

@dobey please let us know if you are still upstream for maliit-keyboard + framework. Personally, I have been in touch with Rodney on Maliit issues in Debian + Ubuntu. So, he is around and also using Maliit.

@KAMiKAZOW

Copy link
Copy Markdown
Member

That said, I'm not sure what you mean with "to elevate"? If the project is no longer maintained, the README should really be updated to reflect that.

To clarify: I have commit access because I write reasonable release notes when someone of the actual developers prepares a release. I'm not and never been in regular contact with any of them.

@dobey

dobey commented Mar 5, 2024

Copy link
Copy Markdown
Contributor Author

At a glance, the PR needs a small update to drop some now unused files - I can do that alongside some testing.

What do you mean by this @evelikov ? I'm pretty sure I removed all the related files for this here.

@evelikov

evelikov commented Mar 6, 2024

Copy link
Copy Markdown

At a glance, the PR needs a small update to drop some now unused files - I can do that alongside some testing.

What do you mean by this @evelikov ? I'm pretty sure I removed all the related files for this here.

My initial feeling was that spellpredictworker.cpp and spellpredictworker.h should go

I was off - so is the current code btw. Currently we disable hunspell/spell checker, when presage is off 😱

I think the following files need minor work:

  • .github/workflows/main.yaml - don't install the dev package
  • src/lib/logic/wordengine.cpp - drop Presage references (in comments)
  • tests/autopilot/lomiri_keyboard/tests/test_keyboard.py - drop presage tests
  • tests/unittests/common/wordengineprobe.cpp - drop Presage references (in comments)
  • tests/unittests/ut_editor/wordengineprobe.cpp - as above
  • tests/unittests/ut_preedit-string/ut_preedit-string.cpp - as above
  • tests/unittests/ut_word-candidates/wordengineprobe.cpp - as above

Will prep an updated PR later today, unless someone beats me to it.

As presage has been unmaintained since 2018, among various other concerns with
the feature and how the ngram databases are created, remove text prediction for
now until a better implementation can be done, after keyboard layouts support
has been refactored to be much simpler.
@dobey

dobey commented Mar 7, 2024

Copy link
Copy Markdown
Contributor Author

Currently we disable hunspell/spell checker, when presage is off 😱

I don't see anything suggesting so in the code. They are separate settings, and separate properties in language plugins. I've also tested the change fairly heavily with hunspell still working.

Will prep an updated PR later today, unless someone beats me to it.

These references were all ineffectual, but I've removed them in this PR now as well.

@evelikov

evelikov commented Mar 8, 2024

Copy link
Copy Markdown

I don't see anything suggesting so in the code. They are separate settings, and separate properties in language plugins. I've also tested the change fairly heavily with hunspell still working.

I meant that the existing code (aka without this PR) disables hunspell when built without presage... as we can see from the changes to westernlanguagesplugin.cpp and meson.build in this PR.

Although I could be missing something 🤷

In either way, with this PR merged we don't need to care about any of that. Thanks again 🙇

@dobey dobey requested a review from KAMiKAZOW March 11, 2024 19:36
@dobey dobey merged commit 9b64784 into maliit:master Mar 13, 2024
@dobey dobey deleted the drop-presage branch March 13, 2024 22:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants