On 03/06/2017 09:43 PM, Tamas Zolnai wrote:
Anyway, you must be right with that it's not a good idea to change the
staticType_, so I added an other fix which does not affect staticType_, but
uses a typeHint only for parsing a specific value from XCU file.
Can you review this change please? I hope you're happy with that:
https://gerrit.libreoffice.org/#/c/34933/
see below
On Monday, March 06, 2017 18:40 GMT, "Tamas Zolnai"
<tamas.zol...@collabora.co.uk> wrote:
So it's clear that the generated xcu file from Windows registry is
incomplete\invalid, so it can be a good idea to fix it, but this would take an
amount of time since registry keys used for LO are always strings as I see (see
comments in configmgr/source/winreg.cxx) and I guess it will work on the same
way for the end of the time for compatibility reasons. So it is impossible to
find out the type from the registry key. To find out that we need to parse the
corresponding xcd file here too (as configmgr code does).
The existing mapping from the Windows registry to the configmgr data
model is naive, which inevitably gives rise to issues like tdf#106283.
If you/anybody wants to promote that Windows registry feature, it would
make sense to invest time into a proper mapping between the two data
models. (I can help with that and/or you can look at the comment at the
head of configmgr/source/dconf.cxx for an idea of what to look out
for---though that mapping is even more complicated as it is
bidirectional.) Sure, that may take time (what's the issue with that),
and may unfortunately be complicated by backwards-compatibility
concerns. (An alternative would have been to not allow the existing
Windows registry feature in in its current naive form, but then again
the restricted amount of things that can reasonably be done with it in
its current form may have been sufficient for its initially envisioned
users.)
(I have only superficial knowledge of the Windows registry, but from
looking at
<https://en.wikipedia.org/wiki/Windows_Registry#Keys_and_values> one
backwards-compatible way to encode configmgr value+type might be with a
two-element REG_MULTI_SZ list whose first element encodes the type of
value in the second element. Or, even simpler, from looking at the
comment at the head of configmgr/source/winreg.cxx, in addition to
"Value" and "Final" keys introduce a "Type" key. Wouldn't that be
rather trivial to implement?)
It can be a solution, but as I see you created a bug report (tdf#92755) about avoiding
using these temporary xcu files for Windows registry, so I'm not sure it worth to try to
make these xcu files valid (having type information), if they will be removed later,
right? So I'm not sure why you suggest to fix the code which generates these xcu files.
(your comment on bugzilla: "[...] For such extension props it must generate an
oor:type attribute.[...]") Also fixing this bug by replacing xcu generation with a
better implementation which using configmgr's internal data would also take me very far
from fixing the bug I intended to (tdf#106283).
Whether or not we go via intermediary temporary xcu files, the mapping
from the Windows registry to the configmgr data model must be fixed.
(That is, for such an extension prop, the mapping code needs to provide
a type that it somehow extracts from the Windows registry data model,
whether that type is then stored in an intermediary xcu file or directly
used when instantiation a configmgr::PropertyNode. That's what I meant
with "For such extension props it must generate an oor:type attribute.")
Cleaning up those intermediary xcu files is of course orthogonal to
fixing tdf#106283.
"[...]On the other hand, it is important that the PropertyNode representing
such an extension prop has a staticType_ of TYPE_ANY, so that later layers or
programmatic calls can store values of different type."
So this part is also not clear to me. What do you mean later? When can it
happen that the same property get a different type, which is defined in the
specific xcu/xcd file?
<http://web.archive.org/web/20101103025920/http://util.openoffice.org/common/configuration/oor-document-format.html>:
"[...] dynamically added properties behave as if defined in the schema
as nillable, non-localized property of type any." (And properties of
type any can take on values of different types over time.)
What do you mean programmatic calls can store values of different type? When a
programmatic call would store a different typed value for a typed property? One
specific property always defined with a type even if this property is part of
an extensible group, right? So I can't see why this type would be overriden by
a programmatic call? Or if this is a use case (using properties as something in
which you can store anything) then I guess this also must be true for all
properties, not only a member of an extensible group. What's the difference?
I also tested that case when an extensible group has properties with different
types (xs:boolean-xs:long) and it also works. I thought you might thinking of
that case and later means later when the specific extensible group is extended
with a new property with a different type. In this case my change works. So I
would appreciate if you can point out a use case when this code change is
problematic.
So, for such an extension prop (a "dynamically added property", as the
cited document calls them) having e.g.
<prop oor:name="foo" oor:type="xs:boolean"><value>false</value></prop>
in one layer and
<prop oor:name="foo" oor:type="xs:int"><value>0</value></prop>
in a higher layer would no longer work with your change.
So coming back to <https://gerrit.libreoffice.org/#/c/34933/>, I'm not
too excited about that hack. It's a quick-fix to work around one
specific shortcoming of the naive Windows registry mapping, but in a way
that would require that hack to stay for backwards-compatibility reasons
even when/if a more principled fix for that mapping is done.
_______________________________________________
LibreOffice mailing list
LibreOffice@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice
