Hejsil / mecha

A parser combinator library for Zig
MIT License
473 stars 21 forks source link

Mapping string to structure #67

Closed joelreymont closed 3 weeks ago

joelreymont commented 3 weeks ago

Extending the asStr test...

    const parser3 = comptime ascii.range('a', 'z').many(.{ .collect = false }).asStr();
    try expectResult([]const u8, .{ .value = "ad" }, parser3.parse(allocator, "ad"));

    const Defined = struct {
        id: []const u8,
    };

    const parser4 = parser3.map(toStruct(Defined));
    try expectResult(Defined, .{ .value = .{ .id = "foo" } }, parser4.parse(allocator, "foo"));
}

gives me the error below.

What is the correct way of creating a structure where a field is of type string?

❯ zig build test --summary all
test
└─ run test
   └─ zig test Debug native 1 errors
mecha.zig:673:17: error: mecha.test.asStr.Defined and []const u8 does not have same number of fields. Conversion is not possible.
                @compileError(@typeName(T) ++ " and " ++ @typeName(@TypeOf(tuple)) ++ " does not have " ++
                ^~~~~~~~~~~~~
Hejsil commented 3 weeks ago

toStruct only works for array/tuple to struct conversion. You should be able to get a tuple by doing this:

const parser4 = mecha.combine(.{parser3}).map(toStruct(Defined));
joelreymont commented 3 weeks ago

I tried that but I still get the same error with this

    const Defined = struct {
        id: []const u8,
    };

    const parser4 = comptime combine(.{parser3}).map(toStruct(Defined));
    const result = try parser4.parse(allocator, "foo");
    try testing.expectEqualStrings("foo", result.value.id);
Hejsil commented 3 weeks ago

I tried that but I still get the same error with this

    const Defined = struct {
        id: []const u8,
    };

    const parser4 = comptime combine(.{parser3}).map(toStruct(Defined));
    const result = try parser4.parse(allocator, "foo");
    try testing.expectEqualStrings("foo", result.value.id);

Aah right, just had a look and for 1 parser, combine does not return a tuple. It would make sense for toStruct be able to convert T -> struct { field: T } but it currently isn't supported

joelreymont commented 3 weeks ago

Is there a specific reason why combine does not return a tuple for 1 parser?

Should this be considered an enhancement?

Hejsil commented 3 weeks ago

Is there a specific reason why combine does not return a tuple for 1 parser?

Should this be considered an enhancement?

The logic was that you often will do something like this combine(.{ whitespace, token }) where whitespace returns void. The void will then be excluded and only the result of token remains. In that case, it is often not desired that the result of token is wrapped in a tuple.

joelreymont commented 3 weeks ago

Would the same result be achieved by wrapping result of token in a tuple and providing an unwrap combinator that can be used for 1-item tuples if desired?

As it stands, I'm forced to map using a function that takes a string and returns my structure which seems less elegant than an using an unwrap combinator.

Hejsil commented 3 weeks ago

I mentioned earlier this:

It would make sense for toStruct be able to convert T -> struct { field: T } but it currently isn't supported

Wouldn't that work for your usecase?

joelreymont commented 3 weeks ago

It would work. I looked at toStruct, though, tried to figure out how to specify what field is and couldn't figure out a good approach.

On a second thought, I guess the name of the field doesn't matter, just as it doesn't matter when overlaying a tuple over structure fields.

joelreymont commented 3 weeks ago

PR #72 should fix this.