D26342: Allow overriding to disable auto language detection

[Michel Ludwig]

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 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D26342

To: sdepiets, #frameworks, cullmann, mlaurent, mludwig
Cc: aacid, mludwig, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, 
ngraham, bruns

D26342: Allow overriding to disable auto language detection

[Michel Ludwig]

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 you and do my thing".
  
  
  Yep, that's right, and it won't even tell you which language it has detected. 
When the autodetection feature was introduced, it fundamentally changed the 
behaviour of Sonnet, breaking KatePart's spell checking in the process.
  
  > It seems to me that what would make sense is that the "use this language" 
is what sets autoDetectLanguageDisabled to false, or at least the function that 
should get a new overload saying setLanguage(¿qstring? language, bool 
disableAutoDetection) if we want to be sure not to change existing behaviour
  
  Ideally, the autodetection feature should be redesigned as it is only usable 
for simple text fields in its current form. KatePart requires a more 
sophisticated way of detecting languages(s).

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, michaelh, 
ngraham, bruns

D25495: Fix Sonnet autodetect failing on Indian langs

[Michel Ludwig]

mludwig added a comment.


  Thanks for the updated patch!
  
  In D25495#585855 , @waqar wrote:
  
  > > There is also a bug in GuessLanguagePrivate::guessFromTrigrams(const 
QString &sample, const QStringList &languages): if m_minConfidence is left to 
its default value of '0', that function will always return an empty list. I 
will propose a fix shortly.
  >
  > Alright, I am excited to hear.
  
  
  Here is my proposal:
  
  https://phabricator.kde.org/D26346
  
  > 
  > 
  >> The real issue behind Bug 176537 is a different one, however. On-the-fly 
spell checking in Kate(Part) will only check one line at a time, potentially 
not providing enough text for a meaningful language detection.
  > 
  > To be honest, I haven't ever had an issue with that. I mostly test on 
QOwnNotes, and spellchecking works the same way there i.e., one line at a time. 
If there is a dictionary present, sonnet will guess the language correctly most 
of the times. But you are right in that,..more text would enable sonnet to be 
more accurate. However, autodetection works on a sentence basis, and sentences 
can sometimes be quite short.
  
  With your patch, language detection works much better in KatePart. However, 
due to way Sonnet is used in KatePart for on-the-fly spell checking, one issue 
still is that every word is basically checked against every dictionary. So, in 
the sentence "English is an interesting langage", "langage" may not be 
considered to be misspelled since "langage" is a correctly spelled French word, 
for example. I will still work on that.

INLINE COMMENTS

> guesslanguage.cpp:589
> +//if guessing from trigrams fail
> +if (candidateLanguages.isEmpty() && !scriptsList.isEmpty()) {
> +for (const QChar::Script script : scriptsList) {

Couldn't this if-statement can be dropped? I guess one can argue that sometimes 
there may be a language without trigrams that would even be a better language 
guess?

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D25495

To: waqar, mludwig, cullmann
Cc: ognarb, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26346: Sonnet: Fix returning from GuessLanguagePrivate::guessFromTrigrams

[Michel Ludwig]

mludwig updated this revision to Diff 72552.
mludwig added a comment.


  Small fix to the documentation.

REPOSITORY
  R246 Sonnet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26346?vs=72551&id=72552

REVISION DETAIL
  https://phabricator.kde.org/D26346

AFFECTED FILES
  src/core/guesslanguage.cpp
  src/core/guesslanguage.h

To: mludwig, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26346: Sonnet: Fix returning from GuessLanguagePrivate::guessFromTrigrams

[Michel Ludwig]

mludwig created this revision.
mludwig added a reviewer: Frameworks.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
mludwig requested review of this revision.

REVISION SUMMARY
  The documentation states that "Default is returning best guess without caring 
about confidence - exactly as after call to setLimits(1,0)."
  
  However, for the default value m_minConfidence = 0, 
GuessLanguagePrivate::guessFromTrigrams always returns an empty list.
  
  This patch is an attempt at fixing that.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D26346

AFFECTED FILES
  src/core/guesslanguage.cpp
  src/core/guesslanguage.h

To: mludwig, #frameworks
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26342: Allow overriding to disable auto language detection

[Michel Ludwig]

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: mludwig, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25495: Fix Sonnet autodetect failing on Indian langs

[Michel Ludwig]

mludwig added a comment.


  You are right that in GuessLanguage::identify(const QString &text, const 
QStringList &suggestionsListIn), if a language without trigrams is not present 
in 'suggestionsListIn', it will never be 'guessed'.
  
  I'd suggest to move your changes to GuessLanguage::identify(const QString 
&text, const QStringList &suggestionsListIn) after the call to 
d->identify(text, d->findRuns(text)); but only add those languages for which 
there is a dictionary AND which don't have trigrams.
  
  There is also a bug in GuessLanguagePrivate::guessFromTrigrams(const QString 
&sample, const QStringList &languages): if m_minConfidence is left to its 
default value of '0', that function will always return an empty list. I will 
propose a fix shortly.
  
  The real issue behind Bug 176537 is a different one, however. On-the-fly 
spell checking in Kate(Part) will only check one line at a time, potentially 
not providing enough text for a meaningful language detection. I plan to 
perform the language detection inside KatePart, so that there is also feedback 
regading the detected language that is shown to the user, who can then also 
override the detected language, if desired.

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D25495

To: waqar, mludwig, cullmann
Cc: ognarb, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26321: Expose show-line-count in the ConfigInterface

[Michel Ludwig]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R39:3026a9caa868: Add "line-count" property to the 
ConfigInterface (authored by mludwig).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D26321?vs=72484&id=72518#toc

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26321?vs=72484&id=72518

REVISION DETAIL
  https://phabricator.kde.org/D26321

AFFECTED FILES
  autotests/src/configinterface_test.cpp
  src/include/ktexteditor/configinterface.h
  src/utils/kateconfig.h
  src/view/kateview.cpp

To: mludwig, #ktexteditor, cullmann, dhaumann
Cc: dhaumann, cullmann, kwrite-devel, kde-frameworks-devel, cent, LeGast00n, 
szutmael, GB_2, domson, michaelh, ngraham, bruns, demsking, head7, kfunk, sars

D26321: Expose show-line-count in the ConfigInterface

[Michel Ludwig]

mludwig added a comment.


  In D26321#585381 , @cullmann wrote:
  
  > I am ok with this , but in the long run, we want to change the behavior of 
the view's config interface to just dispatch to the internal config interface 
like for the document.
  >  You missed to add the line-count to KTextEditor::ViewPrivate::configKeys()
  
  
  Good catch :) I've added it now.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D26321

To: mludwig, #ktexteditor, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, cent, LeGast00n, szutmael, 
GB_2, domson, michaelh, ngraham, bruns, demsking, head7, kfunk, sars, dhaumann

D26321: Expose show-line-count in the ConfigInterface

[Michel Ludwig]

mludwig updated this revision to Diff 72484.
mludwig added a comment.


  Add "line-count" to KTextEditor::ViewPrivate::configKeys()

REPOSITORY
  R39 KTextEditor

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26321?vs=72445&id=72484

REVISION DETAIL
  https://phabricator.kde.org/D26321

AFFECTED FILES
  autotests/src/configinterface_test.cpp
  src/include/ktexteditor/configinterface.h
  src/utils/kateconfig.h
  src/view/kateview.cpp

To: mludwig, #ktexteditor, cullmann
Cc: cullmann, kwrite-devel, kde-frameworks-devel, cent, LeGast00n, szutmael, 
GB_2, domson, michaelh, ngraham, bruns, demsking, head7, kfunk, sars, dhaumann

D26321: Expose show-line-count in the ConfigInterface

[Michel Ludwig]

mludwig created this revision.
mludwig added a reviewer: KTextEditor.
mludwig added a project: KTextEditor.
Herald added projects: Kate, Frameworks.
Herald added subscribers: kde-frameworks-devel, kwrite-devel.
mludwig requested review of this revision.

REVISION SUMMARY
  The show-line-count property is added to the ConfigInterface.
  
  As Kile doesn't use KatePart's status bar, it needs a way to query whether 
the line count should be shown or not, so that Kile can show the line count in 
its own status bar.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D26321

AFFECTED FILES
  autotests/src/configinterface_test.cpp
  src/include/ktexteditor/configinterface.h
  src/utils/kateconfig.h
  src/view/kateview.cpp

To: mludwig, #ktexteditor
Cc: kwrite-devel, kde-frameworks-devel, cent, LeGast00n, szutmael, GB_2, 
domson, michaelh, ngraham, bruns, demsking, head7, cullmann, kfunk, sars, 
dhaumann

D25244: Implement ISpellChecker backend for Windows >= 8

[Michel Ludwig]

mludwig added a comment.


  I've tried it out and works just fine! :)

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D25244

To: cullmann, #frameworks, vonreth
Cc: mludwig, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D25244: Implement ISpellChecker backend for Windows >= 8

[Michel Ludwig]

mludwig added a comment.


  In D25244#562817 , @cullmann wrote:
  
  > Some feedback?
  >  Else I would just try that.
  >  For me it worked fine (obviously).
  
  
  I will try it out over the weekend. It's something I planned to work on 
myself, but you were faster ;)

REPOSITORY
  R246 Sonnet

REVISION DETAIL
  https://phabricator.kde.org/D25244

To: cullmann, #frameworks, vonreth
Cc: mludwig, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D17310: Use a Kile Icon that is similar to the original

[Michel Ludwig]

mludwig accepted this revision.

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D17310

To: trickyricky26, #vdg, ngraham, #kile, mludwig
Cc: alex-l, loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, 
bruns

D17310: Use a Kile Icon that is similar to the original

[Michel Ludwig]

mludwig added a comment.


  In D17310#387589 , @trickyricky26 
wrote:
  
  > - Make Kile letters a lighter color
  
  
  Yep, thanks. That's a great icon now - I like it a lot!

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D17310

To: trickyricky26, #vdg, ngraham, #kile
Cc: alex-l, loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, 
bruns

D17310: Use a Kile Icon that is similar to the original

[Michel Ludwig]

mludwig added a comment.


  In D17310#387633 , @alex-l wrote:
  
  > I'm the author of this one:
  >
  > F6530055: immagine.png 
  >
  > That has been in Breeze for years. Please next time don't wait so much to 
complain about an icon that being the app launcher one can be easily noticed 
when updated.
  
  
  Please don't override the icons of applications without asking whether the 
developers are ok with it!

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D17310

To: trickyricky26, #vdg, ngraham, #kile
Cc: alex-l, loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, 
bruns

D17310: Improve Kile icon with LaTeX font

[Michel Ludwig]

mludwig added a comment.


  In D17310#385180 , @trickyricky26 
wrote:
  
  > Here are some designs that are more faithful to the original:
  >
  > F6520878: kile-faithful.svg.png 
  >  This has almost the same letter design as the original. I adjusted it a 
bit to fit onto the notepad.
  >
  > @mludwig 
  >  What do you think of these designs? Are they close enough to the original 
to be recognizable?
  
  
  Yes, I like this version better, thanks. What about giving a blue foreground 
to the notebook and use white for the text?

REPOSITORY
  R266 Breeze Icons

REVISION DETAIL
  https://phabricator.kde.org/D17310

To: trickyricky26, #vdg, ngraham, #kile
Cc: loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17310: Improve Kile icon with LaTeX font

[Michel Ludwig]

mludwig added a comment.


  In D17310#383350 , @ngraham wrote:
  
  > #kile  folks, are you good with this 
now? If we don't hear back in a week or two, we'll have to assume yes. :)
  
  
  Sorry, but I'm still not a fan of this icon.
  
  Moreover, as you pointed out in https://phabricator.kde.org/T10243, the 
"Don't destroy the app's existing branding" recommendation in the HIG has 
actually been violated by the previous version of this icon already.

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D17310

To: trickyricky26, #vdg, ngraham, #kile
Cc: loh.tar, mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17310: Improve Kile icon with LaTeX font

[Michel Ludwig]

mludwig added a comment.


  In D17310#370758 , @ngraham wrote:
  
  > I would not recommend embedding the app name into the icon itself. The text 
isn't localizable, the icon would become really cramped, and in any event it 
would look weird since no other app does this.
  >
  > For those reasons, our HIG specifically recommends against this: 
https://hig.kde.org/style/icon.html?highlight=icons#general-guidelines
  
  
  Well, it's not really arbitrary text, but the name of the application, so I 
it wouldn't be localized much. Besides, Kile's official icons have contained 
the character "K" or the string "Kile" for over 15 years.
  
  F6453199: kile_big.png  F6453202: 
favicon.ico 
  
  Off the top of my head, another example of such an icon design is the one of 
Evince , which contains a stylized letter 
'e' - I know it's from the competition ;)
  
  Or to some extent, the icon of kcharselect, which I really like.
  
  The biggest issue that I have with the square root icon is that it doesn't 
symbolize enough that it's the icon of an application. To me, it just looks 
like the application of a file type. Some dynamic element is missing, like a 
pencil, for example, to indicate that it's an application for editing such 
files.

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D17310

To: trickyricky26, #vdg, ngraham, #kile
Cc: mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D17310: Improve Kile icon with LaTeX font

[Michel Ludwig]

mludwig added a comment.


  Hey, thanks for working on this.
  
  I don't mind the change in font style.
  
  However, do you think it's possible to add "Kile" somewhere onto the icon as 
well, preferably quite largely. With this icon, I always find it hard to locate 
Kile in the task manager.

REPOSITORY
  R266 Breeze Icons

BRANCH
  improve-kile-icon (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D17310

To: trickyricky26, #vdg, ngraham, #kile
Cc: mludwig, ndavis, ngraham, kde-frameworks-devel, michaelh, bruns

D13442: Implemented displaying of total lines in kate

[Michel Ludwig]

mludwig added a comment.


  If you decide to add a config option, it would be great if you could add a 
programmatic way to inform applications that don't use the default status bar 
about changes regarding that config option.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D13442

To: shubham, #ktexteditor, cullmann, brauch
Cc: mludwig, zhigalin, ngraham, dhaumann, kwrite-devel, kde-frameworks-devel, 
michaelh, kevinapavew, bruns, demsking, head7, cullmann, kfunk, sars

D11838: Turn on line numbers by default

[Michel Ludwig]

mludwig added a comment.


  In D11838#238728 , @dhaumann wrote:
  
  > Personally, I dislike the idea of passing options in createView for a very 
simple reason: we have document variables, .kateconfig files, settings on the 
UI, the command line, the ConfigInterface. Adding yet another way to configure 
KTextEditor sounds like a bad idea.
  
  
  All these mechanisms allow to change "current" config settings, but not their 
default values, as far as I can see. Passing options to createView would also 
allow it, e.g., to control which actions should be created. Kile has to delete 
some actions after creating a view at the moment, which could be avoided. But 
that's another discussion.
  
  > If at all, the only clear way is to let an application decide to not share 
settings with other KTextEditor applications. E.g. 
KTextEditor::Editor::setUseSharedConfig(bool share).
  
  I support this idea, but one should maybe also think about whether some 
settings could be better read/saved using read/writeSessionConfig. 
"showLineNumbers" could fall into that category.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D11838

To: ngraham, #kate, #ktexteditor, dhaumann, mludwig
Cc: aacid, acrouthamel, brauch, mludwig, kfunk, dhaumann, #frameworks, 
michaelh, kevinapavew, ngraham, demsking, cullmann, sars

D11838: Turn on line numbers by default

[Michel Ludwig]

mludwig added a comment.


  In D11838#238269 , @ngraham wrote:
  
  > So what's our path forward here? Is there a way to only turn on line 
numbers for Kate itself, and not for the katepart?
  
  
  One suggestion would be to extend the method for creating new views in 
KTextEditor::Document 

 by adding a parameter for specifying default config values.
  
  For instance, you could create a new interface for KTextEditor::Document that 
contains the method
  
virtual View* createView(QWidget *parent, const QLinkedList>& defaultConfigList, KTextEditor::MainWindow *mainWindow = nullptr) = 0
  
  The method would set the values given in 'defaultConfigList' as default 
config values when creating a view.
  
  Kile could then create its views as follows:
  
QLinkedList defaultConfigList;
defaultConfigList << qMakePair("ShowLineNumbers", false);
KTextEditor::View *view = document->createView(parent, defaultConfigList);
  
  (also see KTextEditor::ConfigInterface 

 for an example of how to create interfaces)

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D11838

To: ngraham, #kate, #ktexteditor, dhaumann, mludwig
Cc: brauch, mludwig, kfunk, dhaumann, #frameworks, michaelh, kevinapavew, 
ngraham, demsking, cullmann, sars

D11838: Turn on line numbers by default

[Michel Ludwig]

mludwig added a comment.


  In D11838#238269 , @ngraham wrote:
  
  > That said, I'm still struggling to understand what the problems is with 
line numbers in Kile. Most of the editors listed at 
https://beebom.com/best-latex-editors/ have line numbers on by default, 
including TeXmaker, which is what all of my scientist friends use. Screens are 
wider than ever before, and average users are going to just maximize the window 
anyway, making the small amount of horizontal space taken up by the line 
numbers a non-issue.
  
  
  On that website four screenshots out of eight show line numbers. And 
especially in TeXmaker's case, I wouldn't call the amount of space needed for 
line numbers small.
  
  The point is that line numbers do not bring much added value when editing 
LaTeX documents. To reduce clutter on Kile's user interface, line numbers 
should not be shown by default. It's not just a question of horizontal screen 
space, but also of usefulness/usability.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D11838

To: ngraham, #kate, #ktexteditor, dhaumann, mludwig
Cc: brauch, mludwig, kfunk, dhaumann, #frameworks, michaelh, kevinapavew, 
ngraham, demsking, cullmann, sars

D11838: Turn on line numbers by default

[Michel Ludwig]

mludwig requested changes to this revision.
mludwig added a comment.
This revision now requires changes to proceed.


  I don't think that this is ok for Kile.
  
  KatePart is an editor component that needs to fit many different contexts. 
For Kile, I don't see a need to have line numbers switched on by default.
  
  However, it the *default* setting for showing line numbers is made 
configurable, then this change is fine by me.

REPOSITORY
  R39 KTextEditor

REVISION DETAIL
  https://phabricator.kde.org/D11838

To: ngraham, #kate, #ktexteditor, dhaumann, mludwig
Cc: mludwig, kfunk, dhaumann, #frameworks, michaelh, kevinapavew, ngraham, 
demsking, cullmann, sars

Re: KTabWidget vs QTabWidget

[Michel Ludwig]

On 09/25/2015 01:57 AM, Jeremy Whiting wrote:

Michel,

Would you mind if I move the DropWidget you wrote for kile into
kwidgetsaddons and give it a name like KDropWidget and relicense it to
LGPL v2+ ?


That's totally fine with me.

Michel
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel