Review Request 108667: Let dolphin look at the firstDayOfWeek as set up in the system's locale settings.

[Frank Reininghaus]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108667/#review26502
-----------------------------------------------------------


Sorry about the last comments, it seems that I must have hit some key which made the browser submit the form.


In order to keep the code more readable, I wonder if one could slightly modify your approach and adjust the currentWeek and modifiedWeek variables directly? One could just add an

if (weekStartDay != 1) {
    // Adjust week
}

after the determination of currentWeek and modifiedWeek, respectively. This would prevent the need for the odd condition in the switch () statement.

Moreover, I think that this would greatly benefit from a unit test (but we can work on that together when the patch is ready if you want).

- Frank Reininghaus


On Jan. 30, 2013, 6:06 p.m., Daniel Kreuter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108667/
> -----------------------------------------------------------
> 
> (Updated Jan. 30, 2013, 6:06 p.m.)
> 
> 
> Review request for Dolphin and Frank Reininghaus.
> 
> 
> Description
> -------
> 
> When sorting files by date, dolphin doesn't note the user's system settings. There it's possible to define another first day of week instead of the default (f.e. Sunday instead of Monday).
> This patch fixes this issue.
> 
> 
> This addresses bug 181337.
>     http://bugs.kde.org/show_bug.cgi?id=181337
> 
> 
> Diffs
> -----
> 
>   dolphin/src/kitemviews/kfileitemmodel.cpp ab39e55 
> 
> Diff: http://git.reviewboard.kde.org/r/108667/diff/
> 
> 
> Testing
> -------
> 
> Performed the steps described in comment #3 of the bug report which I won't post here again.
> 
> 
> Thanks,
> 
> Daniel Kreuter
> 
>

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