DevHeads.net

Replacement for KDateTime

Hello,

the kdepim is discussing how we can replace KDateTime inside kdepim (primarly
kcalcore) . There are some issues, why we can't replace it simply with
QDateTime.

* indivual timezone support, this is something that we need when parsing ical
and have no known timezone information. I havn't looked into it, but I think
this can make it eventually into QDateTime.

* dateOnly support is used a lot. In ICAL it differs if you set dateonly or
datetime. dateonly events f.ex. lasts a day without timezone info f.ex. "New
Year" is a dateonly event that lasts one day - it everywhere startsat 1.
january at 0:00 o'clock and ends at 24:00 o'clock in your current timezome.
So it a different UTC time for every place at the world. That's why you can't
replace it with a datetime event, that needs a timezone, so it would be
wrongly displayed at other timezones.

The core problem is with that that QDateTime is invalid if either the date or
the time is invalid. KDateTime is valid also with invalid time. And this is
behaviour change cannot go into QDateTime, that's why we need anything to
replace KDateTime.

The solutions we are were came along at akademy:

* using QDateTime in a inalid way and check on our own if time&date are valid
- This approch would lead to many errors, because every application must be
aware, that qdatetime is used in a "strange way"

* split the API everywhere into date and time members/functions -> makes it
much more worse to use the API, because you have to request everytime both
elements.

* keep KDateTime and transform it to an own framework -> this is a very big
overhead for only one feature

As midterm solution we came up to split KDateTime to an own lib inside
kde4support, so that we can acctually get rid of linking to every other lib,
that would help to make the start faster and the memory footprint smaller.

Nothing is hammered in stone and I want to start a disscussion, maybe others
have good suggestions how we can solve this issues.

Just for clearifing it- it is only a discussion about how we should transform
the public API of kcalcore.

Regards,

sandro

PS: I had a very nice time attending the akademy!

Comments

Re: Replacement for KDateTime

By Alex Merry at 08/02/2015 - 07:22

On Saturday 01 August 2015 20:47:16 Sandro Knauß wrote:
Making it its own framework seems like an odd approach for something which
seems to be quite a calendar-specific thing. Could you not put an approriate
class directly into KCalCore? This could basically be KDateTime, or it could
be something that's basically a union of QDateTime and maybe a QDate?

This seems like it could be binary incompatible if not done with great care.

Alex

Re: Replacement for KDateTime

By John Layt at 08/02/2015 - 09:26

This is the real problem and the biggest stumbling block,and I'm sorry
I haven't had the time to come up with a solution as promised. I'll
try sort it out in the next week or so. It was something I wanted in
QTimeZone but it was far to complex to get in the first version., or
indeed maybe ever. I need to see if there's a way we can derive our
own support separate to Qt.

I've been over this before on the PIM list, it's a fundamental mistake
to say that 'Date Only' is an attribute of the datetime, it is in fact
an attribute of the Event itself, and indeed is in the current API as
such. I fail to see why it needs to be in the datetime as well? If
there is anywhere using the datetime property instead of the Event
property it is doing it wrong, make it check the Event instead. Do not
put it back in the datetime. Use the datetime to represent the valid
start time or endtime of the all day event, i.e. 00:00 in the time
zone of the datetime on which the event starts or finishes.

You do *not* want to do this, we specifically got rid of KDateTime to
be compatible with Qt and other apps using Qt to attract new users and
devs. If you do this you also need all of KLocale again which we also
do not want. Don't even go there, we changed it for a purpose.

If you plan to release KCalCore as a Framework, then it must be free
of KDateTime and KLocale in the API, otherwise no-one else can use it
and you're locked-in with a bad API.

John.

Re: Replacement for KDateTime

By John Layt at 08/04/2015 - 19:38

OK, I had a quick look at this and to do it in Qt and it wasn't actually
that hard. You can see the result at:

<a href="https://codereview.qt-project.org/#/c/122750/" title="https://codereview.qt-project.org/#/c/122750/">https://codereview.qt-project.org/#/c/122750/</a>

This should support the full set of VTIMEZONE rules, have a look and let me
know if not.

