We've just had a long discussion about the future of KIcon in APIs, which led into a discussion about what we're doing in frameworks at all and why.
Several people don't think refactoring kdelibs into independent frameworks is a good thing. Do we need to decide if it's worth it? I'm posting the log for archival reasons mostly. There are several misunderstandings in it, but those are mostly untangled later on. The content of http://paste.kde.org/118765/ is: namespace KIcon { QIcon fromTheme(const QString &name) { return QIcon(new KIconEngine(...)); } } Where the content of the function is intended to be 'what KIcon ctor does' [13:08:26] <steveire> ogoffart: KIcon("foo") should be replaced with QIcon::fromTheme("foo"), right? [13:08:37] <steveire> How does QIcon::fromTheme work?> [13:08:45] <ogoffart> steveire: yes. [13:08:48] <steveire> Does it use a kde pluing? [13:08:54] <steveire> plugin* [13:09:01] <ogoffart> steveire: it has its own implementation of the xdg icon spec [13:09:15] <ogoffart> steveire: it uses a plugin only to know which theme to load. [13:09:27] <ogoffart> that is, to read kde settings. [13:10:06] <steveire> So is KIconEngine also obsolete? [13:10:24] <steveire> I guess this stuff might be in the spreadsheet from randa [13:11:37] <ogoffart> steveire: yes. [13:11:43] <steveire> The spreadsheet is mostly silent on that topic. [13:11:54] <steveire> It just says Qt needs overlay support to replace KIcon. [13:13:34] <ogoffart> steveire: but we discussed at the contributor summit, and aseigo and dfaure said that nothing really use the overlay. (Or if they use overlay, it is not something that is in the tier1, 2 or 3, so this could go into a kde specific library of even in the application itself) [13:13:47] <steveire> Right. [13:13:55] <dfaure> "nothing uses overlays" is not true, kfileitem.cpp does [13:14:16] <dfaure> but I suppose we could develop something that uses QIcon::fromTheme twice and combines the results [13:14:22] <ogoffart> yes [13:15:17] <-> 45PAALFRM is now known as miroslav [13:17:35] <steveire> Why does mpyne recommend not using the official Qt? [13:17:58] <DxSadEagle> does QIcon::fromTheme implement appropriate appropriate caching & lazy loading? [13:20:23] <pinotree> does QIcon::fromTheme() read also in the icon theme of the component data? [13:22:24] <pinotree> can QIcon::fromTheme() take a different component data and/or icon loader to read icons from? [13:23:16] <dfaure> the icon theme of the component data? a plugin coming with its own icon theme? [13:23:49] <pinotree> with own icons in a icon-theme tree under its $datadir/icons [13:24:15] <pinotree> this greatly avoids polluting the global icon theme with app- or plugin-specific icons [13:24:42] <dfaure> ah, "own icons", not a replacement icon theme. [13:25:05] <pinotree> well, nothing forbids you to put a whole icon theme there :) [13:25:20] <dfaure> a feature actually used by.... ? [13:25:37] <pinotree> ls -d /usr/share/kde4/apps/*/icons [13:28:42] <-> atomo_yawn is now known as atomopawn [13:29:56] <dfaure> ah I see, even if you don't have a full icon theme, it's still organized like one [13:30:31] <dfaure> ogoffart: should QIcon::fromTheme take an additional optional argument with a base directory for the app-specific icon theme? [13:30:58] <steveire> I presume that stuff is built into the KIconEngine? [13:31:20] <steveire> I could create KIcon::fromTheme which uses that, but still use QIcon in the API [13:31:43] <ogoffart> dfaure: you can only set the theme globaly with QIcon::setThemeName or something [13:32:02] <ogoffart> dfaure: then again, the default theme name is fetch from the KDE config while running on KDE [13:32:20] <dfaure> steveire: you wouldn't have access to the icon theme loader in qt to add paths to it [13:32:41] <dfaure> ogoffart: you didn't read the above discussion it seems ;) This is about /usr/share/kde4/apps/kgpg/icons/hicolor/ [13:33:02] <dfaure> the icon theme name is hicolor just fine, the issue is "for this specific icon, I want to also look into /usr/share/kde4/apps/kgpg/icons [13:33:37] <ogoffart> DxSadEagle: it has per-application cache [13:36:13] <ogoffart> ah [13:36:18] <ogoffart> dfaure: there is QIcon::setThemeSearchPaths [13:36:44] <dfaure> ah. [13:36:48] <ogoffart> again this is application global, but one can add /usr/share/kde4/apps/kgpg there [13:36:55] <ogoffart> in kgpg [13:37:04] <dfaure> indeed. [13:37:25] <steveire> '<dfaure> steveire: you wouldn't have access to the icon theme loader in qt to add paths to it' I don't understand this I think [13:37:27] <ogoffart> the default is also fetch from the plugin [13:38:00] <steveire> Looking at the implementation of KIcon it looks like an empty class with a magic constructor [13:38:01] <DxSadEagle> that's too global... [13:38:05] <dfaure> steveire: QIcon::fromTheme uses a qt-internal-iconloader [13:38:30] <steveire> Let me look at the code again :) [13:38:36] <dfaure> steveire: KIcon uses KIconEngine uses KIconLoader, but the discussion here is about porting that to QIcon::fromTheme.... [13:39:08] <dfaure> DxSadEagle: yeah it's too global for libs to add their own search paths. But looking at /usr/share/kde4/apps/*/icons, it seems only apps do that. [13:39:15] <dfaure> libs just use icons from the actual icon theme... [13:39:32] <steveire> dfaure: Yes. The point I'm trying to make is only about using KIcon vs QIcon in the APIs [13:39:33] <DxSadEagle> dfaure: and none of the things in there are parts? [13:40:00] <steveire> KIcon::fromTheme would just do what the KIcon ctor does [13:40:14] <dfaure> I don't understand the point of a "KIcon::fromTheme" [13:40:19] <ogoffart> dfaure: so the plugin says the default search path is: KGlobal::dirs()->resourceDirs("icon"); [13:40:20] <dfaure> it's indeed what the ctor does ;) [13:40:34] <dfaure> DxSadEagle: no, but that's a good point. [13:40:44] <DxSadEagle> dfaure: even okular? [13:40:58] <winterz> daitheflu, bcooksley-away: we don't generate the PyKDE dox. Simon emails me a tarball containing the PyKDE dox and I hang it up on api.kde.org. So there are 2 problems: 1) we need to copy over the PyKDE dox from the old EBN for the versions we have 2) we need to nag Simon to send us PyKDE dox updates for KDE 4.7, KDE 4.6... maybe some others [13:41:21] <dfaure> ogoffart: the plugin has no idea if we're about to load an icon from a part or from an app [13:41:37] <dfaure> DxSadEagle: there's only a /usr/share/kde4/apps/okular/icons/hicolor/16x16/apps/okular-gv.png [13:41:46] <dfaure> not sure if that's loaded by okular app or part [13:41:51] <ogoffart> but how does KIcon does? [13:42:34] <dfaure> ogoffart: by being passed the pointer to a KIconLoader in the ctor [13:42:35] <steveire> dfaure: From what I understand, KIcon::fromTheme would allow us to take KIcon out of our APIs, and would have all the features you're discussin glike app specific icons would already work [13:43:01] <steveire> But if QIcon::fromTheme can be made to do that too, maybe that's ok [13:43:04] <dfaure> steveire: you can already use QIcon in the API, and let people write KIcon("foo") when calling the API, no need for KIcon::fromTheme (very redundant) [13:43:15] <DxSadEagle> dfaure: I recall it being able to also load stuff from plain app/kfoo/pics [13:43:47] <dfaure> DxSadEagle: yes, pics. That's different, it's not organized like a theme, you only need a QStandardPaths + "kfoo/pics/foo.png" and pass the full path to QIcon [13:45:28] <steveire> I guess. [13:45:34] <DxSadEagle> dfaure: just that can currently be loaded by loadIcon, too. [13:46:19] <steveire> What I'd really like is to put some factory method in a namespace instead of having a static method in the class (no one can create an instance of a namespace) [13:47:03] <DxSadEagle> and you're suggestiong while we're discussing how global is insufficient? ;-) [13:47:04] -*- dfaure isn't sure which namespace/class steveire is talking about [13:49:15] <winterz> daitheflu, bcooksley-away: my sent-mail folder says I sent such a nag to Simon on 31 July 2011. and got no response. i'll re-nag one more time [13:52:10] <ervin> dfaure: I predict KIcon being used in the api if the type is still there though ;) [13:52:29] <dfaure> ervin: not if the tiers prevent it... [13:52:46] <ervin> dfaure: it's better if there's no KIcon type really [13:53:01] <ervin> dfaure: ok, sorry, thought as well about apps internals [13:53:14] <ervin> in doubt they'll go for K* types always [13:53:42] <ervin> we'd just make it harder for app code to move into libs IMO [13:53:49] <ervin> since they'd have to adapt to the type [13:54:06] <ervin> a convenience ctor doesn't justify the existence of a type imho [13:54:15] <ogoffart> ervin: you rather have a KIcon macro? :-) [13:54:36] <ervin> ogoffart: I prefer steveire's proposal at that point [13:54:47] <ervin> namespace with a factory method [13:55:45] <steveire> http://paste.kde.org/118765/ [13:56:12] <steveire> Just for clarity :) [13:57:05] <dfaure> breaks compat big way, because we can't have KIcon as both a class and a namespace [13:57:33] <dfaure> so every single piece of code currently doing KIcon("foo") will break. [13:57:59] <dfaure> I'm all for making the kdelibs-frameworks API as clean as possible, but at the same time we should make the porting minimal, just like Qt5 promises minimal SIC. [13:58:33] <steveire> We can 1) Use Bertjans porting to for that (there is going to be porting and a porting tool anyway) or 2) Use a namespace of a different name and keep the KIcon class [13:58:34] <ervin> I'm fine with a different namespace name [13:58:49] <steveire> I prefer option 1 [13:58:58] <dfaure> "We" can use a complex tool, but not every kde app author out there. [13:58:59] <DxSadEagle> steveire: porting tool, hihi. Were you around for KDE4 port? ;-) [13:59:03] <ervin> the thing is the namespace proposed would live in the ui integration lib, KIcon really has to move to a kde4support [13:59:06] <dfaure> So I much prefer option 2. [13:59:12] <steveire> We have yet to see what the realities of Qt 5 porting will be [13:59:27] <steveire> DxSadEagle: Nope, I wasn't [13:59:41] <dfaure> steveire: yes, the future is always a bit uncertain. But that's no reason to make it worse. [13:59:47] <ervin> agreed [14:00:12] <steveire> Meh. Ok. I don't feel so strongly about it to argue it. [14:01:02] <steveire> ervin: solid unit tests require kio. Can I comment those tests out until either the tests are ported or kio is in a tier somewhere? [14:01:10] <pinotree> i just wonder why is using k-api so bad these days [14:01:24] <steveire> pinotree: It creates a walled garden [14:01:33] <ervin> steveire: hm, which one? [14:02:00] <steveire> If my api is MyClass::someMethod(QUrl), a Qt developer can use it. If it is MyClass::someMethod(KUrl) they can't [14:02:12] <steveire> ervin: The last one in the CMakeLists file [14:02:13] <ervin> steveire: might be something from the past, tried to build without linking to it? [14:02:28] <pinotree> steveire: so what's the problem with kicon? it's a qicon, no? [14:02:29] <steveire> I want to make solid part of the new build system [14:02:41] <steveire> pinotree: It is indeed a QIcon. [14:02:56] <steveire> MyClass::someMethod(QIcon) vs MyClass::someMethod(KIcon) Same issue [14:03:13] <pinotree> you're picking a different issue [14:03:31] <pinotree> kicon is a qicon with the added points of using kde's icon loader, what's bad in it that you wanted to dismantle it? [14:03:57] <ervin> steveire: give it a try without linking to kio, if that doesn't work then yes just disable it for the time being, it's not an autotest anyway [14:04:05] <steveire> ervin: Ok [14:04:19] <daitheflu> winterz: thank you for the information :) [14:04:19] <steveire> mbensi is solid maintainer now, right? [14:04:28] <ervin> steveire: nope [14:04:34] <ervin> steveire: afiestas is [14:04:38] <steveire> Ah, ok [14:04:48] <ervin> steveire: mbensi (aka [Nef]) maintains one of the libsolid backends [14:05:08] <steveire> pinotree: I'm not sure how I'm picking a different issue [14:05:18] <pinotree> <steveire> MyClass::someMethod(QIcon) vs MyClass::someMethod(KIcon) Same issue [14:05:22] <steveire> Right. [14:05:23] <-> atomopawn is now known as atomo_yawn [14:05:32] <steveire> For me the issue is having KIcon in the api [14:05:49] <pinotree> as parameter/return value or as class in general? [14:05:49] <steveire> If that's not the issue, then what is? How is it different? [14:05:56] <steveire> pinotree: Yes [14:06:03] <steveire> Sorry [14:06:07] <pinotree> "A or B?" "yes" [14:06:09] <steveire> Parameter or return value [14:06:44] <pinotree> then i don't see why kicon("...") usages should be replaced with qicon::fromtheme() [14:06:55] <pinotree> or why kicon as class should go away [14:07:13] <steveire> If KIcon remains as a non-deprecated class, then use creep will make it re-appear in APIs [14:07:39] <pinotree> oh please [14:07:43] <steveire> I'm also not certain replacing it with QIcon::fromTheme is the right thing to do anymore [14:08:12] <steveire> You think my use-creep argument is ridiculus? [14:08:31] <steveire> ridiculous* [14:08:32] <pinotree> it is ridicolous if you use it as argument for dismantling kicon [14:08:48] <steveire> Why? [14:08:54] <steveire> What is dismantling? [14:09:13] <pinotree> ok, s/dismantling/removing/ [14:09:50] <ervin> well, on top of that, an extra type can't be justified only by having ctors in its API really [14:10:34] <steveire> What is the disadvantage of removing it? It is a porting step? Is that the only one? Is that big enough to justify not removing/deprecating it? [14:11:11] <pinotree> steveire: let me reverse the question: why do you want to remove kicon as class, instead of just use qicon in public api? [14:11:32] <steveire> If it remains as a class people will use it as a class and put it in APIs [14:11:47] <steveire> That creates a walled garden [14:12:04] <ervin> and again, design wise it made no sense to create it in the first place [14:12:19] <steveire> It means that developers using Qt can't use our stuff without using 'all' of our stuff [14:12:23] <pinotree> so kde apps cannot use kde classes because it creates a walled garden for qt developers? [14:12:26] <ervin> this inheriting + ctor fetish is just so 90s :-) [14:12:37] <steveire> ervin: I fully agree [14:13:03] <steveire> I really hate the 'we must inherit' stuff [14:13:20] <steveire> pinotree: I'm talking about frameworks [14:13:26] <pinotree> i really hate the 'we must use only classes in qt' stuff [14:13:43] <steveire> If kde apps feel good about using KIcon then so be it. But there's no reason for it [14:13:55] <-> Quintasan_ is now known as Quintasan [14:14:03] <pinotree> the fact that *you* see no good reasons, does not mean there aren't [14:14:13] <steveire> Do you see any? [14:14:39] <pinotree> i already said that almost one hour ago [14:14:47] <steveire> I don't propose removing KIcon completely, just removing it from APIs and deprecating it [14:15:05] <pinotree> don't deprecate it, it is useful for kde stuff [14:15:09] <ervin> pinotree: come on, stop painting things in black, kicon is first and foremost a design mistake as api, we've the opportunity to fix it [14:15:13] <steveire> pinotree: I've also repeated myself on several points. Enlighten me again please [14:15:30] <ervin> you'd get the exact same service with factory methods [14:16:13] <pinotree> ervin: thanks for portraiting me as i am not, i'll stop here. [14:16:52] <ervin> pinotree: well I felt ignored, or at least the design point on the inheritance there :) [14:17:10] <pinotree> i always find interesting when people jump from technical arguments to "you suck"-like personal arguments [14:17:11] <steveire> So "it's useful for kde stuff" is the reason for keeping it? [14:17:36] <pinotree> steveire: read again what i said. qicon as it is it is not enough [14:17:40] <svuorela> KIcon has, as I've heard one big advantage. cross-application caching. [14:17:54] <steveire> Where? In KIconEngine? [14:18:01] <pinotree> <pinotree> does QIcon::fromTheme() read also in the icon theme of the component data? [14:18:06] <pinotree> → no [14:18:09] <pinotree> <pinotree> can QIcon::fromTheme() take a different component data and/or icon loader to read icons from? [14:18:09] <steveire> If we create a QIcon witha KIconEngine, then we get the same feature. [14:18:12] <pinotree> → no [14:18:14] <dfaure> I think there's a misunderstanding here. Steve isn't proposing to ditch KIconLoader, just to replace KIcon("foo") with KIconSomething::fromTheme("foo") [14:18:15] <steveire> Read kicon.cpp [14:18:33] <dfaure> pinotree: so there is no feature loss in this proposal [14:18:33] <pinotree> dfaure: isn't that worse? [14:18:37] <steveire> Or rather, SomeNamespace::someMethod("foo") [14:18:51] <dfaure> pinotree: only an API change, where there is no QIcon subclass, so the only way to pass an icon to a method is const QIcon& icon [14:18:54] <DxSadEagle> like someMehtod = SmallIconSet? ;-) [14:19:11] <dfaure> DxSadEagle: haha ;) kde2 is back ;) [14:19:46] -*- svuorela isn't old enough to catch david and maksim's jokes. [14:19:55] <montel_> :) [14:19:57] <dfaure> svuorela: look at kiconloader.h ;) [14:19:58] <steveire> pinotree: I'm not proposing replacing KIcon with QIcon::fromTheme. [14:20:09] <pinotree> dfaure: if all people claimed that qt5 and kde5 are "mostly SC", then i don't see how such a unreasoned change would comply to that [14:20:23] <dfaure> pinotree: mostly SC means keeping KIcon in a layer that apps can use [14:20:27] <pinotree> steveire: <steveire> ogoffart: KIcon("foo") should be replaced with QIcon::fromTheme("foo"), right? ← one hour ago [14:20:49] <steveire> pinotree: There has been an hour of discussion since then, and even a pastebinned API ? [14:21:16] <steveire> And talk about a method in a namespace, and talk about using KIconEngine [14:21:31] <steveire> And talk about how the only thing KIcon is is a ctor [14:21:50] <steveire> The KIcon type doesn't have carry any benefits. [14:21:58] <pinotree> steveire: i *know*, but then don't claim that you didn't propose such stuff, when you really did (not knowing anything about what kicon does at all) [14:22:28] <kdepepo> QIcon myIcon = KUI::icon("foo"); [14:22:46] <kdepepo> where KUI is the namespace [14:22:49] <steveire> I didn't say I didn't propose it. I said I *don't*. It's subtle, maybe a bit too subtle. But I have not been discussing fromTheme in some time [14:23:34] <pinotree> and if i didn't start asking those two questions, you would have kept your idea of replacing it, honestly [14:23:39] <steveire> I have been disussing a method in a namespace which *does* use KIconEngine and has the exact features KIcon currently has [14:24:12] <steveire> pinotree: Yes. But you did bring up that point. Almost immediately the discussion changed to not using QIcon::fromTheme [14:24:24] <steveire> We're now discussing the discussion, which is far too meta [14:24:27] <pinotree> all of this because "there must not be k-classes, otherwise the poot qt developers cry"... sad [14:24:46] <steveire> Let's get back to the topic kdepepo just posted. What's the propblem with it? [14:25:36] <pinotree> that i don't care, easy [14:25:46] <steveire> huh? [14:26:14] <steveire> The problem is that you don't care about the snippit posted? [14:26:40] <steveire> pinotree: That is indeed what frameworks is about. There have been several discussions about it. Feel free to bring it up on k-c-d and change the direction of the people doing the work [14:26:41] <pinotree> no, i don't care about this whole "making kde a qt subsidiary" thing [14:27:00] <pinotree> steveire: i'ts pretty clear the direction cannot be changed at all [14:27:17] <steveire> Ok. [14:27:31] <pinotree> that leaves me the question why i'm still here... let's fix this up [14:27:35] <steveire> I'll just post this log for archival purposes to k-c-d then. [14:27:38] <-- pinotree (~pino@kde/pino) has left #kde-devel ("farewell") [14:27:48] <steveire> We'll see if anything comes of it [14:28:26] <dfaure> not sure a discussion about a meta discussion will bring anything ;) [14:28:58] <ervin> dfaure: likely not [14:29:11] <ervin> well, maybe some breakthrough in metaphysics [14:29:37] <dfaure> steveire: IMHO post an api proposal, not a meta-meta-discussion [14:29:55] -*- Jucato inserts http://xkcd.com/917/ for some lighthearted humor .... [14:30:13] <dfaure> steveire: the discussion about ditching KIconLoader is a complex one, but indeed not the one that happened in the last hour. Instead your proposal is only a "small" api change, and keeping KIcon as compat, I see no issue there. [14:30:15] <DxSadEagle> steveire: you're basically saying we should favor hypothetical Qt developers over actual KDE app developers. [14:30:31] <dfaure> DxSadEagle: no, we're saying that independent libs make everyone happy [14:30:39] <DxSadEagle> dfaure: do they now [14:31:12] -*- DxSadEagle wonders about how much code will get acicdentally moved to Qt without all copyright owners' permissions? [14:31:28] <steveire> If they don't, then we can stop the frameworks refactoring. I guess we need to decide before we do more work on it [14:31:40] <steveire> DxSadEagle: I predict none [14:31:41] <ervin> I wonder why everyone focuses on the emotional fantasy of kde vs qt, while really KIcon is easy: design wise a type inheriting from QIcon for the features offered makes no sense [14:32:15] <DxSadEagle> But hey, my technical PoV leans more towards forking Qt4.8 than reorganizing kdelibs. [14:32:30] <steveire> dfaure: Ok, to be clear (and for hte archived discussions): I absolutely do not support removing KIconLoader anymore. [14:32:59] <steveire> But I know you can see that. [14:33:02] <dfaure> DxSadEagle: code-into-Qt sounds like a different issue from "making the frameworks independent from each other". [14:33:13] <dfaure> steveire: yep. [14:33:34] <kdepepo> r1251673 ??? [14:33:49] <DxSadEagle> dfaure: it's not entirely, since you're trying to use more Qt stuff while simultaneously not regressing functionality/causing massive SC issues. [14:34:09] <dfaure> fun, heh? ;-) [14:34:38] <dfaure> sorry, gotta go, talk to you guys tomorrow [14:35:18] <DxSadEagle> IMHO: poorly timed with the toolkit going in a rather iffy direction.
