-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add a std.mem.Allocator argument to the type definition of every custom function parameter? #30
Comments
I like this idea, but we'll need to game plan it's implementation. The biggest sticking point is determining how the allocator would be provided when the function is actually called. This is due to the If you can come up with a small example of how you'd use this, that would make it easier to implement I think. |
Well, what I was thinking was that because a) libraries shouldn't supply allocators and b) an allocator is already required to be passed to the library, you could just c) leverage the allocator the library already has been permitted to use and just make it something passed to the parse_fn, usage_fn, and help_fn from within cova, whether or not the end user actually needs to use it in their parse_fn, usage_fn, and help_fn's; if they don't need it, it's just a pointer that gets passed around and doesn't slow anything down, and if they do, they can use it just as you, the developer of the library would. I want to be able to build strings at runtime from the runtime arguments as part of the parsing of values, and potentially in the other functions, but I've been taking everything one step at a time for the most part, as you've undoubtedly observed. I am new to Zig and using allocators, and I didn't want to break the very intelligent convention of allowing the end user to decide how to allocate objects, so that's why I thought that for any function the end user can supply to the library, an allocator should be made available. |
Using the provided allocator is the best way I can think of too. I'm fully on board with that. The only additional change will be adding an I'm also fairly new to memory allocation (as well as Zig) and agree that Zig's allocator paradigm makes a lot of sense. I think implementing this proposal by using the Allocator provided for initialization also falls right in line with how many items in the standard library work (like I'll push forward with this idea and let you know when I have it implemented! |
I haven't read much of the stdlib yet but _alloc as an internal variable your end users can generally access is kinda weird, so I wonder if there's a better way; specifically for Except for within these functions and via the object that the user initially passed, cova's allocator shouldn't really be accessible by the user. |
So, I'm generally inclined to agree, however Zig does not, and will not, have private fields. As such, there isn't a way to reasonably hide the allocator that's not overbearing. Idiomatic Zig dictates that fields should be openly available to get/set and that their usage should be defined by good naming and documentation. As such, I use the You can read more about this from Andrew and other contributors in issue #9909. With that in mind "adjusting the code within Cova" has to involve either sharing a pointer to the allocator amongst the subordinate Argument Types or meticulously passing the allocator to dozens of function calls throughout the library that shouldn't require an allocator by default. In this case, the former allows for better separation of concerns in my opinion. Ultimately, this system does work, but I do understand your concern. Coming from more of an OOP background, it took me a while to adjust to this and I still find it a bit jarring at times. |
I suppose it's fine that it's so available I guess. The only thing I can think of is to make a global variable that's only accessible within the library and assignable via a public function, but that's a bad solution to what is probably a non-issue. Let me know how you choose to resolve this! |
- Added `_alloc` field and `init()` method to Options and Values so the `mem.Allocator` can be passed in from the parent Command during Initialization. - Added `mem.Allocator` Callback Function Parameter requirement so that all callbacks can use the Argument Type's allocator if needed. - Updated Docs to reflect these changes. - Closes #30
- Allowed Custom `Value.Typed` Types in `Value.Config.custom_types` to make custom Value Types more seamless and configurable. - Fixed misssing `mem.Allocator` parameter in `Command.Config.usage_fn`. - #30
- Allowed arbitrary Types in `Value.Config.custom_types` to overwrite existing Types in `Value.Generic`. - Added `custom_parse_fns` field into `Value.Config` to allow for customizable Parsing Functions for added or overwritten Types. - #30
Looks to me like this is working correctly! |
It doesn't have to be used, but it should be available if necessary.
The text was updated successfully, but these errors were encountered: