Sonnet: fix wrong language for suggestions in mixed-language texts
ClosedPublic

Authored by dfaure on Dec 23 2017, 7:34 PM.

Details

Summary

When AutoDetectLanguage is set, the same document can contain multiple
languages. In that case, Highlighter stores the language information
into the block's userdata, and changes the spellchecker language at
every change. So the suggestions were using whatever was the language
of the last block that was checked, which led to pretty random behaviour
for the user.

Fixed by using a QTextCursor to retrieve the cached language for the
word we're showing suggestions for.

Includes a separate commit for Loader:

honour Settings::defaultClient(), useful for unittests.
Test Plan

Spellchecking in kmail composer, writing text in French
and having a signature with word in English.

Diff Detail

Repository
R246 Sonnet
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
dfaure created this revision.Dec 23 2017, 7:34 PM
Restricted Application added a project: Frameworks. · View Herald TranscriptDec 23 2017, 7:34 PM
dfaure requested review of this revision.Dec 23 2017, 7:34 PM
mlaurent accepted this revision.Dec 24 2017, 8:49 AM

Good thanks !

This revision is now accepted and ready to land.Dec 24 2017, 8:49 AM
dhaumann accepted this revision.Dec 24 2017, 11:59 AM
dhaumann added a subscriber: dhaumann.

Good enough :)

src/ui/highlighter.cpp
487 ↗(On Diff #24337)

I am not a big fan of -1 in this use case. What about -2 etc, or even 0? So this API easily leads to unexpected results.

But ok, my comment is unrelated to your patch.

dfaure added inline comments.Dec 24 2017, 12:28 PM
src/ui/highlighter.h
134 ↗(On Diff #24337)

The -1 special value is documented here, but yeah, special values are bad.

In practice SpellCheckDecorator calls this with the default value anyway.

And I guess any GUI will always want to limit this, you don't really want to offer 300 suggestions to the user...

So maybe we could remove the -1 special value (in the new method) anyway.

dhaumann added inline comments.Dec 24 2017, 7:23 PM
src/ui/highlighter.h
134 ↗(On Diff #24337)

Of course I am aware of that - that's why I wrote this is rather unrelated to your patch (or let's say it is in accordance with the existing API).

I don't request any changes here, so if at all, you could add a
// TODO: KF6: all values <= 0 will not truncate the suggestion list.

Please push :-)

Ah well surely we can test for <=0 already right now. It can't break any user of the code, since -2 (and further down) would have led to an empty list, which is pretty pointless.

This revision was automatically updated to reflect the committed changes.