Fixing display of user defined variables of type date (and time) , need help

[Friedrich W. H. Kossebau]

Am Freitag, 28. September 2012, 14:21:33 schrieb Sebastian Sauer:
> Aloha Friedrich and * :)
> 
> On 09/27/2012 11:49 PM, Friedrich W. H. Kossebau wrote:
> > Hi,
> > 
> > I looked into why date variables are always only displayed with the
> > default
> > format and found that the error lies in that
> > KoOdfNumberStyles::format(...)
> > expects the value for date (and seems also time) to be a number in string
> > form:
> > --- 8< ---
> > QString format(...)
> > {
> > [...]
> > 
> >           case Date: {
> >           
> >              bool ok;
> >              int v = value.toInt(&ok);
> >              return ok ? formatDate(v, format.formatStr) : value;
> > 
> > [...]
> > }
> > --- 8< ---
> > 
> > Problem now is that DateVariable keeps the value as date string around and
> > not in that single number string version (from what I understood, no yet
> > the complete picture).
> 
> Indeed and it makes sense cause the variableManager()->value() is used
> direct for display

Hm, is it really used directly for display? From how I understood meanwhile 
the existing code the idea of that UserVariable is that is takes the original 
data value from the variable manager (which keeps the data as found in the 
ODF document with <text:user-field-decl>) and creates the actual text string 
to display by applying the format set for the UserVariable (as found in the 
data-style referred to by the "style:data-style-name" attribute <text:user-
field-get>/<text:user-field-input>).

And the current code also seems to not completely support the 1.2 version of 
the spec, where e.g. time and date are not stored as a float-value, but 
instead in the dateTime/date format as specified for xmlschema (the one 
similar to ISO 8601).

So I am currently creating a test document which both has variables declared 
following the 1.2 spec (from what I understand) as well as variables following 
that old OOo(?) way, to test also backward compatibility.
Hopefully I can turn it into some automatic test in the end, for now manual 
testing is at least a start, to assist in fixing/completing the code.

> but that opens the problem that it does not contain
> the information what format was used to produce the date/time what makes
> it rather hard to extract the original value (means translate it back)
> to e.g. apply another format...
> 
> I think for the load/save roundtrip this is not a problem cause we just
> load, display and save it like it was but as soon as someone likes e.g.
> during editing change the display format those formatted string value
> needs to be translated to its QTime/QDate/QDateTime representation to
> proper apply the format.
> 
> > There is also this TODO (from you, Sebastian):
> > --- 8< ---
> > void UserVariable::valueChanged()
> > {
> > 
> >      //TODO apply following also to plugins/variables/DateVariable.cpp:96
> >      //TODO handle formula
> >      QString value = variableManager()->value(m_name);
> >      value = KoOdfNumberStyles::format(value, m_numberstyle);
> > 
> > kDebug() << m_name<<variableManager()->value(m_name) <<
> > m_numberstyle.formatStr << value;
> > 
> >      setValue(value);
> > 
> > }
> 
> iirc that's what the TODO was referring to. In the DateVariable we
> already proper parse the formatted value string to earn a
> QTime/QDate./QDateTime representation that can then be used future.
> 
> It's a special case which would needs to be handled in the UserVariable
> where the original formatted string is reused with a new format *AND*
> for editing where a user likes to change the format. Both is handled in
> the DateVariable but not in the UserVariable.
> 
> > --- 8< ---
> > 
> > Are you working on this any time soon, Sebastian? Or could you point me
> > into the direction how this TODO is planned to be solved, so I can give
> > it a try?
> There seems to be no way currently to make local members a
> variable-instance has accessible to other variables. The KoProperties
> are shared among all instances of a variable (iirc, or not?) and so the
> easy way to just extend the DateVariable to proper write the in loadOdf
> read m_definition into the KoProperties like
> props->setProperty("definition", m_definition); is not possible.
> 
> I think what would be needed is a generic "variableInstanceProperty" (or
> since Kovariable inherits from QObject somebody could just use
> QObject::setProperty) and then in loadOdf and on changing the
> m_definition of a DateVariable someone could
> QObject::setProperty("dateDefinition", m_definition) and in the
> UserVariable extract that info using
> QObject::property("dateDefinition").toString() and then something like
> QDate date = parseDate(variableManager()->value(m_name),
> variableManager()->variable(m_name)->property("dateDefinition")); or so...
> 
> Maybe it (the parsing) could even be done a level up in the KoOdfNum*
> itself? iirc there is some nice parsing code in
> calligra/filters/sheets/xlsx/NumberFormatParser.cpp and there is also
> quit some logic in the DateVariable which looks like something that may
> of more common use (means e.g. also for other variables).
> 
> All that untested and without looking at the code for a long while and
> just now had my first looks after finally figuring out the git checkout
> errors I had for a longer time got solved :) Hope it gives some ideas
> and hope it helps to address that problem. Lot of thanks for looking
> into it! :)

Cheers
Friedrich