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

Added JSON stringify function to Table #17

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

VisenDev
Copy link

Added capability to JSON stringify TOML Tables, this allows them to be parsed into zig structs very easily with the usage of std.json

This makes it much easier to parse TOML data into a zig struct. This approach was inspired by my other toml parser which uses a rust backend. I'd rather have a solution in pure zig

Example

test {
    const toml =
        \\[data]
        \\author = "Robert"
        \\github = "VisenDev"
        \\#heres a comment
        \\
        \\[numbers]
        \\list = [1, 2, 3]
    ;
    const toml_type = struct {
        data: struct {
            author: []const u8,
            github: []const u8,
        },
        numbers: struct {
            list: []const u32,
        },
    };

    var parser = try parseContents(std.testing.allocator, toml);
    defer parser.deinit();

    var table = try parser.parse();
    defer table.deinit();

    var json = try table.stringify();
    defer json.deinit();

    const parsed = try std.json.parseFromSlice(toml_type, std.testing.allocator, json.items, .{});
    defer parsed.deinit();
}

@VisenDev
Copy link
Author

Since this has not been merged yet, I will close it and send a new pull request with more updated bug fixes

@VisenDev VisenDev closed this Nov 13, 2023
@VisenDev VisenDev reopened this Nov 13, 2023
@VisenDev
Copy link
Author

Actually I realized that the new commits seem to have been included automatically into this pull request, so this pull request has the latest updates

src/toml.zig Outdated
@@ -22,6 +22,13 @@ pub const Key = union(enum) {
pub const DynamicArray = std.ArrayList(Value);
pub const TableArray = std.ArrayList(*Table);

pub const TomlStringifyError = error{
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of the redefinition of all of the error values here. I could be wrong but it seems the only possible error with stringify is the OutOfMemory error value, so a simple !std.ArrayList(u8) should suffice.

@aeronavery
Copy link
Owner

I do like the idea. I think the function should be named to toJson or something of that nature because this is a TOML library so in my mind stringify implies it outputting TOML rather than JSON.

@VisenDev
Copy link
Author

I think the function should be named to toJson

Okay, that name is fine

I'm not a fan of the redefinition of all of the error values here. I could be wrong but it seems the only possible error with stringify is the OutOfMemory error value, so a simple !std.ArrayList(u8) should suffice

The reason I did this is zig wasn't able to infer the error set on its own (I get a compile error) - So I have to provide it explicity.

toml.zig:117:60: error: unable to resolve inferred error set
                var table_string = try self.Table.stringify();
                                       ~~~~~~~~~~~~~~~~~~~~^~

However your are correct that the OutOfMemory error is the only neccessary member, I can remove the others. I will make these changes and add a new commit. Thanks

@VisenDev
Copy link
Author

VisenDev commented Nov 15, 2023

I was also planning on adding some sort of helper function which would simply the usage from looking like this

    var parser = try parseContents(std.testing.allocator, toml);
    defer parser.deinit();

    var table = try parser.parse();
    defer table.deinit();

    var json = try table.stringify();
    defer json.deinit();

    const parsed = try std.json.parseFromSlice(toml_type, std.testing.allocator, json.items, .{});
    defer parsed.deinit();
    

To looking something like this

    const parsed = try parseIntoType(toml_type, std.testing.allocator, toml_string, .{});

@VisenDev
Copy link
Author

@aeronavery I made the requested changes

@VisenDev
Copy link
Author

@aeronavery Just wanted to follow up to see what the status of this pr is right now

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.

2 participants