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/#review26669
-----------------------------------------------------------


Thanks for the new patch, looks better!

I just tried to test it, created a set of files with different modification times and first wanted to check what it looks like without patch:

http://ompldr.org/vaGN3ag/testi.png

So it seems that the 'first week day' issue is not the only thing that needs fixing in that function ;-) The problem is:

1. If a file was modified in the current month, it is only shown in the 'current week' (i.e., 'Today', 'Yesterday', or 'Wednesday', etc.) if it is from the same calendar week. If that is not the case, the 'Last Week'/'Two weeks ago' stuff is determined based on the calendar week distance.

2. If a file was modified last month, it is shown in the 'current' week if it is less than 8 days old. Similarly, it is shown in 'Last Week' if it is less than 15 days old, etc.

This is why we get such a broken grouping. I'm thinking that the easiest way to fix this and to resolve the 'first week day' issue is to throw away the concept of 'calendar weeks' in that function and always group based on the age in days (like it's currently done for files from last month).

This would mean that we replace

switch (currentWeek - modifiedWeek) {
    // ...
}

by

switch (daysDistance / 7) {
    // ...
}

and remove all code related to currentWeek/modifiedWeek.

OTOH, this would change the behaviour. However, considering that there is to my knowledge no bug report about this issue that I found the first time I tried grouping by date, I wonder how many people would notice... What do you think?

- Frank Reininghaus


On Feb. 4, 2013, 7:40 p.m., Daniel Kreuter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108667/
> -----------------------------------------------------------
> 
> (Updated Feb. 4, 2013, 7:40 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 1062aa5 
> 
> 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/20130205/dbf90d57/attachment.htm>