With your change, we are not allowed anymore to change for example only font and keeping different size, style for each kind of font (window, menu, toolbar...), right ?

In D27808#624506, @bport wrote:

With your change, we are not allowed anymore to change for example only font and keeping different size, style for each kind of font (window, menu, toolbar...), right ?

IIUC, that is a feature of KFontDialog (from KDELibs4Support), not available in QFontDialog.

I probably should mention that in the commit message.

Porting with such feature regression is hard to justify.

Perhaps we should port KFontDialog (it is ~150 lines of cpp) to kwidgetsaddons instead, KFontChooser is already there.
Or better perhaps implement here a simple QDialog embedding KFontChooser and providing equivalent features needed here that KFontDialog has.
This is a typical work we did for KF5 https://community.kde.org/Frameworks/Porting_Notes

In D27808#625052, @meven wrote:

Porting with such feature regression is hard to justify.

I honestly don't know how many users actually use that feature; if it's still widely used, then yeah, that would definitely be a feature regression.

Perhaps we should port KFontDialog (it is ~150 lines of cpp) to kwidgetsaddons instead, KFontChooser is already there.
Or better perhaps implement here a simple QDialog embedding KFontChooser and providing equivalent features needed here that KFontDialog has.
This is a typical work we did for KF5 https://community.kde.org/Frameworks/Porting_Notes

Since, AFAICS, only the fonts KCM uses that functionality, it might be better to move/copy KFontDialog to the KCM, and maybe rename it; it'd be a helper class of sorts.

And another question, "General", "Menu", "Toolbar", how many users actually set these three to individual font families/styles/sizes?

In D27808#625181, @ahmadsamir wrote:

And another question, "General", "Menu", "Toolbar", how many users actually set these three to individual font families/styles/sizes?

If anything this is just not relevant here. Either you port things away from kde4support or you change the behavior but the two things should not be done together.
Generally the community has rarely removed any working features and this is a kind of feature plasma is known for : customization.

In D27808#625064, @ahmadsamir wrote:

In D27808#625052, @meven wrote:

Porting with such feature regression is hard to justify.

I honestly don't know how many users actually use that feature; if it's still widely used, then yeah, that would definitely be a feature regression.

Perhaps we should port KFontDialog (it is ~150 lines of cpp) to kwidgetsaddons instead, KFontChooser is already there.
Or better perhaps implement here a simple QDialog embedding KFontChooser and providing equivalent features needed here that KFontDialog has.
This is a typical work we did for KF5 https://community.kde.org/Frameworks/Porting_Notes

Since, AFAICS, only the fonts KCM uses that functionality, it might be better to move/copy KFontDialog to the KCM, and maybe rename it; it'd be a helper class of sorts.

Moving KFontDialog would imply porting it to QDialog anyway.
But license might get in the way since kdelibs4support is LGPL and plasma is GPL.
It would mean the moved KFontDialog would stay LGPL in a GPL codebase. Which is possible but not recommendable at all.

> implement here a simple QDialog embedding KFontChooser and providing equivalent features needed here that KFontDialog has.

I would suggest you this simpler way to do it, but the code of this QDialog helper would be loosely inspired by KFontDialog anyway but not LGPL.
In the end that's pretty much the same thing as you suggested but better keep license potential issues at bay ;)

In D27808#625186, @meven wrote:

In D27808#625181, @ahmadsamir wrote:

And another question, "General", "Menu", "Toolbar", how many users actually set these three to individual font families/styles/sizes?

If anything this is just not relevant here. Either you port things away from kde4support or you change the behavior but the two things should not be done together.
Generally the community has rarely removed any working features and this is a kind of feature plasma is known for : customization.

That wasn't my intention (I mean changing it in this diff), it was a BTW kind of thought/question "is this still needed/used nowadays?", I was thinking of posting the question to the frameworks ML... but was afraid it might start a flame/bike-shedding conundrum there, so I asked here instead :)

In D27808#625064, @ahmadsamir wrote:

In D27808#625052, @meven wrote:

Porting with such feature regression is hard to justify.

I honestly don't know how many users actually use that feature; if it's still widely used, then yeah, that would definitely be a feature regression.

Perhaps we should port KFontDialog (it is ~150 lines of cpp) to kwidgetsaddons instead, KFontChooser is already there.
Or better perhaps implement here a simple QDialog embedding KFontChooser and providing equivalent features needed here that KFontDialog has.
This is a typical work we did for KF5 https://community.kde.org/Frameworks/Porting_Notes

Since, AFAICS, only the fonts KCM uses that functionality, it might be better to move/copy KFontDialog to the KCM, and maybe rename it; it'd be a helper class of sorts.

Moving KFontDialog would imply porting it to QDialog anyway.
But license might get in the way since kdelibs4support is LGPL and plasma is GPL.
It would mean the moved KFontDialog would stay LGPL in a GPL codebase. Which is possible but not recommendable at all.

> implement here a simple QDialog embedding KFontChooser and providing equivalent features needed here that KFontDialog has.

I would suggest you this simpler way to do it, but the code of this QDialog helper would be loosely inspired by KFontDialog anyway but not LGPL.
In the end that's pretty much the same thing as you suggested but better keep license potential issues at bay ;)

Good point(s). :D

We already lost fractional point sizes when porting from KFontDialog to QFontDialog. Best approach is to add the KFontDialog to the platform plugin (and also the old KColorDialog while you are at it), or upstream our features to Qt.

Btw, I set Toolbar font to a "condensed" version, to have more horizontal room for toolbars with text aside icons.

In D27808#625227, @cfeck wrote:

We already lost fractional point sizes when porting from KFontDialog to QFontDialog.

Yeah, I never understood why they opted for integer values only (I will dig in upstream log history to find if there any rationale behind that, if I find nothing I will, FWIW, file a bug report upstream).

Best approach is to add the KFontDialog to the platform plugin (and also the old KColorDialog while you are at it), or upstream our features to Qt.

Now, that sounds like a good idea, sort of like how the open file dialog from KIO is used instead of the upstream vanilla one (I'll need to figure out how that works to begin with).

Btw, I set Toolbar font to a "condensed" version, to have more horizontal room for toolbars with text aside icons.

OK, message received. :)

In D27808#625229, @ahmadsamir wrote:

In D27808#625227, @cfeck wrote:

We already lost fractional point sizes when porting from KFontDialog to QFontDialog.

Yeah, I never understood why they opted for integer values only (I will dig in upstream log history to find if there any rationale behind that, if I find nothing I will, FWIW, file a bug report upstream).

Best approach is to add the KFontDialog to the platform plugin (and also the old KColorDialog while you are at it), or upstream our features to Qt.

Now, that sounds like a good idea, sort of like how the open file dialog from KIO is used instead of the upstream vanilla one (I'll need to figure out how that works to begin with).

Repo is at https://cgit.kde.org/plasma-integration.git/
This is around : QPlatformFontDialogHelper https://code.woboq.org/qt5/qtbase/src/gui/kernel/qplatformdialoghelper.h.html
You can use https://cgit.kde.org/plasma-integration.git/tree/src/platformtheme/kdeplatformfiledialoghelper.cpp as base.

See https://bugreports.qt.io/browse/QTBUG-56547

I spent some time looking (and hacking at a kdeplatformfontdialoghelper...) at the code in kdeplatformfiledialoghelper, KFontChooser and KFontDialog, and I am starting to think that moving KFontDialog to KWidgetAddons (where KFontChooser and KFontRequester live) is a better/more viable solution. Also the differences/features between KFonDialog/KFontChooser v.s. QFontDialog are numerous, unlike KIO open file dialog v.s. QFileDialog.

Are there examples of how to move/port a class from one repo to another?

With D28122 I think you can go ahead and port this code to it.
Unless you want to take care of the QPlatform integration for it first.

I am not so sure about the QPlatform integration bit, it looks easier/cleaner to just use KFontChooserDialog; QFontDialog isn't widely used in KDE code, so porting to KFontChooserDialog is doable (though that sounds like going back in time to using KFontDialog in KDE4, then porting to QFontDialog in KF5... now back to KFont* variant....).

For now I'll port the code here to KFontChooserDialog.

Seems good to me
@bport should be the best reviewer here.

Ping.