<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, 8:21 a.m., <b>Sebastian Trueg</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#file40246line607" 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">607</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">QString</span> <span class="n">dayOfYearString</span><span class="p">(</span><span class="n">KCalendarSystem</span><span class="o">::</span><span class="n">StringFormat</span> <span class="n">format</span> <span class="o">=</span> <span class="n">KCalendarSystem</span><span class="o">::</span><span class="n">LongFormat</span><span class="p">)</span> <span class="k">const</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;">an example would be good here since it is rather unclear what the method (and its friends below) return.</pre>
</blockquote>
<p>On October 26th, 2010, 9:48 p.m., <b>John Layt</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;">I've reworked the apidox. I'm now rethinking the methods entirely. The KCalendarSystem api for yearString() and the rest is needed because you can't just display the year() number in the gui, the localized string form may have a different digit set or even a non-decimal or non-numeric form (Hebrew is the big case here). However, the more recent addition of a formatDate() version to the KCalendarSystem api where you can pass in your own format string (e.g. "%Y" for the long year) makes the reason for having them moot. I could remove them entirely, but the whole purpose of KLocalizedDate is to provide localized versions in a simple way. Perhaps slightly modify the name to remove the "String" and remove the default value to look like:
int year() const;
QString year(KCalendarSystem::StringFormat format) const;
I think that might make it more obvious?</pre>
</blockquote>
<p>On October 29th, 2010, 3 p.m., <b>Sebastian Trueg</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;">why not formatYear() to match the big brother method?</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;">Nice, I'll take that. I was a little uneasy about using the same method name with different return types. Also allows me to set default values too.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On October 26th, 2010, 8:21 a.m., <b>Sebastian Trueg</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#file40246line736" 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">736</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">KDate</span> <span class="n">readDate</span><span class="p">(</span><span class="k">const</span> <span class="n">QString</span> <span class="o">&</span><span class="n">dateString</span><span class="p">,</span> <span class="n">bool</span> <span class="o">*</span><span class="n">ok</span> <span class="o">=</span> <span class="mi">0</span><span class="p">)</span> <span class="k">const</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;">Confusing that this is not a static method.</pre>
</blockquote>
<p>On October 26th, 2010, 10:36 p.m., <b>John Layt</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;">I had initially made it static, but I wondered if there would then be confusion as to what Calendar System would be used if accessed from an instantiated KDate rather than statically, e.g. called as myDate.readDate() rather than KDate::readDate(). Would the user expect the instantiated date's calendar system to be used rather than the global that it really would? Or should having clear documentation be enough?</pre>
</blockquote>
<p>On October 29th, 2010, 3:02 p.m., <b>Sebastian Trueg</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;">IMHO a static version with an optional KCaldendarSystem parameter might do the trick. Although the latter might even be superfluous since using a custom caldendarsystem is not a simple usage pattern anymore in which case it would be ok for the developer to go back to using KCalendarSystem.</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;">Yep, I'll probably go with the basic static and a big flashing neon warning :-) The other problem with passing in a calendar system to use is you are likely to be wanting a different locale with it too, i.e. different language or date format, so that's better using KCS anyway.</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>