I'm not sure if it will be ready for 5.6 feature freeze on Friday, or if
Thiago will approve of it, but clearly it's not going to be available when
KCalCore is released. I'm still thinking about work-arounds, but it's going
to be messy internally at the very least, and may not be do-able.

John.

Re: Replacement for KDateTime

By Sandro =?ISO-88... at 08/05/2015 - 03:15

Hey John,

Many many thanks getting things done! That will make our life easier to port
KCalCore away from KDateTime.

Regards,

sandro

Re: Replacement for KDateTime

By Martin Klapetek at 08/02/2015 - 11:08

Fwiw, over the year(s) of Plasma5, many times it was expressed
that KLocale is greatly missed, especially when it comes to date/time
formatting. QLocale just doesn't cut it and the digital clock applet is
doing many tricks to get the stuff formatted as wanted. The biggest
downside is that it applies to the clock applet only, so there can't be
a single system-wide "Use 24h-clock format" switch. Clock in panel
can/will be 24h format while everything else will be LC_TIME format
(time stamps in Dolphin eg).

There are some other examples where KLocale was missed in Plasma
but I can't remember which ones that was right now, maybe someone
will fill it.

Maybe it could be worth bringing KLocale back in some limited form
as an intermediate solution?

Alternatively, do you John have any roadmap about QLocale? Perhaps
we could help with filling the missing bits into QLocale directly too.

Cheers

Re: Replacement for KDateTime

By John Layt at 08/03/2015 - 16:36

[Reply sent again from subscribed account]

On 2 August 2015 at 16:08, Martin Klapetek <martin. ... at gmail dot com> wrote:

Yes, KLocale was in many ways the best localization library around, I
and others worked hard to make it that good, but ultimately it's one
that lost because it made KDE a walled garden and failed to play well
with others. ICU is an ugly mess, but it's a supported, sponsored,
ubiquitous mess that is the industry standard. Sometimes you just have
to take a stepp backwards in order to be able to move forward.

For a little bit more background on why see
<a href="https://community.kde.org/KDE_Core/KLocale" title="https://community.kde.org/KDE_Core/KLocale">https://community.kde.org/KDE_Core/KLocale</a> and
<a href="https://community.kde.org/KDE_Core/Platform_11/Locale" title="https://community.kde.org/KDE_Core/Platform_11/Locale">https://community.kde.org/KDE_Core/Platform_11/Locale</a>

People are free to liberate it from kdelibs4support and embed it
themselves, but it's a lot to ship and maintain, and at the end of the
day you end up with localization that doesn't match what the rest of
the system does.

The roadmap was waiting on two things: me to get motivated to work on
Option 3 after burning out on the first two rejected options, and for
Qt to drop Windows XP/Vista support. We can't get new Locale stuff
into Qt that only works on a couple of platforms, it has to work on
every platform from day 1 with some lowest common denominator of
features, and Windows XP was so pathetic we couldn't do much more with
it than QLocale already does. With XP/Vista gone, we can target a more
useful feature set. But even once all that is implemented, I still
have to convince Lars and Thiago to accept it :-)

I've recently been trying to update the plan at
<a href="http://wiki.qt.io/Locale_Support_in_Qt_5" title="http://wiki.qt.io/Locale_Support_in_Qt_5">http://wiki.qt.io/Locale_Support_in_Qt_5</a> for those interested, it
provides a lot of background detail about what we want to do and why
it has been so hard to achieve. Frankly, 18 months ago I had been
looking for sponsorship to implement all this as it is a massive
undertaking, but it's one of those things everyone wants to just work
but no-one is interested in paying to be done right.

Re: Replacement for KDateTime

By John Layt at 08/03/2015 - 16:35

On 2 August 2015 at 16:08, Martin Klapetek <martin. ... at gmail dot com> wrote:

Yes, KLocale was in many ways the best localization library around, I
and others worked hard to make it that good, but ultimately it's one
that lost because it made KDE a walled garden and failed to play well
with others. ICU is an ugly mess, but it's a supported, sponsored,
ubiquitous mess that is the industry standard. Sometimes you just have
to take a stepp backwards in order to be able to move forward.

