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

2016-01-01 Thread Kai Uwe Broulik 

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


Would be cool if we could have something like this for KIO::iconNameForUrl as 
well

- Kai Uwe Broulik


On Okt. 22, 2015, 7:26 vorm., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125158/
> ---
> 
> (Updated Okt. 22, 2015, 7:26 vorm.)
> 
> 
> 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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2015-10-22 Thread David Faure 

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

Ship it!


This looks good to me. Don't move the struct out of the function, it's very 
local to it, and initializing on first use is better than during every program 
startup.

- David Faure


On Oct. 2, 2015, 12:59 p.m., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125158/
> ---
> 
> (Updated Oct. 2, 2015, 12:59 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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2015-10-22 Thread Harald Sitter 

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

(Updated Oct. 22, 2015, 7:26 a.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks.


Changes
---

Submitted with commit 93d71cdd19a6a844363751bd0299205d00d9a3a6 by Harald Sitter 
to branch master.


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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2015-10-02 Thread Harald Sitter 


> On Sept. 21, 2015, 7:22 a.m., David Faure wrote:
> > src/core/kfileitem.cpp, line 905
> > 
> >
> > 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)

The two-stage lookup is to cache the QStandardPaths::standardLocations() as 
they do not change 99% of the time, so the roundtrip into QStandardPaths is not 
necessary.


> On Sept. 21, 2015, 7:22 a.m., David Faure wrote:
> > src/core/kfileitem.cpp, line 976
> > 
> >
> > Should be combined with the previous block
> > 
> > if (isLocalUrl && !delaySlowOperations && isDir()) {
> > // icons for standards paths
> > 
> > if (isDirectoryMounted(url)) {
> > // iconFromDirectoryFile
> > }
> > }

I ordered this the other way around as we want .directory to take preference 
over the standardpath icon


- Harald


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


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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2015-10-02 Thread Harald Sitter 

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

(Updated Oct. 2, 2015, 12:59 p.m.)


Review request for KDE Frameworks.


Changes
---

- use a static array for location->icon mapping order
- establish order home > desktop > documents > * causing the respective icons 
to override any other icons that might match a path
- merge directory icon lookups into one conditional branching into another 
conditional for .directory (requires mounting, standardpath icon lookup does 
not)


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 (updated)
-

  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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2015-10-02 Thread Harald Sitter 

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



src/core/kfileitem.cpp (line 903)


This actually makes my eyes bleed a fair amount. Are there any objections 
to moving the struct declaration out of the function? If so, any perferences on 
where to put it?

Other suggestions also very welcome.


- Harald Sitter


On Oct. 2, 2015, 12:59 p.m., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125158/
> ---
> 
> (Updated Oct. 2, 2015, 12:59 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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2015-09-12 Thread Frank Reininghaus 

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


This looks quite inefficient to me. Every time KFileItem::iconName() is called, 
you iterate through the QMap (by the way, you could just as well use a vector 
of pairs if you don't do any map lookups), create 8 temporary QStringLists by 
calling QStandardPaths::standardLocations(it.key()), and then iterate through 
each of these lists and compare localDirectory to every list item.

I think it would be much better to create a static map that maps the location 
strings to the icon names directly. Then you could do everything with a single 
lookup in the map.

I'm still not sure if I understand why the process which creates these 
directories cannot just add a .directory file with the desired icon name. Is 
there a reason why the problem can't be solved this way?

- Frank Reininghaus


On Sept. 11, 2015, 10:50 vorm., Harald Sitter wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125158/
> ---
> 
> (Updated Sept. 11, 2015, 10:50 vorm.)
> 
> 
> Review request for KDE Frameworks.
> 
> 
> Bugs: 352498
> https://bugs.kde.org/show_bug.cgi?id=352498
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> 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
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2015-09-12 Thread Christoph Feck 

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


Do we really need to add overhead to the icon loading for every directory, just 
because the user was too lazy to change the icons via the .directory entry?

- Christoph Feck


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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2015-09-12 Thread Harald Sitter 

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


Changes
---

new approach as suggested by Frank, flattening out the map once and then simply 
doing a key lookup on it.
this has the unfortunate side effect that runtime changes to the paths config 
(as seen in our paths KCM) will not get picked up until after application 
restart. BUT given that this feature is not exactly required and these paths 
usually do not get changed often by the user, we are probably better served 
ignoring this downside (the other options would be dbus glueing to the kcm or 
making qstandardpaths qfilesystemwatch...)


Bugs: 352498
https://bugs.kde.org/show_bug.cgi?id=352498


Repository: kio


Description (updated)
---

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 (updated)
-

  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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

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

2015-09-11 Thread Harald Sitter 

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

Review request for KDE Frameworks.


Bugs: 352498
https://bugs.kde.org/show_bug.cgi?id=352498


Repository: kio


Description
---

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

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel