On 10/16/2012 03:24 AM, Friedrich W. H. Kossebau wrote:
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?
I was referring here to Kovariable/KoVariableManager like used e.g. in
calligra/plugins/variables/PageVariable.cpp
if (property == KoInlineObject::PageCount) {
KoOdfNumberDefinition defaultDefinition; // FIXME Should fetch
from pagestyle
QString newValue = value.toInt() >= 0 ?
m_numberFormat.formattedNumber(value.toInt(), &defaultDefinition) :
QString();
setValue(newValue);
}
See also calligra/libs/kotext/KoVariableManager.h
/**
* Set or create a variable to the new value.
* @param name the name of the variable.
* @param value the new value.
* @param type The type of the value. This is only set for
user-defined variables.
* For non user defined variables the value is empty. If set then
it should be one
* of the following values;
* \li string
* \li boolean
* \li currency
* \li date
* \li float
* \li percentage
* \li time
* \li void
* \li formula
*/
void setValue(const QString &name, const QString &value, const
QString &type = QString());
That means for all non UserVariables (like date/time/etc) setValue is
called with the final string to display. The KoVariable itself has no
information how that strting got actually produced (formatings,
datastyles, etc).
That means anyone who tries to use KovariableManager->value("VarName")
gets a string and that's it. The original information (like format, etc)
is more or less lost. In rea;ity its not since the variables themself
remember those original things, save it back, allow editing e.g. the format.
The problem raises if, like in the case of a UserVariable, something
else then the variable plugin itself tries to do something more with the
variable's value then only displaying it. In the case of the
user-variable it allows to reference another variable (like a date/time
variable), take the value there and reformat it with another format for
display. Kovariable/KoVariableManager does not allow that.
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>)
calligra/plugins/variables/DateVariable.cpp
void DateVariable::update()
{
...
switch (m_displayType) {
case Time:
if (m_definition.isEmpty()) {
setValue(target.time().toString(Qt::LocalDate));
}
else {
setValue(target.time().toString(m_definition));
}
break;
case Date:
if (m_definition.isEmpty()) {
setValue(target.date().toString(Qt::LocalDate));
}
else {
setValue(target.toString(m_definition));
}
break;
}
}
setValue() is called with the final product (means the string to
display) after applying formatings and stuff. The whole original
information like m_definition and the target QDateTime is never passed
out of the DateVariable and hence noone except the DateVariable can use
the original information.
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>).
Its actually a multilevel step.
In the DatePlugin we do:
1) DateVariable+DateFormat => DateString
In the UserVariable we do:
2) DateString+OtherDateFormat=>NewDateString
Note that in 1) we only pass the resulting DateString to the
KoVariable::setValue and KovariableManager. In 2) the UserVariable
fetches that DateString and tries to make it into NewDateString what
fails cause what the UserVariable would need to do is:
3) DateVariable+OtherDateFormat=>NewDateString
where DateVariable is the input we get at 1) but that is including the
DateFormat never ever made available to the outside world like for the
UserVariable.
A quick a dirty way would be to just
QDateTime dt = parse(DateString);
QString newDateString = format(dt, otherDateFormat);
but that is error-prune and will be in some cases not possible. Better
would be to just make it possible that the UserVariable can access the
original DateVariable QDateTime from the DatePlugin and use it's own
OtherDateFormat to produce the proper NewDateString.
The fundamental problem I see here is that KoVariable/KovariableManager
was never designed to pass more in/on/getOut then a QString which
represents the final string for display for the variable(s). In the case
of UserVariables (and I am sure there are lot more cases) that is just
not enough. What would be needed is the original date, the unformatted
raw-value and/or in the case of the DatePlugin the QDateTime.
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).
The code? You mean those;
DateVariable::loadOdf
...
// hopefully this simple detection works in all cases
const bool isDateTime = (value.indexOf(QLatin1Char('T')) != -1);
?
Or the UserVariable itself? The UserVariable itself should in the ideal
case not need to deal with read/write formats since the DatePlugin does
that already and also produces the proper QDateTime. But then there is
certainly the use-case that a UserVariable may need to read/write a ODF
cached value.....
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.
Makes lot of sense. Especially extending the unittests. iirc the
OdfNumberFormat parsing had quit already quit a few unittests but seems
none of them tests exactly that problem. Well, iirc cause the problem is
more down/up the stack at the place where we push/pull variables in...
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
_______________________________________________
calligra-devel mailing list
calligra-devel@kde.org
https://mail.kde.org/mailman/listinfo/calligra-devel