For a little bit more background on why see
<a href="https://community.kde.org/KDE_Core/KLocale" title="https://community.kde.org/KDE_Core/KLocale">https://community.kde.org/KDE_Core/KLocale</a> and
<a href="https://community.kde.org/KDE_Core/Platform_11/Locale" title="https://community.kde.org/KDE_Core/Platform_11/Locale">https://community.kde.org/KDE_Core/Platform_11/Locale</a>

People are free to liberate it from kdelibs4support and embed it
themselves, but it's a lot to ship and maintain, and at the end of the
day you end up with localization that doesn't match what the rest of
the system does.

The roadmap was waiting on two things: me to get motivated to work on
Option 3 after burning out on the first two rejected options, and for
Qt to drop Windows XP/Vista support. We can't get new Locale stuff
into Qt that only works on a couple of platforms, it has to work on
every platform from day 1 with some lowest common denominator of
features, and Windows XP was so pathetic we couldn't do much more with
it than QLocale already does. With XP/Vista gone, we can target a more
useful feature set. But even once all that is implemented, I still
have to convince Lars and Thiago to accept it :-)

I've recently been trying to update the plan at
<a href="http://wiki.qt.io/Locale_Support_in_Qt_5" title="http://wiki.qt.io/Locale_Support_in_Qt_5">http://wiki.qt.io/Locale_Support_in_Qt_5</a> for those interested, it
provides a lot of background detail about what we want to do and why
it has been so hard to achieve. Frankly, 18 months ago I had been
looking for sponsorship to implement all this as it is a massive
undertaking, but it's one of those things everyone wants to just work
but no-one is interested in paying to be done right.

John.

Re: Replacement for KDateTime

By Thiago Macieira at 08/02/2015 - 13:36

On Sunday 02 August 2015 17:08:04 Martin Klapetek wrote:
If the format you're looking for is in the CLDR, you're welcome to add the
support to QLocale.

If the format you're looking for requires support from translators, please add
a new class to QtCore.

The roadmap currently stands as follows:

Re: Replacement for KDateTime

By Martin Klapetek at 08/03/2015 - 02:33

It's not really about any missing locale, it's about setting different
parameters
for the given locale. For example David Edmundson (a brit) repeatedly tells
me
that en_GB (cldr) uses "12h clock format but pretty much any clock is
guaranteed
to be 24h clock format".

So what is missing/wanted is telling QLocale to use en_GB *but* return
any time string in 24h format for example. Or to use ISO date format
by default. The stuff coming from cldr might not always be what
the user wants. And there's no easy and global way to override it
other than setting different LC_TIME value, which is less than ideal
as it can give you 24h clock format but will leave you with different
date format too, so it's a hit'n'miss game.

If the format you're looking for requires support from translators, please
Suppose there's such QLocale setting as described above, then it would
be just a matter of some regexp inside the time formatting function which
would add/remove the "ap"/"AP" strings for the clock. I imagine.

Cheers

Re: Replacement for KDateTime

By John Layt at 08/03/2015 - 16:57

There's code that does that floating around in KDE or Qt, can't recall
where, but it is fraught with problems in unconventional languages.
You also need to replace the hh with HH to get the 24 rather than 12.

But it's a hard rule in QLocale: no overriding default settings
provided by the system, as those are the system settings and what the
user expects you to use, e.g. on Windows and Mac you can switch from
12 to 24 hours and have it respected by Qt. If you want a different
format, then pass a custom pattern each time.

The problem actually is that Plasma is not considered a system
platform by QLocale, it doesn't go looking for what Plasma wants, it
just uses the underlying GNU/Linux system settings. Convince Qt to
hard-code in a lookup of some Plasma config file with user overrides
to be applied whenever Qt apps run under Plasma and then you can get
what you need. I had plans somewhere for how that would work, and how
we could also make Gtk apps use our settings too by manipulating the
LC_* settings.

John.

Re: Replacement for KDateTime

By Ben Cooksley at 08/04/2015 - 01:52

On Tue, Aug 4, 2015 at 8:57 AM, John Layt < ... at kde dot org> wrote:
I'm assuming QLocale isn't routed through the platform plugin like
other parts of Qt do in these cases, so we can't manipulate things
there?

Ben

Re: Replacement for KDateTime

By Thiago Macieira at 08/04/2015 - 02:17

