Review Request 130094: Implement QPlatformTheme::fileIconPixmap()

[Eugene Shalygin]

> 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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20170421/660dd854/attachment-0001.html>