> 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) > > Eugene Shalygin wrote: > 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?
Took a look at KIO::iconNameForUrl() and not sure whether to use it and discard iconOptions parameter to this function or not. Do you want to finish with this question here, or should I move to phabricator right away? - 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 > >
