> On April 21, 2017, 11:12 a.m., Kai Uwe Broulik wrote: > > Thanks for your patch! > > > > Plasma reviews are nowadays handled on https://phabricator.kde.org/ - could > > you perhaps upload it there again? I just don't want it to get lost here :/ > > > > Also, I'm not sure about having a separate FileIconProvider class. You also > > seem to reimplement quite a lot of logic. Can you perhaps try > > KIO::iconNameForUrl (in KIOCore I think) which has all of that mimetype > > resolution and special folders logic already built-in, potentially allowing > > you to reduce your patch to like 10 lines of code :) > > > > (Btw using Qt 5.7 features is fine, Plasma-integration is released > > alongside Plasma, not KDE Frameworks, and the upcoming Plasma 5.10 release > > will depend on Qt 5.7 anyway)
This patch was applying against frameworkintegration for a long time, and there I wanted no dependencies. BTW, do you know what platform theme plugin can be extended to make this work in non-Plasma environment (like GNOME) too? > On April 21, 2017, 11:12 a.m., Kai Uwe Broulik wrote: > > src/platformtheme/kdeplatformtheme.cpp, line 122 > > <https://git.reviewboard.kde.org/r/130094/diff/1/?file=494561#file494561line122> > > > > Why is this only built in 5.7 and below? Was this removed in 5.8? > > (didn't check Qt code) Signature of the function was changed in 5.8 if you are asking about the #ifdef. Did I understand you correctly? - Eugene ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/130094/#review103073 ----------------------------------------------------------- On April 21, 2017, 11:11 a.m., Eugene Shalygin wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/130094/ > ----------------------------------------------------------- > > (Updated April 21, 2017, 11:11 a.m.) > > > Review request for Plasma. > > > Repository: plasma-integration > > > Description > ------- > > Implement QPlatformTheme::fileIconPixmap() to make QFileIconProvider work. > > > Diffs > ----- > > src/platformtheme/kdeplatformtheme.h 7835439 > src/platformtheme/kdeplatformtheme.cpp 704f176 > > Diff: https://git.reviewboard.kde.org/r/130094/diff/ > > > Testing > ------- > > Manual testing. > > > Thanks, > > Eugene Shalygin > >
