Review Request 125158: add logic to use icons for default xdg user dirs

[David Faure]

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



src/core/kfileitem.cpp (line 905)
<https://git.reviewboard.kde.org/r/125158/#comment59233>

    This is much better done by a readonly array, in order to take less memory and less CPU time.
    
        static const struct { QStandardPaths::StandardLocation loc, QString icon } s_icons[] = { { ..., ...} ... };
    
        static const int s_iconsCount = sizeof s_icons / sizeof *s_icons;
    
        for (int i = 0 ; i < s_iconsCount; ++i) {
         ...
        }
    
    (you don't even need the O(log n) lookup here, since the only use of the "map" is to iterate over it)



src/core/kfileitem.cpp (line 912)
<https://git.reviewboard.kde.org/r/125158/#comment59235>

    I'm wondering about the case where DocumentsLocation == DownloadLocation, as it happens on many setups (IIRC).
    
    It might be a good idea to make Documents win, rather than Download. This can be done by moving Documents to the end of the array (map.insert replaces existing values).
    
    I'd put Home as the very last one, for the same reason.
    
    (and this is also a reason not to use a QMap for "mapping" -> to be able to control order of iteration)



src/core/kfileitem.cpp (line 976)
<https://git.reviewboard.kde.org/r/125158/#comment59234>

    Should be combined with the previous block
    
        if (isLocalUrl && !delaySlowOperations && isDir()) {
            // icons for standards paths
        
            if (isDirectoryMounted(url)) {
                // iconFromDirectoryFile
            }
        }


- David Faure


On Sept. 12, 2015, 1:29 p.m., Harald Sitter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125158/
> -----------------------------------------------------------
> 
> (Updated Sept. 12, 2015, 1:29 p.m.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 352498
>     https://bugs.kde.org/show_bug.cgi?id=352498
> 
> 
> Repository: kio
> 
> 
> Description
> -------
> 
> this acts as an additional fallback to mimetype iconing and .directory
> overrides. xdg user dirs are relocatable and can change depending on user
> locale as well as user configuration and additionally are used in a range
> of different desktop environments making this a very runtime sort of
> decision.
> 
> BUG: 352498
> 
> 
> Diffs
> -----
> 
>   autotests/kfileitemtest.h 615324f2b45fdc90a7841bdd0c8aa7f47cdf57a2 
>   autotests/kfileitemtest.cpp 5f728a411401fe3009924b66970d9ae6f12c60f2 
>   src/core/kfileitem.cpp 966d8626708a8f2672f1777c873f4e27e13023d6 
> 
> Diff: https://git.reviewboard.kde.org/r/125158/diff/
> 
> 
> Testing
> -------
> 
> maked
> autotested
> installed
> dolphin and file open dialogs now show icons
> 
> 
> Thanks,
> 
> Harald Sitter
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20150921/94b73257/attachment.html>