On Tuesday 04 August 2015 17:52:18 Ben Cooksley wrote:
Right.

If a LC_TIME solution isn't possible, we could consider a QSystemLocale
configuration that allowed the GUI platform plugin to set some details.

Obviously this would not apply to non-GUI apps.

Re: Replacement for KDateTime

By Thiago Macieira at 08/03/2015 - 22:03

On Monday 03 August 2015 21:57:36 John Layt wrote:
That's not going to happen.

The only solution I'm going to accept is one that also applies to /bin/ls.

So far, the idea of setting LC_TIME to a file containing the rules is the only
solution I've seen to accomplish that.

Re: Replacement for KDateTime

By John Layt at 08/04/2015 - 05:45

On 4 August 2015 at 03:03, Thiago Macieira < ... at kde dot org> wrote:
Reading the contents of a POSIX format file and then converting the
POSIX format to CLDR format seems broken to me given the difference in
features supported and the different format codes that do not exactly
map one-to-one. Reading a CLDR json file seems less work with a more
reliable result and long-term may be a way to offer the custom locale
feature or minimised embedded platform support without our shipping
all the data internally.

The fundamental mis-match between POSIX and CLDR/ICU is at the heart
of many locale issues on Linux. The failure to keep POSIX up to date
with new features while still keeping it at the core of much of the
underlying Linux infrastructure is frustrating. In an ideal world CLDR
would become the sole source of data files and things like glibc would
at least switch to using the shared data resources or better yet a
single specialised localization library (preferably someone would pay
for it not to be ICU!). Anyway, that's all a pipe-dream, it's defeated
greater minds than mine, we just have to accept that some scenarios
will always be broken and try to make the rest look as good as
possible.

Most of this discussion is very very off-topic, it belongs on the Qt
development list, can we get back to the main topic of KCalCore and
QDateTime? I have limited time to spare and I'd rather use it to solve
the immediate problems that we can fix.

John.

Re: Replacement for KDateTime

By John Layt at 08/04/2015 - 09:48

OK, I stole some time from work to look at KCalCore and uses of dateOnly.

There are 16 calls to isDateOnly() or setDateOnly(), 7 of which can be
removed with no effect.

Of the remaining 9, 6 are used internally in private iCal parsing and
while slightly messy can be worked around OK with a flag or enum or
struct.

The remaining 3 uses are inside the IncidenceBase::setDtStart() and
Recurrence::setStartDateTime() public methods, if the passed in
datetime is dateOnly() then the allDay() incidence flag gets set. We
can either add an extra parm to the methods to take a flag, or just
require the calling of setAllDay() in addition. There are about 21
calls to setDtStart() inside KCalCore that would need to be analysed
for impact.

Given the size of the other changes required around KDateTime (time
spec / time zone especially), this part is rather small. The
implications are bigger, as you have to think about all the code
outside KCalCore that calls setDtStart() and dtStart(), but the change
from KDT to QDT will affect them all anyway and we will have masses of
testing to do anyway to cover everything.

John.

Re: Replacement for KDateTime

By Sergio Luis Martins at 08/09/2015 - 11:25

On Tuesday, August 04, 2015 14:48:02 John Layt wrote:
Hi,

I think QDateTime is fine, it has a date and a time part just like KDateTime.
We just need to decorate it a bit to get the semantics we need.

Like:

namespace KCalCore {
class DateTime : public QDateTime { // Or composition instead of inheritance
bool isValid() const { return date.isValid(); }
bool isDateOnly() const { return date.isValid() && !time.isValid(); }
};
}

This is imho the simplest solution, isValid() is called *all* over the place
in kdepim, any another approach will be a lot of work.

P.S.: can we drop <a href="mailto:kde-core- ... at kde dot org">kde-core- ... at kde dot org</a> from this thread now? Since we're not
moving KDateTime to a separate library anymore.

Regards,
Sérgio Martins

Re: Replacement for KDateTime

By Thiago Macieira at 08/03/2015 - 12:34

On Monday 03 August 2015 08:33:54 Martin Klapetek wrote:
How would that setting be set? Who would set it? Is that something that the
clock app would use and allow the user to set?

If so, why not extract the time format and show that to the user? Or, at
least, parse it to set the default of "Use 24-hour clock"? If the user wants
to toggle 24-hour, save the new time format in the config file and use it to
format the time next time.

It means the roadmap is empty.

Re: Replacement for KDateTime

By Martin Klapetek at 08/03/2015 - 13:09

I may have explained it poorly, so let me try again. tl;dr at the bottom :)

I can put a checkbox to the Plasma panel clock which would either
enable or disable 24h clock format. And in fact we do exactly that.
If the user wants to see 24h clock format in the panel, he can check
that checkbox. If he wants AM/PM clock, he can uncheck the checkbox.
Simple as that.

The implementation takes the Qt.locale().timeFormat(Locale.ShortFormat)
(it's QML), checks for the 24h clock option and either adds "AP"
or removes "AP" from the format string returned by Qt.locale().
The actual clock on the panel is then printed using
Qt.formatTime(currentTimeObject, thatModifiedTimeFormatString).

Now, this will affect _only_ the Plasma panel clock. This will _not_
affect Dolphin's time stamps, Gwenview's picture-taken times,
this won't affect anything else but the panel clock and the panel clock
alone. But if I want to have 24h clock format, I want it everywhere,
in all apps, system-wide, not _only_ in the panel clock. Everything
else will be however using time formatted by the LC_TIME locale
default format.

Currently each and every application would have to implement similar
code as the Plasma panel clock - get the locale's time format string,
check for some config, add/remove "AP" from the format string, print
the time using QDateTime().toString(theModifiedTimeFormatString).
Or each application would have to read the date/time format string
from somewhere, some global config in order to have the same time
format everywhere.

Is it clear now? Do you see what I'm getting at?

Setting different LC_TIME values proven to not be feasible, because
very often users want "just" 24h clock format _and_ their locale's
date format. Or some date format and their locale's time format.

Ideally there would be a global setting, set from System Settings,
perhaps written into ~/.config/QtProject.conf, which QLocale could
internally read and return the time formatted according to that setting
already. This would spare every single application doing it on its own.
Then, calling QDateTime().toString() or QLocale().toString(QDateTime)
would return the time formatted by user preference already and it would
return it in all apps, uniformly, same format.

I guess such feature might not be feasible for QLocale, hence my
suggestion to bring back KLocale in a limited form, purely for global
date/time formatting purposes around KDE apps, to have the same
date/time format everywhere around.

As a side note, missing global date/time format setting is a regression
from the kde4 times.

tl;dr - it'd be nice to be able to set a custom time format for all
QDateTime/
QLocale users without them needing to do anything at all.

Hope it's clear now :)

Cheers

Re: Replacement for KDateTime

By davispuh at 08/03/2015 - 16:18

2015-08-03 20:09 GMT+03:00 Martin Klapetek <martin. ... at gmail dot com>:

What about non-Qt applications? I was thinking about this and actually it would
be a workaround for LC_ locales which are too limiting. Currently locale
can be in format language[_script][_country][.codeset][@modifier] which
represents country's standard but not user's preferences like use of 24h clock
instead of 12h.

Using @modifier for such user preferences could be one of solutions by
creating locale like en_US.UTF-8@24h and it would work for all applications
(note that currently QLocale ignores modifier) but what when need some other
preferences, it would become too complex as even currently locales like
de_DE@euro are used.

So better would be to introduce new variable LC_SETTINGS which would
contain a list of user preferences that override some aspects of locale and
user could use different settings than his country's locale as
currently only way
is to use different countries locale. But with such variable, user, for example,
could specify LC_SETTINGS="24h:euro" and no matter which locale
(eg. even en_US) he would always get 24h clock with euro as currency. And
mostly only libc and Qt would need this change as applications which
use standard functions like strftime would work correctly as it would be done
automatically.

Re: Replacement for KDateTime

By =?utf-8?Q?Thoma... at 08/03/2015 - 14:03

Disclaimer: I may talk nonsense.

What about exporting LC_TIME=KDE and have a ~/.local/share/i18n/locales/KDE which can be configured from the locale kcm.

This way *all* applications (including even mc ;-) would follow the pattern the user configures there. (Except the KDE SC4 applications, but we could probably write the legacy config for them)

The major pitfall (what I don't know how to do) is:
"how to get userpath locale definitions recognized"?

Cheers,
Thomas

Re: Replacement for KDateTime

By John Layt at 08/03/2015 - 17:17

[Again resend from subscribed account, damn gmail!]

Not nonsense, exactly my long-term plan to get Gtk and other apps to
use the right Plasma locale settings when running under Plasma. It
'just' requires the kcm to learn how to write and compile a POSIX
locale file.

One problem is Qt completely ignores the contents of that file, and
indeed I'm not sure it would even manage to extract the correct locale
name even so you'd end up with some default.

John.

Re: Replacement for KDateTime

By =?utf-8?Q?Thoma... at 08/04/2015 - 04:11

That actually doesn't seem to be a really big deal - one would keep a set of adjustments and merge them into a copy of the base-de-toujours from /usr/share/i18n/locales/

Compilation could (should?!) then be done by localedef and I figured (and tested ;-) the relevant env vars to operate on local definitions are I18NPATH and LOCPATH.

One problem would be to update running processes (QLocale to track the locale file and emit some signal on changes)

Humm?

QLocale::name() should please return "KDE" in this case (where we may be a little smarter and name it en_KDE or de_KDE etc.)

Where would that fail?

Cheers,
Thomas

Re: Replacement for KDateTime

By John Layt at 08/04/2015 - 05:03

On 4 August 2015 at 09:11, Thomas Lübking <thomas. ... at gmail dot com> wrote:
You would need code to decompile and parse the source file, then code
to cleanly change the required values, then code to write it out in
the format the compiler wants. All achievable, the command line tools
may well support it, but still work that needs doing, and the last
step along the long path to locale nirvana once many more pressing
issues are solved. And when the underlying file gets updated you'll
need a mechanism to spot that and update the derived file too.

This is one of the areas Qt doesn't do well, on Windows and Mac soon
as the underlying locale is change you get different results from
QLocale, leading to inconsistent looking UI and weird bugs. On Linux
it doesn't respond to changes at all until the app restarts, a more
consistent approach. The best approach would be to have a QEvent that
the app can catch and know to refresh everything when it is ready to
do so. Again, something on the plan but a longer term issue to solve,
as monitoring locale files or envvars or a registry for changes across
multiple platforms in Qt would take some work.

Well, QLocale wouldn't extract a valid language and country code from
it correctly to look up in it's internal locale database, so it would
fallback to using C locale instead. The name should also be standards
compliant so everything else would read it properly too, i.e. be valid
ISO codes etc, so something more like 'en_GB_kde'. And even once
around the name problems, QLocale would still be using its internal
data based on CLDR and not the override data held in the locale file
we point to. Nor do we want to use the data in the POSIX file as it
has too many shortcomings and lacks features and simply uses a
different set of format codes to CLDR. That POSIX file can only ever
really be used to tell non-Qt POSIX locale using apps what settings to
use under Plasma (i.e. Gtk/glibc stuff). If we're going to use a
locale file for Qt internally we'd be better looking to load the CLDR
json format instead. Indeed, it's an option I had been thinking of to
replace Qt's custom locale feature once we stop shipping our internal
CLDR data, or to allow embedded platforms to choose what supported
locales to ship, but there are issues there and I'm not sure it's
something Thiago would buy into.

John.

Re: Replacement for KDateTime

By Thiago Macieira at 08/04/2015 - 10:54

On Tuesday 04 August 2015 10:03:20 John Layt wrote:
Ugh...

If the date/time format in the POSIX locale is different, there are probably
other differences too. I withdraw my requirement to make /bin/ls work then.

What's your suggestion to make this work for a KDE environment and
applications, John?

Re: Replacement for KDateTime

By Thiago Macieira at 08/04/2015 - 10:52

On Tuesday 04 August 2015 10:03:20 John Layt wrote:
The problem with that is a thundering herd. All applications suddenly start
updating, even if they haven't got anything to show.

It's the same problem and requires the same solution as the language itself.
There are mechanisms, but they're neither very well-used and they cause
thundering herd.

Re: Replacement for KDateTime

By John Layt at 08/03/2015 - 17:15

Not nonsense, exactly my long-term plan to get Gtk and other apps to
use the right Plasma locale settings when running under Plasma. It
'just' requires the kcm to learn how to write and compile a POSIX
locale file.

One problem is Qt completely ignores the contents of that file, and
indeed I'm not sure it would even manage to extract the correct locale
name even so you'd end up with some default.

John.

Re: Replacement for KDateTime

By Thiago Macieira at 08/03/2015 - 22:01

On Monday 03 August 2015 22:15:51 John Layt wrote:
We just need to fix QSystemLocale and make it fall back to the C API if the
locale is a file or one not found in the database it carries.

Re: Replacement for KDateTime

By David Jarvie at 08/02/2015 - 14:32

On Sunday 02 Aug 2015 14:26:18 John Layt wrote:
Date-only KDateTime instances are not only used for Event start/end timestamps. In KAlarm they are also used among other things for alarm snooze times (independently of whether the event is date-only or not). So usage of the date-only attribute is *not* restricted to Events (even if that is all it is used for in kcalcore).

Having a date-only attribute in KDateTime is very useful because it allows both date-time and date-only values to be encapsulated in a single class. This avoids having to be able to pass either a QDate or QDateTime or to have a separate flag everywhere this is used, and it allows date-time values to be compared to date-only values using class methods. Without this encapsulation, it would make a significant amount of code more cumbersome. KAlarm for one will certainly need the equivalent of KDateTime even if it has to have its own class, but given its use in kcalcore I think it should be available in a library. Also, do you know that date-only is NOT used elsewhere (apart from kcalcore and KAlarm)?

In order to use QDateTime in the representation of date-time or date-only values, the best solution in my opinion would be to have a new date time class which contains a QDateTime member together with a bool member indicating whether it is date-only. For a date-only value, the time component of the QDateTime should be set to 00:00:00, to avoid the problem of what an invalid value indicates.

Re: Replacement for KDateTime

By davispuh at 08/03/2015 - 07:59

2015-08-02 21:32 GMT+03:00 David Jarvie < ... at kde dot org>:
Sorry for injecting myself, but IMO there's no such thing as Date-only and
what you need is something like QDateTimeRange (just made up) where you would
have start QDateTime, end QDateTime and it could represent any Event/Interval.
Like whole day, part of day or even multiple days. And could also check whether
some QDateTime is inside this range.

Re: Replacement for KDateTime

By Sergio Luis Martins at 08/09/2015 - 11:03

On Monday, August 03, 2015 14:59:59 Dāvis Mosāns wrote:
Hi,

iCalendar RFC (which KCalCore implements) distinguishes between all day events
and events with time. A time interval going from 00:00:00 to 23:59:59 is not
equivalent to a date only event, so we can't go that route.

Regards,
Sérgio Martins

Re: Replacement for KDateTime

By David Jarvie at 08/03/2015 - 08:26

On Monday 03 Aug 2015 12:59:59 you wrote:
Date-only extends the current QDateTime concept to allow it to represent a single date (which is intrinsically a time range) or a single date-time, without requiring any extra date-time information to be stored - only a boolean flag is required to indicate whether the time component should be ignored.

A generalised time range on the other hand requires storing distinct start and end times, and cannot (except for special cases) be represented by a single date or date-time.

Re: Replacement for KDateTime

By davispuh at 08/03/2015 - 09:29

2015-08-03 15:26 GMT+03:00 David Jarvie < ... at kde dot org>:
That sounds like kind of a hack... trying to save few bytes for very
minimal use case.
As soon as you'll need to represent something outside of that you'll
need a proper range.
Besides can implement range with QDateTime + qint64

Re: Replacement for KDateTime

By David Jarvie at 08/03/2015 - 10:33

On Monday 03 Aug 2015 14:29:41 Dāvis Mosāns wrote:
My point is that a QDateTime already contains both a date and a date-time. A date-only option provides the option to select one or the other. QDateTime does not contain a range, so a time range is something different (except for the special case of a single date). Adding a flag to select data which is already there is IMHO different in scope from adding extra data (either another QDateTime or a qint64).

Re: Replacement for KDateTime

By davispuh at 08/03/2015 - 13:22

2015-08-03 17:33 GMT+03:00 David Jarvie < ... at kde dot org>:
Ok, I don't really understand what's your use case and so I don't see why you
would need that. There's QDate and QTime, you can use QDateTime::date() to
extract QDate and for other cases you really do need a range. Sounds
like you want QDateOrTime (also made up), but I think very few applications
would use something like. And IMO that's bad design, because cleaner code
would be with method overloads which take either QDate or QTime.

Re: Replacement for KDateTime

By David Jarvie at 08/03/2015 - 14:58

On Monday 03 Aug 2015 18:22:28 you wrote:
There are a number of cases in kdepim where a date-time or a date can be supplied. Using KDateTime makes the code cleaner - there is no need to provide overloads or to track whether it's date-only when calling multiple layers of functions or classes, because the KDateTime value provides that information.

Re: Replacement for KDateTime

By John Layt at 08/03/2015 - 17:19

Not to mention needing the time zone as well which a QDate doesn't.

John.

Re: Replacement for KDateTime

By John Layt at 08/03/2015 - 16:43

On 2 August 2015 at 19:32, David Jarvie < ... at kde dot org> wrote:
lxr tells me it is only KCalCore and KAlarm. KCalCore uses it about 16
times. If we want KCalCore used outside kdepim then I do think they
need to try and live without the isDateOnly option and find a nice way
of doing it . For KAlarm I don't see a problem with you having your
own KADateTime class to do this.

John.

Re: Replacement for KDateTime

By =?utf-8?Q?Thoma... at 08/03/2015 - 14:49

Sorry, but I don't follow here.

QDateTime *has* a QDate AND a QTime member.
They can independently be null/invalid.

=> With a QDateTime with invalid QTime member, you *have* a QDate-only QDateTime, agreed?

The "problem" would be that

if (kdt.isValid()) {
if (kdt.isDateOnly())
foo();
else
bar();
}

turns into:

if (qdt.date().isValid()) {
if (!qdt.time().isValid())
foo();
else
bar();
}

right?
Doesn't look that horrible.

Now, it however seems to me you'd like to have a flag "butIgnoreTheTime" that hints for some client code behavior, ie. you actually want to carry valid date and time in the object, but in addition have a flag _in_the_object_ to effectively pass a parameter to a function to do something different with this object.

Is this actually correct?

=> That frankly sounds like a bool trap on steroids.

You've an object flying around that may or not have set this flag for some™ ominous reason in the past for some™ function call and different modules might have different requirements for this flag for different reasons... ewwww.

Imo, if you want to embed instructions on what to do with the object inside the object, they should rather be not the least ambiguous, ie. provide dynamic properties or such.

But, ultimately, function parameters belong into the function parameter list - not into object attributes (yes, "unless you're doing some hardcore optimizations or dirty side-channel communication hacks")

Cheers to my 2¢,
Thomas

Re: Replacement for KDateTime

By David Jarvie at 08/03/2015 - 15:07

On Monday 03 Aug 2015 19:49:52 Thomas Lübking wrote:
As I understand it, a QDateTime is invalid if either the date or time component is invalid. People would usually expect that if QDateTime::isValid() returns false, the object must be invalid. So a date-only value in which only the date was valid would make the whole object invalid, which is misleading and would almost certainly lead to mistakes.

Re: Replacement for KDateTime

By John Layt at 08/03/2015 - 17:00

That is correct and it is a long-standing QDateTime behaviour that I
would hate to change in a non-major release due to the huge amount of
code that expects it to work that way. It's just not an option to use.

John.

Re: Replacement for KDateTime

By Thiago Macieira at 08/03/2015 - 22:09

On Monday 03 August 2015 22:00:04 John Layt wrote:
QDateTime::isValid() indicates whether it's a valid date-time pair. It isn't.
The QDateTime object may still contain a valid date or valid time though.

And I agree with John that we can't change it in 5.x and we probably won't
change in 6.0 either because that would lead instead to code that assuming
that a date-only QDateTime equals a full QDateTime at midnight. Or, worse,
somehow parsing a time-only QDateTime as a few hours into Nov 24, -4713, which
leads to severe issues when this time-only QDateTime somehow gets converted to
time_t (out of range).