<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://svn.reviewboard.kde.org/r/5692/">http://svn.reviewboard.kde.org/r/5692/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 26th, 2010, 1:35 p.m., <b>David Jarvie</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Looks a good piece of work. I have only had time so far to look at the header file.
The main issue is that it needs a class level comment to explain what the class is for. One extra item which the class comment could have would be an explanation of how QDate relates to a KDate using the Gregorian calendar and western locales.
I'm also concerned about the handling of weeks - is the ISO week the only week which exists, or do some calendar systems have alternatives? If so, some revamping might be needed.</pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Thanks David. The apidox was largely a copy-and-paste from the old KCalendarSystem apidox which is not great in places. I'm in the process of re-writing it all to a higher standard, I'm just posting some interim replies as it's taking a while :-)
Good point on the week numbers, I do plan to implement support for other week numbering systems (US for example), so I might as well get the api for it right now. I think people should be able to cope with the difference from QDate / KCalendarSystem functionality.
That's actually got me to thinking, while I've tried to be consistent with the QDate/KCalendarSystem api, perhaps it would be better to fix a couple of other bad bits of api I inherited?</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 26th, 2010, 1:35 p.m., <b>David Jarvie</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://svn.reviewboard.kde.org/r/5692/diff/1/?file=40246#file40246line307" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/KDE/kdelibs/kdecore/date/kdate.h</a>
<span style="font-weight: normal;">
(Diff revision 1)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">public:</pre></td>
</tr>
</tbody>
<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">307</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">bool</span> <span class="n">addYearsOn</span><span class="p">(</span><span class="kt">int</span> <span class="n">years</span><span class="p">);</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;">I wonder about the method name. Possibly addYearsTo() instead? Similarly for addMonthsOn() and addDaysOn().</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;">Done.</pre>
<br />
<p>- John</p>
<br />
<p>On October 25th, 2010, 8:54 p.m., John Layt wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://svn.reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs.</div>
<div>By John Layt.</div>
<p style="color: grey;"><i>Updated 2010-10-25 20:54:08</i></p>
<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;">The KCalendarSystem api for localizing dates is awkward, inconvenient, unintuitive, and long-winded, causing many mistakes to be made or localization to be ignored altogether. This change adds a new KDate class designed to make localizing dates as easy as using QDate.
Some QDate code may look like:
QDate myDate( aYear, aMonth, aDay );
int doy = myDate.dayOfYear();
The KDE localized date code looks something like:
QDate myDate;
KGlobal::locale()->calendar()->setDate( myDate, aYear, aMonth, aDay );
int doy = KGlobal::locale()->calendar()->dayOfYear( myDate );
The localized KDate code would look like:
KDate myDate( aYear, aMonth, aDay );
int doy = myDate.dayOfYear();
Much easier.
KDate is a thin wrapper around KCalendarSystem and QDate, with a near identical api to QDate and as such can be used as a drop-in replacement with very few changes. Some deprecated or unnecessary KCalendarSystem methods have not been included, but these can still be accessed via the calendar() methods. Some new convenience methods have also been added such as setCurrentDate() and addYearsOn().
Some methods have QDate overloads for convenience, and the assignment and comparison operators partially work with QDate on the rhs. If anyone knows how to make it work with QDate on the lhs, or any other QDate compatibility ideas, I'm all ears.
For now I only intend this to be used as a convenience class by apps internally and not to be used in kdelibs api as I don't see much advantage in that, but I may do so if the demand for convenience is there.
I have named it KDate, but there is the possibility people may get confused and think that KDateTime also localizes datetime's, but that is not the case. If people think this will be a problem KLocalizedDate is an alternative if more awkward name.</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;">Full unit tests included.</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>/trunk/KDE/kdelibs/kdecore/CMakeLists.txt <span style="color: grey">(1189756)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kdate.h <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/date/kdate.cpp <span style="color: grey">(PRE-CREATION)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/tests/kcalendartest.h <span style="color: grey">(1189756)</span></li>
<li>/trunk/KDE/kdelibs/kdecore/tests/kcalendartest.cpp <span style="color: grey">(1189756)</span></li>
</ul>
<p><a href="http://svn.reviewboard.kde.org/r/5692/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>