On Wednesday 17 Aug 2011 19:51:47 Thiago Macieira wrote:
> WinCE is still a supported platform. Don't remove that.
Right, I'll put it back in.
> > * Change inline methods to normal methods
>
> Why?
> > * Add Private classes to QDate and QTime
>
> I don't think we need them.
In Qt4 we couldn't fix the problems and bugs in QDate because of the inlines
and lack of private classes as it would have broken BC. I don't want to make
that mistake and lock us into the chosen implementation for all of Qt5. I
can't guarantee that everything will get in for 5.0 or that I won't need to
change anything later. Removing inlines and using private classes gives me
some room for fixing mistakes and adding features later if needed.
Of course, if the design changes from having a QCalendarSystem embedded in the
QDate, to having a vastly simplified QDate with wrappers then this may be moot
(see below).
> Yeah. It's not like 32 extra bits would make much of a difference, though.
> I'd go for the safer assumption and just go with qint64.
I need no further encouragement :-)
> > The definition of isValid() is now that the date falls inside the range
> > supported by the calendar formulas for correctly converting the jd into a
> > ymd format, so for the current formulas 4800 BC to 1,400,000 AD.
>
> I'd just use jd != std::numeric_limits<qint64>::min() (that is,
> Q_INT64_C(-0x8000000000000000).
For isNull() that looks a nicer way.
The reason for having isValid() being the formula range is that the formulas
don't support the full jd range and using any of the year()/month()/day()
methods or toString() outside this valid range will return random invalid
results. We need a way to tell devs that this has happened and isValid()
seems the logical place. For most uses the formula range is all they need,
for people needing deep time then they will be using JD's only anyway and will
just use isNull() instead.
> I'd vote for keeping QDateTime strictly Gregorian Calendar / Julian day and
> using convenience wrapper classes around. Since the JD is the payload
> anyway, having the extra functions for ALL calendars is unnecessary.
There's a few problems with using a wrapper.
1) We use wrappers in KDE and it's awkward and non-obvious and not integrated.
I'm forever cleaning up after devs who forget to use them. It also opens the
way for confusion and inconsistent usage between the QDate and the wrapper in
the same code, we should be kind to our devs and save them from themselves.
2) In Windows and Mac calendars are completely built-in to their date classes,
no extra wrappers required. A dev doesn't even know a different calendar gets
applied, it just magically happens in the background. They get the calendar
system that is set in the users locale or the users specifically chosen
calendar without any extra thought or effort or code. I want the same simple
seamless experience for our devs and users, I don't want people to have to
think about it or make a conscious choice to localise correctly, I just want
it to happen automatically, otherwise experience shows it won't happen at all.
3) It exposes the QCalendarSystem class as public when I want to keep it a
private implementation detail for now.
On the other hand, there's lots of code out there the assumes QDate is always
Gregorian and does things like QDate(2000,1,1) or myDate.setDate(2000,1,1) and
assume it uses the Gregorian. There's probably also things like PIM code that
assume the maths does Gregorian. Doing it 100% the built-in way like my
initial proposal would seriously break that sort of code which we can't do.
We need to figure out a compromise.
> Note I'd like to propose QDate use the proleptic Gregorian calendar instead
> of the Julian calendar for dates before October 1580. The date of the
> adoption of the Gregorian calendar varies from region to region, so it's
> best to have a separate Julian calendar class for when that is necessary.
No argument here, that was my plan too. It's far more standard. I'm only
using the current code to ensure my refactoring doesn't break anything in the
unit tests until I'm ready to do the full calendar support.
> QDate, QTime and QDateTime's toString and fromString functions should use
> the C locale exclusively. If you want localised date/time formatting, you
> use QLocale or the calendaring classes you're about to add.
So in most cases devs will have to remember to do:
QLocale::system().toString(myDate);
rather than using the current and more obvious:
myDate.toString();
>From KDE experience I can tell you that devs will get this wrong a lot.
Having multiple toString() methods in different places doing different things
will just cause confusion and mistakes: do it in one place and do it
consistently, either only in QDate or only in QLocale.
This also seriously breaks source and behaviour compatability as we currently
don't have toString(QDate) methods in QLocale, so every date formatting
operation would need to be changed when porting to Qt5. My current proposal
will break compatability where devs are not using the default argument, but
that is simply a change of namespace and/or enum value for the argument.
> Right now, since these classes produce system locale formatting and can
> read both English and system, there are some occasions when the parsing
> can go wrong (imagine a locale with a day of the week abbreviated as "sat"
> but that isn't Saturday).
Then shouldn't that be solved by only parsing using the system locale by
default? If someone wants to parse English dates then they should explicitly
do that using a custom locale or by setting a flag on the parser. It's a
behavioural change, but how many people will be relying on that?
> I'd split the formatter and parser, which is a huge section of the code.
> Also note the parser is used by the Q{Date,Time,DateTime}Edit classes too.
OK, will do.
> I'd keep in QDateTime only the simplest parser and formatter (ISO
> formatting, for example) if possible, so one can turn off the parser and
> formatter code in the Qt feature system.
Hmmm. OK, I'll try that. I'll also try make calendars able to be switched
off for those that don't want it.
So, let's restate the problem constraints here:
1) Qt doesn't localise dates to the system calendar or system formats
correctly. We need to make this seamless and easy otherwise it won't be used.
2) There's a lot of existing code that assumes QDate uses Gregorian and we
can't silently break that behaviour.
3) We need to remain as source compatible as possible.
So for backwards-compatability a wrapper is best, but for easy localization
integrated is best. I'm not sure we can find a perfect solution in there,
something has to give.
If we could ignore 2) then my solution would be the way, but I don't think we
can choose that way, at least not 100%. Perhaps for the QDate(2000,1,1) and
setDate(2000,1,1) we could add a required QLocale::CalendarSystem argument and
force devs to change existing code to to be explicit about what they want. A
new setLocalDate() method would use the correct calendar. Not ideal but
better than silently breaking stuff. I'm not sure how year() month() day()
should act in that case.
If we could ignore 3) then I would remove all the ymd/calendar and
parse/format stuff from QDate and make it just a container for storing/passing
the JD, with maybe some convenience methods that take a calendar system as
argument. Any formatting and parsing would be done only in QLocale which
would apply the calendar and formats for that Locale. A QCalendarSystem base
class would provide the ymd functions. A QLocalDate class could provide a
convenience wrapper that combines all three in what people now expect from a
QDate. This would force source changes on everyone, but would ensure there's
no subtle bugs from invisible behaviour changes.
The more I think about it, the more attractive this becomes as the only clean
way to achieve both aims. The question is how acceptable such a big break
would be?
> QDateTime needs timezone support too.
Yep, the planned model is for QDateTime to get the tz support, QTime will just
be a dumb container, like in KDE.
> An interesting question for TZ support: suppose you have a timezone
> definition that points to a file that isn't in /usr/share/zoneinfo. This
> file can change. When is this file reloaded, if at all?
Interesting indeed and I have no answer (yet), I'll see how KDE deals with it.
There's a lot to be done. I want to finish the Date/Calendar stuff in August,
giving me all of September to work on Time Zones. And I need to check on the
general QLocale stuff as well before feature freeze, it's probably more vital
for 5.0. The calendar and time zone stuff could wait for 5.1 if we set things
up right, but it would be cleaner in 5.0.
John.
_______________________________________________
Qt5-feedback mailing list
[email protected]
http://lists.qt.nokia.com/mailman/listinfo/qt5-feedback