Skip to content

Allow parser to take optional secondary std.mem.Allocator parameter #159

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

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

Conversation

maxbol
Copy link

@maxbol maxbol commented Apr 30, 2025

Not sure if this would be interesting for merging into the main tree, but I wrote a little wrapper to allow parsing functions to take the parser allocator as a secondary parameter. Potential use case: washing strings, reordering lists etc.

Of course the pitfall here is that the main program must remember to deallocate these values at some point.

Parsers are still allowed to take ONLY the value parameter. Not sure how zig-ish this is, but at least it doesn't break backward compat.

Merge only if this speaks to you, and lmk if there is something I clean up/rename/make more idiomatic (not a prof zig developer by any means :))

fn parseLowercaseString(value: []const u8, allocator: std.mem.Allocator) ![]const u8 {
    return std.ascii.allocLowerString(allocator, value);
}

fn parseSomethingElse(value: u32) i32 {
    // Still allowed to only take the value as parameter
    ...
}

...

const params = comptime clap.parseParamsComptime(
    \\--to-lower <raw_str>
);

const parsers = comptime {
    .raw_str = parseLowercaseString,
};

@maxbol maxbol force-pushed the pass-allocator-to-parsers branch from 8be64d4 to d263275 Compare April 30, 2025 17:42
@maxbol
Copy link
Author

maxbol commented Apr 30, 2025

Removed nix crud that was added by mistake

@maxbol maxbol changed the title Allow passing allocator to parsers with withAllocatorParser() Allow parser to take optional secondary std.mem.Allocator parameter Apr 30, 2025
@Hejsil
Copy link
Owner

Hejsil commented Apr 30, 2025

Yea, giving an allocator to the value parsers is pretty useful. Instead of allowing both signature fn ([]const u8) !T and fn (std.mem.Allocator, []const u8) !T, I'd prefer to always just take the latter. If the function doesn't need the allocator argument it can remove it.

@maxbol
Copy link
Author

maxbol commented Apr 30, 2025

Sounds just fine by me but also like it's going to break current consumers with custom parsers. I'll stick to my own branch for now and let you cook on this 🔥

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