[prev in list] [next in list] [prev in thread] [next in thread]
List: calligra-devel
Subject: Re: Review Request 111413: Plan - Set First Day of Week to Monday if using ISO Week for week numberi
From: "Friedrich W. H. Kossebau" <kossebau () kde ! org>
Date: 2013-11-24 9:50:29
Message-ID: 20131124095029.16082.192 () vidsolbach ! de
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
> On Nov. 23, 2013, 11:04 p.m., Friedrich W. H. Kossebau wrote:
> > 3rdparty/kdgantt/kdganttdatetimegrid.cpp, lines 28-29
> > <http://git.reviewboard.kde.org/r/111413/diff/1/?file=169234#file169234line28>
> >
> > Hm. libkdgantt is a copy of an external lib that is just not packaged anywhere \
> > (and in the next weeks will be hopefully updated with a newer version)
> > And so far it is a Qt-only lib.
> >
> > But I see the need to change it to use KCalendarsystem, as QDate::weekNumber() \
> > seems hardcoded to ISO 8601, ignoring any settings done in the KDElibs layer.
> > And as Plan is the only one using libkdgantt in Calligra, no one else is hurt by \
> > also linking libkdecore, okay.
> > Seems you did this patch when libkdgantt was still (accidentally) linked to \
> > libkdecore, so that has to be added again.
> > For that change 3rdparty/kdgantt/CMakeLists.txt to also link to \
> > ${KDE4_KDECORE_LIBS}, so it has lines like: target_link_libraries(calligrakdgantt \
> > ${KDE4_KDECORE_LIBS} ${QT_QTGUI_LIBRARY}) target_link_libraries(calligrakdgantt \
> > LINK_INTERFACE_LIBRARIES ${KDE4_KDECORE_LIBS} ${QT_QTGUI_LIBRARY})
> > And please also create a separate patch for this change to libkdgantt, which then \
> > is added to 3rdparty/kdganttpatches/ (hm, actually with git that might not be \
> > really needed anymore, but let's just do it for the tradition).
And also add ${KDE4_INCLUDES} to the include_directories
- Friedrich W. H.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111413/#review36515
-----------------------------------------------------------
On July 6, 2013, 12:51 a.m., Alvaro Soliverez wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111413/
> -----------------------------------------------------------
>
> (Updated July 6, 2013, 12:51 a.m.)
>
>
> Review request for Calligra.
>
>
> Bugs: 321290
> http://bugs.kde.org/show_bug.cgi?id=321290
>
>
> Repository: calligra
>
>
> Description
> -------
>
> Plan uses week numbering extensively. The default week numbering in KDE is ISO \
> Week, which uses Monday as first day of the week, regardless of other locale \
> settings.
> To make it consistent through the application, if week numbering format is ISO \
> Week, set the first day of the week to Monday, and display it accordingly in charts \
> and graphs.
> Without this patch, the week could be set to start on Sunday, and week would be \
> shown on graphs as ranging from Sunday to Saturday, but the week number of working \
> days would be off by one.
> Example:
>
> - Week numbering: ISO Week
> - First day of week: Sunday
>
> Without patch:
> - Week displayed: 2013-07-07 to 2013-07-13
> - Week number shown on charts: 27
>
> - Expected week number: 28 (Week 27 ends on Sunday 2013-07-07)
>
> With patch:
> - Week displayed: 2013-07-08 to 2013-07-14
> - Week number shown on charts: 28
>
>
> Additionally, there were a few calls to QDate::week() to get the week number, \
> instead of using KGlobalCalendar.
>
> Diffs
> -----
>
> plan/libs/ui/kptganttview.cpp 81073a3
> plan/libs/models/kptaccountsmodel.cpp 71c3100
> plan/libs/models/kcalendar/kdatetable.cpp b389345
> 3rdparty/kdgantt/kdganttdatetimegrid.cpp 79c405c
>
> Diff: http://git.reviewboard.kde.org/r/111413/diff/
>
>
> Testing
> -------
>
> Tested on a live file. It displays calendars and gantt charts correctly.
>
>
> Thanks,
>
> Alvaro Soliverez
>
>
[Attachment #5 (text/html)]
<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 \
solid;"> <tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://git.reviewboard.kde.org/r/111413/">http://git.reviewboard.kde.org/r/111413/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: \
10px;"> <p style="margin-top: 0;">On November 23rd, 2013, 11:04 p.m. UTC, \
<b>Friedrich W. H. Kossebau</b> wrote:</p> <blockquote style="margin-left: 1em; \
border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; \
border-collapse: collapse; margin: 2px padding: 2px;"> <thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; \
font-size: 9pt; padding: 4px 8px; text-align: left;"> <a \
href="http://git.reviewboard.kde.org/r/111413/diff/1/?file=169234#file169234line28" \
style="color: black; font-weight: bold; text-decoration: \
underline;">3rdparty/kdgantt/kdganttdatetimegrid.cpp</a> <span style="font-weight: \
normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">28</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="cp">#include <kcalendarsystem.h></span></pre></td> </tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" \
align="right"><font size="2"></font></th> <td bgcolor="#c5ffc4" width="50%"><pre \
style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td> <th \
bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid \
#C0C0C0;" align="right"><font size="2">29</font></th> <td bgcolor="#c5ffc4" \
width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span \
class="cp">#include <kdeversion.h></span></pre></td> </tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: \
-pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hm. libkdgantt is a copy \
of an external lib that is just not packaged anywhere (and in the next weeks will be \
hopefully updated with a newer version)
And so far it is a Qt-only lib.
But I see the need to change it to use KCalendarsystem, as QDate::weekNumber() seems \
hardcoded to ISO 8601, ignoring any settings done in the KDElibs layer.
And as Plan is the only one using libkdgantt in Calligra, no one else is hurt by also \
linking libkdecore, okay.
Seems you did this patch when libkdgantt was still (accidentally) linked to \
libkdecore, so that has to be added again.
For that change 3rdparty/kdgantt/CMakeLists.txt to also link to ${KDE4_KDECORE_LIBS}, \
so it has lines like: target_link_libraries(calligrakdgantt ${KDE4_KDECORE_LIBS} \
${QT_QTGUI_LIBRARY}) target_link_libraries(calligrakdgantt LINK_INTERFACE_LIBRARIES \
${KDE4_KDECORE_LIBS} ${QT_QTGUI_LIBRARY})
And please also create a separate patch for this change to libkdgantt, which then is \
added to 3rdparty/kdganttpatches/ (hm, actually with git that might not be really \
needed anymore, but let's just do it for the tradition).</pre> </blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; \
white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">And also \
add ${KDE4_INCLUDES} to the include_directories</pre> <br />
<p>- Friedrich W. H.</p>
<br />
<p>On July 6th, 2013, 12:51 a.m. UTC, Alvaro Soliverez wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" \
style="background-image: \
url('http://git.reviewboard.kde.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); \
background-position: left top; background-repeat: repeat-x; border: 1px black \
solid;"> <tr>
<td>
<div>Review request for Calligra.</div>
<div>By Alvaro Soliverez.</div>
<p style="color: grey;"><i>Updated July 6, 2013, 12:51 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="http://bugs.kde.org/show_bug.cgi?id=321290">321290</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
calligra
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" \
style="border: 1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Plan uses week numbering extensively. The default week numbering in KDE \
is ISO Week, which uses Monday as first day of the week, regardless of other locale \
settings.
To make it consistent through the application, if week numbering format is ISO Week, \
set the first day of the week to Monday, and display it accordingly in charts and \
graphs.
Without this patch, the week could be set to start on Sunday, and week would be shown \
on graphs as ranging from Sunday to Saturday, but the week number of working days \
would be off by one.
Example:
- Week numbering: ISO Week
- First day of week: Sunday
Without patch:
- Week displayed: 2013-07-07 to 2013-07-13
- Week number shown on charts: 27
- Expected week number: 28 (Week 27 ends on Sunday 2013-07-07)
With patch:
- Week displayed: 2013-07-08 to 2013-07-14
- Week number shown on charts: 28
Additionally, there were a few calls to QDate::week() to get the week number, instead \
of using KGlobalCalendar.
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: \
1px solid #b8b5a0"> <tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: \
-moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: \
break-word;">Tested on a live file. It displays calendars and gantt charts \
correctly.</pre> </td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>plan/libs/ui/kptganttview.cpp <span style="color: grey">(81073a3)</span></li>
<li>plan/libs/models/kptaccountsmodel.cpp <span style="color: \
grey">(71c3100)</span></li>
<li>plan/libs/models/kcalendar/kdatetable.cpp <span style="color: \
grey">(b389345)</span></li>
<li>3rdparty/kdgantt/kdganttdatetimegrid.cpp <span style="color: \
grey">(79c405c)</span></li>
</ul>
<p><a href="http://git.reviewboard.kde.org/r/111413/diff/" style="margin-left: \
3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>
_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic