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

Reply via email to