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

[Daniel Kreuter]

> On Feb. 1, 2013, 10:44 a.m., Frank Reininghaus wrote:
> > 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).

Hi Frank

Thanks for your review. I will adjust the currentWeek or modifiedWeek so we can remove the calculation from the switch.

Moving it to kdelibs: Which API should handle this? Seems not so easy for me atm.

Unit testing would be grat and if you can help me there would be nice, since I only know Junit (java)


- Daniel


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


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/4b9b07bf/attachment.htm>