[PATCH 0/1] MR2330: xmllite/writer: implement prefix autogeneration
Looks good to me but would love some opinion(s) Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2330
From: David Kahurani <k.kahurani(a)gmail.com> Signed-off-by: David Kahurani <k.kahurani(a)gmail.com> --- dlls/xmllite/tests/writer.c | 12 ++++++------ dlls/xmllite/writer.c | 32 ++++++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/dlls/xmllite/tests/writer.c b/dlls/xmllite/tests/writer.c index 572b3a9fd67..8b610a68875 100644 --- a/dlls/xmllite/tests/writer.c +++ b/dlls/xmllite/tests/writer.c @@ -1628,11 +1628,11 @@ static void test_WriteAttributeString(void) { L"xml", L"a", L"", L"value", "<e xml:a=\"value\" />", "<e xml:a=\"value\"" }, /* Autogenerated prefix names. */ - { NULL, L"a", L"defuri", NULL, "<e p1:a=\"\" xmlns:p1=\"defuri\" />", "<e p1:a=\"\"", S_OK, 1, 1, 1 }, - { NULL, L"a", L"defuri", L"b", "<e p1:a=\"b\" xmlns:p1=\"defuri\" />", "<e p1:a=\"b\"", S_OK, 1, 1, 1 }, - { L"", L"a", L"defuri", NULL, "<e p1:a=\"\" xmlns:p1=\"defuri\" />", "<e p1:a=\"\"", S_OK, 1, 1, 1 }, - { NULL, L"a", L"defuri", L"", "<e p1:a=\"\" xmlns:p1=\"defuri\" />", "<e p1:a=\"\"", S_OK, 1, 1, 1 }, - { L"", L"a", L"defuri", L"b", "<e p1:a=\"b\" xmlns:p1=\"defuri\" />", "<e p1:a=\"b\"", S_OK, 1, 1, 1 }, + { NULL, L"a", L"defuri", NULL, "<e p1:a=\"\" xmlns:p1=\"defuri\" />", "<e p1:a=\"\"" }, + { NULL, L"a", L"defuri", L"b", "<e p1:a=\"b\" xmlns:p1=\"defuri\" />", "<e p1:a=\"b\"" }, + { L"", L"a", L"defuri", NULL, "<e p1:a=\"\" xmlns:p1=\"defuri\" />", "<e p1:a=\"\"" }, + { NULL, L"a", L"defuri", L"", "<e p1:a=\"\" xmlns:p1=\"defuri\" />", "<e p1:a=\"\"" }, + { L"", L"a", L"defuri", L"b", "<e p1:a=\"b\" xmlns:p1=\"defuri\" />", "<e p1:a=\"b\"" }, /* Failing cases. */ { NULL, NULL, L"http://www.w3.org/2000/xmlns/", L"defuri", "<e />", "<e", E_INVALIDARG }, @@ -1647,7 +1647,7 @@ static void test_WriteAttributeString(void) { L"xml", NULL, NULL, L"value", "<e />", "<e", E_INVALIDARG }, { L"xmlns", L"a", L"defuri", NULL, "<e />", "<e", WR_E_XMLNSPREFIXDECLARATION }, { L"xmlns", L"a", L"b", L"uri", "<e />", "<e", WR_E_XMLNSPREFIXDECLARATION }, - { NULL, L"xmlns", L"uri", NULL, "<e />", "<e", WR_E_XMLNSPREFIXDECLARATION, 0, 0, 1 }, + { NULL, L"xmlns", L"uri", NULL, "<e />", "<e", WR_E_XMLNSPREFIXDECLARATION }, { L"xmlns", NULL, L"uri", NULL, "<e />", "<e", WR_E_XMLNSPREFIXDECLARATION, 0, 0, 1 }, { L"pre:fix", L"local", L"uri", L"b", "<e />", "<e", WC_E_NAMECHARACTER }, { L"pre:fix", NULL, L"uri", L"b", "<e />", "<e", E_INVALIDARG }, diff --git a/dlls/xmllite/writer.c b/dlls/xmllite/writer.c index 128d666e82c..0ad3176564e 100644 --- a/dlls/xmllite/writer.c +++ b/dlls/xmllite/writer.c @@ -1003,8 +1003,36 @@ static HRESULT WINAPI xmlwriter_WriteAttributeString(IXmlWriter *iface, LPCWSTR { if (is_empty_string(prefix) && !is_empty_string(uri)) { - FIXME("Prefix autogeneration is not implemented.\n"); - return E_NOTIMPL; + struct element *element; + WCHAR buf[16]; + int count = 0; + + if (is_xmlns_local) + return WR_E_XMLNSPREFIXDECLARATION; + + if (writer->indent) + { + count = writer->indent_level; + } + else + { + LIST_FOR_EACH_ENTRY(element, &writer->elements, struct element, entry) + { + count += 1; + } + } + + count = swprintf(buf, ARRAY_SIZE(buf), L"p%d", count); + + if ((ns = writer_find_ns(writer, buf, uri))) + write_output_attribute(writer, ns->prefix, ns->prefix_len, local, local_len, value); + else + { + ns = writer_push_ns(writer, buf, count, uri); + write_output_attribute(writer, buf, count, local, local_len, value); + } + + return S_OK; } if (!is_empty_string(uri)) ns = writer_push_ns(writer, prefix, prefix_len, uri); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2330
I'd expect this should consider already defined namespaces. For example, if you had defined namespace with "p1" prefix manually, autogenerating one in the same element should not reuse the name. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2330#note_26160
Also generated names should not depend on current element depth, or indentation. That seems wrong. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2330#note_26162
On Thu Mar 9 11:54:07 2023 +0000, Nikolay Sivov wrote:
Also generated names should not depend on current element depth, or indentation. That seems wrong. I'm curious about this because HTML(which I understand is a completely different markup language) seems to use this concept with the header tag. To nest a header under another header, you increment the suffix.
This doesn't seem outright wrong or far-fetched to me. Also, I thought my testing was biased so I tested again with the same observation. Based on this concept, the problem with you have described above will not occur unless someone manually places a prefix without considering how nested the attribute is. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2330#note_26512
On Thu Mar 9 11:54:07 2023 +0000, David Kahurani wrote:
I'm curious about this because HTML(which I understand is a completely different markup language) seems to use this concept with the header tag. To nest a header under another header, you increment the suffix. This doesn't seem outright wrong or far-fetched to me. Also, I thought my testing was biased so I tested again with the same observation. Based on this concept, the problem with you have described above will not occur unless someone manually places a prefix without considering how nested the attribute is. We'll need some tests for various cases, if we don't have them covered already.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/2330#note_26556
On Thu Mar 9 19:41:24 2023 +0000, Nikolay Sivov wrote:
We'll need some tests for various cases, if we don't have them covered already. Hmmm.... I think I have noticed some issues with this approach.
Closing this for now. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2330#note_31577
This merge request was closed by David Kahurani. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2330
participants (3)
-
David Kahurani -
David Kahurani (＠kahurani)
-
Nikolay Sivov (＠nsivov)