D26342: Allow overriding to disable auto language detection

sdepiets added a comment. Please see below proposal to restore the previous default behavior https://invent.kde.org/frameworks/sonnet/-/merge_requests/1 REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D26342 To: sdepiets, #frameworks, cullmann, mlaurent, mludwig,

D26342: Allow overriding to disable auto language detection

dfaure added a comment. In D26342#675180 , @aacid wrote: > In D26342#675164 , @sdepiets wrote: > > > I don't think that's a regression, in the previous behavior you could try to set any language to

D26342: Allow overriding to disable auto language detection

aacid added a comment. In D26342#675164 , @sdepiets wrote: > In D26342#675142 , @dfaure wrote: > > > This actually breaks language auto-detection for me in the KMail composer. > > > > Testcase:

D26342: Allow overriding to disable auto language detection

sdepiets added a comment. In D26342#675142 , @dfaure wrote: > This actually breaks language auto-detection for me in the KMail composer. > > Testcase: > > - New Mail > - I type "Bonjour," in the body > > Before: It's detected as

D26342: Allow overriding to disable auto language detection

dfaure added a comment. This actually breaks language auto-detection for me in the KMail composer. Testcase: - New Mail - I type "Bonjour," in the body Before: It's detected as French and not underlined as a typo After: The language remains English, and the word is

D26342: Allow overriding to disable auto language detection

This revision was automatically updated to reflect the committed changes. Closed by commit R246:422fac8a6f92: Allow overriding to disable auto language detection (authored by sdepiets, committed by aacid). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D26342?vs=81177=81805#toc

D26342: Allow overriding to disable auto language detection

aacid accepted this revision. aacid added a comment. This revision is now accepted and ready to land. Noone seems to really disagree. Let's merge it. @sdepiets I hope this doesn't break everything ;) REPOSITORY R246 Sonnet BRANCH master REVISION DETAIL

D26342: Allow overriding to disable auto language detection

sdepiets updated this revision to Diff 81177. sdepiets added a comment. Bump since + autotest checks the language as well REPOSITORY R246 Sonnet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26342?vs=81176=81177 BRANCH master REVISION DETAIL

D26342: Allow overriding to disable auto language detection

aacid added a comment. You need to update the \since markers? After that i guess you can commit unless anyone really disagrees? @mludwig @cullmann ? REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D26342 To: sdepiets, #frameworks, cullmann, mlaurent, mludwig

D26342: Allow overriding to disable auto language detection

sdepiets updated this revision to Diff 81176. sdepiets added a comment. - Merge branch 'master' of git.kde.org:sonnet REPOSITORY R246 Sonnet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26342?vs=76992=81176 BRANCH master REVISION DETAIL https://phabricator.kde.org/D26342

D26342: Allow overriding to disable auto language detection

aacid added a comment. In D26342#632842 , @sdepiets wrote: > In D26342#623729 , @aacid wrote: > > > Would it make sense to enshrine this behaviour with an autotest? > > > Sorry what do you mean

D26342: Allow overriding to disable auto language detection

sdepiets added a comment. In D26342#623729 , @aacid wrote: > Would it make sense to enshrine this behaviour with an autotest? Sorry what do you mean by that? Similar to tests/BackgroundTest.cpp ? REPOSITORY R246 Sonnet REVISION DETAIL

D26342: Allow overriding to disable auto language detection

aacid added a comment. Would it make sense to enshrine this behaviour with an autotest? REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D26342 To: sdepiets, #frameworks, cullmann, mlaurent, mludwig Cc: aacid, mludwig, kde-frameworks-devel, LeGast00n, cblack, GB_2,

D26342: Allow overriding to disable auto language detection

sdepiets added a comment. I don't see a use case where you would want to set the Language, but then let the system override it (and only if the user has auto-detection). I left the option in the API to "setAutoDetectLanguageDisabled" to false, for instance if you are not sure which

D26342: Allow overriding to disable auto language detection

aacid added inline comments. INLINE COMMENTS > backgroundchecker.cpp:147 > { > +d->autoDetectLanguageDisabled = true; > d->currentDict = speller; question from someone that has never really used this setSpeller API. Is this behaviour change for sure 100% of the times wanted? If not

D26342: Allow overriding to disable auto language detection

sdepiets updated this revision to Diff 76992. sdepiets added a comment. Disable autodetect on language change REPOSITORY R246 Sonnet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26342?vs=74851=76992 BRANCH master REVISION DETAIL https://phabricator.kde.org/D26342

D26342: Allow overriding to disable auto language detection

mludwig added a comment. @Simon: If you add a call 'setAutoDetectLanguageDisabled(false)' to 'BackgroundChecker::changeLanguage' and 'BackgroundChecker::setSpeller', you should be able to take Albert's suggestion into account. For the Highlighter it should be similar. REPOSITORY R246

D26342: Allow overriding to disable auto language detection

mludwig added a comment. In D26342#619785 , @aacid wrote: > without having used Sonnet much, this seems the wrong API to me. > > Are you saying that it can happen that you tell Sonnet "use this language" and it goes and say "nah i'll ignore

D26342: Allow overriding to disable auto language detection

aacid added a comment. without having used Sonnet much, this seems the wrong API to me. Are you saying that it can happen that you tell Sonnet "use this language" and it goes and say "nah i'll ignore you and do my thing". It seems to me that what would make sense is that the "use

D26342: Allow overriding to disable auto language detection

sdepiets added a reviewer: mludwig. sdepiets added a comment. Can we get some traction on this? It's a relatively light change that doesn't affect the default Sonnet behavior. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D26342 To: sdepiets, #frameworks,

D26342: Allow overriding to disable auto language detection

cullmann added a comment. @mludwig: ok with the current patch? REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D26342 To: sdepiets, #frameworks, cullmann, mlaurent Cc: mludwig, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D26342: Allow overriding to disable auto language detection

sdepiets added reviewers: cullmann, mlaurent. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D26342 To: sdepiets, #frameworks, cullmann, mlaurent Cc: mludwig, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26342: Allow overriding to disable auto language detection

sdepiets added a comment. In D26342#585867 , @mludwig wrote: > I'd support such a change for the BackgroundChecker as well. KTextEditor needs a way to do its own language detection or override the detected language. I've updated the diff

D26342: Allow overriding to disable auto language detection

sdepiets updated this revision to Diff 74851. sdepiets added a comment. Allow overriding to disable auto language detection in BackGround Checker REPOSITORY R246 Sonnet CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26342?vs=72540=74851 BRANCH master REVISION DETAIL

D26342: Allow overriding to disable auto language detection

mludwig added a comment. I'd support such a change for the BackgroundChecker as well. KTextEditor needs a way to do its own language detection or override the detected language. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D26342 To: sdepiets, #frameworks Cc:

D26342: Allow overriding to disable auto language detection

sdepiets edited the summary of this revision. sdepiets added a reviewer: Frameworks. REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D26342 To: sdepiets, #frameworks Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26342: Allow overriding to disable auto language detection

sdepiets created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. sdepiets requested review of this revision. REPOSITORY R246 Sonnet BRANCH master REVISION DETAIL https://phabricator.kde.org/D26342 AFFECTED FILES src/ui/highlighter.cpp