Review Request 118415: Fix looking up icons

Martin Klapetek martin.klapetek at gmail.com
Fri May 30 10:48:55 UTC 2014



> On May 30, 2014, 12:45 p.m., Aleix Pol Gonzalez wrote:
> > src/kicontheme.cpp, line 480
> > <https://git.reviewboard.kde.org/r/118415/diff/1/?file=276419#file276419line480>
> >
> >     The iconPath call doesn't look trivial. I don't think we want the call twice, put it in a variable maybe?

Fwiw, that call does pretty much this:

    if (QFile::exists(file)) {
        return KLocalizedString::localizedFilePath(file);
    }

but sure, why not.


- Martin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/118415/#review58782
-----------------------------------------------------------


On May 30, 2014, 10:23 a.m., Martin Klapetek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/118415/
> -----------------------------------------------------------
> 
> (Updated May 30, 2014, 10:23 a.m.)
> 
> 
> Review request for KDE Frameworks and Christoph Feck.
> 
> 
> Repository: kiconthemes
> 
> 
> Description
> -------
> 
> What the current code does is looking for the best match, storing it into QString path...and then overwrite it with empty string on next run if the icon is not found in the current dir. So we need to check first if the icon is available in the theme and only then store the path, otherwise empty string gets returned non-deterministically.
> 
> This also fixes case of some missing icons in Plasma Next.
> 
> 
> Diffs
> -----
> 
>   src/kicontheme.cpp 78b8485 
> 
> Diff: https://git.reviewboard.kde.org/r/118415/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Martin Klapetek
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20140530/ebf1e53c/attachment-0001.html>


More information about the Kde-frameworks-devel mailing list