> On Oct. 20, 2013, 9:32 a.m., David Faure wrote: > > OK, I'm just wondering if it still makes sense to make it > > static-methods-only. > > It can still very well work the other way, which can allow extra > > flexibility if needed (e.g. for kate to add another widget into it, or use > > it non-modal, etc.). What's the reason for making it less flexible? Just > > the fact that nobody uses the constructor right now? (I assume you checked > > that?). > > > > Kévin? > > Kevin Ottens wrote: > Initially I think I proposed even a namespace. :-) > > But seeing the current solution I think you're right, we could indeed > allow instances to be created. > > Teo Mrnjavac wrote: > Ok, but that would add some more duplication over what KFileDialog > already does. Afaict, the only non-static usage of KEncodingFileDialog was in > KMail's messagecomposer. I had checked with kdepim people a while ago, and > they have no problem with KEncodingFileDialog becoming static only in KF5. > > Kevin Ottens wrote: > I'm not sure which kind of duplication you have in mind, but surely just > making the ctor/dtor pair public and adding a result() public method would be > enough (said method you could use in your static methods BTW reducing some of > the code duplication between them).
OK, discussing with Teo on IRC I think he's right. We could make it instanciable with what I propose... but then if you want a proper API you need accessors for a few properties, and signals when the user interact with the dialog etc. We'd basically duplicate quite some KFileDialog code... which was API not used in the case of KEncodingFileDialog. So he's right, let's keep it based on static alone. - Kevin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/113302/#review42003 ----------------------------------------------------------- On Oct. 19, 2013, 11:30 a.m., Teo Mrnjavac wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/113302/ > ----------------------------------------------------------- > > (Updated Oct. 19, 2013, 11:30 a.m.) > > > Review request for KDE Frameworks, David Faure and Kevin Ottens. > > > Repository: kdelibs > > > Description > ------- > > This makes KEncodingFileDialog static, moves it to staging/kio and moves some > stuff from KFileDialog to KFileWidget to reduce duplication. > > > Diffs > ----- > > kio/CMakeLists.txt ac80e393c877280dd8033ec13e8e772afea6d2f9 > kio/kfile/kencodingfiledialog.h abb939abeb000126005f1a1a9c6fd50b8bd322bd > kio/kfile/kencodingfiledialog.cpp d55d908473aae2859838d88fd776cc65cecf3317 > kio/kfile/kfiledialog.cpp 32eb98a4490a31c5ed51150ca3cb7af5375dc67e > staging/kio/src/filewidgets/CMakeLists.txt > e8d8c2edf11ad6352e13eb6e7436f828a4a55e3a > staging/kio/src/filewidgets/kencodingfiledialog.cpp PRE-CREATION > staging/kio/src/filewidgets/kfilewidget.h > f7b162ab39b997274b21f9eff0c6374545e0a9ad > staging/kio/src/filewidgets/kfilewidget.cpp > 824ac563f3f8c463426f0a45e792f107ac402a40 > > Diff: http://git.reviewboard.kde.org/r/113302/diff/ > > > Testing > ------- > > Seems to work fine in a test app. > > > Thanks, > > Teo Mrnjavac > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
