Review Request 116047: Add a space info observer, to share the space information between all views. Replaced the space information bar by a simple space information text.

[Frank Reininghaus]

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


First of all, thanks for looking into this.

To be honest, I'm not sure if mixing multiple unrelated changes in one patch is a good idea (even if we ignore for a moment that it's not clear at all if people will like the GUI change - my assumption is that most users who use the 'free space' bar won't). Such patches tend to hide subtle regressions.

If I'm not mistaken, one thing that might not be obvious when having a quick look at your patch is that it will query the free space for all paths shown in all Dolphin windows every 10 seconds, even if the computer is not used at all, and the screen is locked (the current code stops the timer if a view loses focus - the bug is that the timer is not restarted again). It gets worse if there are multiple Dolphin windows whose 10-second timers are not in sync.

This will waste energy and kill the batteries of laptop users.

Please discuss such ideas on the mailing list before changing lots of code!

- Frank Reininghaus


On Feb. 25, 2014, 3:19 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116047/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2014, 3:19 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Bugs: 318395 and 327708
>     http://bugs.kde.org/show_bug.cgi?id=318395
>     http://bugs.kde.org/show_bug.cgi?id=327708
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> 1.)
> Add a space info observer, to share the space information between all views.
> The updating of space information in split view is also fixed with this 
> patch. A KUniqueApplication based Dolphin version will also benefit from these
> changes ;)
> 
> 2.)
> Replaced the space information bar by a simple space information text.
> Benefits: We have more space for relevant information and it 
> looks nicer (personal opinion).
> 
> 
> I decided to fix these two bugs with one review request, because splitting
> this up into independent patches is much more work (space observer would need 
> more than just available space info, statusbar would also need a lot more code) 
> and I think it'll be harder to review.
> 
> 
> Btw.:
> If you don't like the space info as text idea, we can extend the free space
> observer, so that it works together with the current space info bar. ;)
> 
> 
> Diffs
> -----
> 
>   dolphin/src/CMakeLists.txt 3f58479 
>   dolphin/src/statusbar/dolphinstatusbar.h b2afe2e 
>   dolphin/src/statusbar/dolphinstatusbar.cpp 671ef4f 
>   dolphin/src/statusbar/spaceinfoobserver.h PRE-CREATION 
>   dolphin/src/statusbar/spaceinfoobserver.cpp PRE-CREATION 
>   dolphin/src/statusbar/statusbarspaceinfo.h 1849462 
>   dolphin/src/statusbar/statusbarspaceinfo.cpp 61b2833 
> 
> Diff: https://git.reviewboard.kde.org/r/116047/diff/
> 
> 
> Testing
> -------
> 
> Works fine for me.
> 
> 
> File Attachments
> ----------------
> 
> Dolphin Status Bar Old
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/02/25/7b5f8dc8-36fc-4751-b1d6-3861af6cc4e3__dolphin_statusbar_old.png
> Dolphin Status Bar New
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/02/25/40f28466-590a-4322-b8f4-50f9ca785b3a__dolphin_statusbar_new.png
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140225/36a82f2f/attachment.htm>