Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add namespace support to Writer #41

Merged
merged 5 commits into from
Nov 2, 2024
Merged

Conversation

ianprime0509
Copy link
Owner

TODO: I need to take another look over this before merging it to make sure I didn't miss anything (and possibly add more tests aside from just the doctests added here).


Closes #40

In addition to the new namespace-aware functions, this commit brings several other enhancements:

  • The element name has been removed as an argument of elementEnd. The writer now keeps track of the current open element automatically.
  • Doctests have been added for all public writer functions.
  • Doc comments have been added for all public writer functions.

Closes #40

In addition to the new namespace-aware functions, this commit brings several other enhancements:

- The element name has been removed as an argument of `elementEnd`. The writer now keeps track of the current open element automatically.
- Doctests have been added for all public writer functions.
- Doc comments have been added for all public writer functions.
@MFAshby
Copy link
Contributor

MFAshby commented Oct 31, 2024

Thanks! I'm trying it out.

I saw some odd behaviour in my app with elementEnd() where the namespace appears to be missing, notice the closing tag </:href>, which should be </D:href>:

<?xml version="1.0" encoding="UTF-8"?>
<D:multistatus xmlns:D="DAV:" xmlns:c="urn:ietf:params:xml:ns:carddav">
  <D:response>
    <D:href>/</:href>
    <D:propstat>
      <D:prop xmlns:z="DAV:">
        <z:current-user-principal><href>/principal/0192c59f-4894-7ecf-bb00-c2348efa927d/</href></:current-user-principal>
...

I wrote a quick test to try to narrow it down:

test "namespaceBug" {
    var raw = std.ArrayList(u8).init(std.testing.allocator);
    defer raw.deinit();
    const out = streamingOutput(raw.writer());
    var writer = out.writer(std.testing.allocator, .{ .indent = "  " });
    defer writer.deinit();

    try writer.bindNs("d", "foospace");
    try writer.elementStartNs("foospace", "root");
    try writer.elementStartNs("foospace", "child");
    try writer.text("Hello, Bug");
    try writer.elementEnd();
    try writer.elementEnd();

    try expectEqualStrings(
        \\<d:root xmlns:d="foospace">
        \\  <d:child>
        \\    Hello, Bug
        \\  </d:child>
        \\</d:root>
    , raw.items);
}

unfortunately the test crashes with a segfault

martin@martin-pinebook ~/d/zig-xml (bug) [1]> zig build test
test
└─ run test failure
Segmentation fault at address 0xffff84c90001
/home/martin/.zigvm/zig-linux-aarch64-0.14.0-dev.1550+4fba7336a/lib/compiler_rt/memcpy.zig:19:21: 0x117e648 in memcpy (compiler_rt)
            d[0] = s[0];
                    ^
/home/martin/.zigvm/zig-linux-aarch64-0.14.0-dev.1550+4fba7336a/lib/std/array_list.zig:926:42: 0x1075ee3 in appendSliceAssumeCapacity (test)
            @memcpy(self.items[old_len..][0..items.len], items);
                                         ^
/home/martin/dev/zig-xml/src/Writer.zig:792:45: 0x111d82b in addPrefixedString (test)
    writer.strings.appendSliceAssumeCapacity(prefix);
                                            ^
???:?:?: 0xffffc0320127 in ??? (???)
Unwind information for `???:0xffffc0320127` was not available, trace may be incomplete


error: while executing test 'Writer.test.namespaceBug', the following command terminated with signal 6 (expected exited with code 0):
/home/martin/dev/zig-xml/.zig-cache/o/92af64094809fc156d3b34b4e7253c6e/test --seed=0x5a53cded --cache-dir=/home/martin/dev/zig-xml/.zig-cache --listen=-
Build Summary: 1/3 steps succeeded; 1 failed; 56/56 tests passed
test transitive failure
└─ run test failure
error: the following build command failed with exit code 1:
/home/martin/dev/zig-xml/.zig-cache/o/15f27f5a52566fe6a27a9eef16e4586b/build /home/martin/.zigvm/zig-linux-aarch64-0.14.0-dev.1550+4fba7336a/zig /home/martin/.zigvm/zig-linux

@ianprime0509
Copy link
Owner Author

Thanks! That bug is an unfortunate consequence of my trying to be a bit too clever with memory management 😄 I pushed up a fix in e00a4a8, and added your example as a test case.

@ianprime0509 ianprime0509 marked this pull request as ready for review November 2, 2024 02:33
@ianprime0509 ianprime0509 enabled auto-merge (squash) November 2, 2024 02:33
@ianprime0509 ianprime0509 disabled auto-merge November 2, 2024 02:33
@ianprime0509 ianprime0509 disabled auto-merge November 2, 2024 02:34
@ianprime0509 ianprime0509 merged commit 8fda155 into main Nov 2, 2024
4 checks passed
@ianprime0509 ianprime0509 deleted the writer-enhancements branch November 2, 2024 02:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support XML Namespaces in Writer
2 participants