Value type aliases.
Closed this issue · 19 comments
I think dropping the requirement that the input and return values of parse_fn share the same type would lead to a more efficient/flexible CLI framework, but it would increase the code size.
I can show it's benefit here:
const std = @import("std");
const cova = @import("cova");
const os = @import("std").os;
const mem = @import("std").mem;
const ascii = @import("std").ascii;
const builtin = @import("builtin");
pub usingnamespace @import("cova"); // Forward namespaces from the original module
const blue = "\x1b[34m";
const yell = "\x1b[93m";
const zero = "\x1b[0m";
/// Cova configuration type identity
pub const CommandT = cova.Command.Custom(.{
.subcmds_help_fmt = "{s}:\t" ++ blue ++ "{s}" ++ zero,
.val_config = .{
.vals_help_fmt = "{s} ({s}):\t" ++ blue ++ "{s}" ++ zero,
.set_behavior = .Last,
.arg_delims = ",;",
},
.opt_config = .{
.help_fn = struct {
fn help(self: anytype, writer: anytype) !void {
try self.usage(writer);
try writer.print(
"\n{?s}{?s}{s}",
.{ CommandT.indent_fmt, CommandT.indent_fmt, self.description },
);
}
}.help,
.usage_fmt = "{c}{?c}{s}{?s} " ++ yell ++ "\"{s}({s})\"" ++ zero,
.allow_abbreviated_long_opts = false,
.allow_opt_val_no_space = true,
.opt_val_seps = "=:",
.short_prefix = null,
.long_prefix = "-",
},
.indent_fmt = " ",
});
///
pub const setup_cmd: CommandT = .{
.name = "vpxl",
.description = "a VP9 encoder by Matt R Bonnette",
.opts = &.{
pathOption("mkv", "input_path", ""),
pathOption("y4m", "input_path", ""),
pathOption("yuv", "input_path", ""),
pathOption("webm", "output_path", ""),
pathOption("ivf", "output_path", ""),
boolOption("resume", "don't be dummy and disable this, this is necessary for thine happiness <3"),
},
};
pub fn boolOption(comptime name: []const u8, comptime description: []const u8) CommandT.OptionT {
return .{
.name = name,
.long_name = name,
.description = description,
.val = CommandT.ValueT.ofType(bool, .{ .name = "", .parse_fn = struct {
pub fn parseBool(arg: []const u8) !bool {
const T = [_][]const u8{ "1", "true", "t", "yes", "y" };
const F = [_][]const u8{ "0", "false", "f", "no", "n" };
for (T) |str| if (ascii.eqlIgnoreCase(str, arg)) return true;
for (F) |str| if (ascii.eqlIgnoreCase(str, arg)) return false;
return error.InvalidBooleanValue;
}
}.parseBool }),
};
}
pub fn pathOption(comptime name: []const u8, comptime val: []const u8, comptime description: []const u8) CommandT.OptionT {
return .{
.name = name,
.long_name = name,
.description = description,
.val = CommandT.ValueT.ofType([]const u8, .{ .name = val ++ " ", .parse_fn = struct {
pub fn parsePath(arg: []const u8) ![]const u8 {
os.access(arg, os.F_OK) catch |err| {
// Windows doesn't make stdin/out/err available via system path,
// so this will have to be handled outside Cova
if (mem.eql(u8, arg, "-")) return arg;
return err;
};
return arg;
}
}.parsePath }),
};
}where specifically parsePath() cannot perform all parsing actions due to this type limitation. I think adjustment could literally support any kind of value parsing from the commandline to the program directly! (reducing the CLI code bleeding into in the meat of your program)
Let me know what you think (and I promise in no way am I suggesting your project wasn't already wonderful, I'm just excited to see what I could do with this)
I love writing CLI apps and I'd been dying to replace the framework I'd been using for something better, perhaps forking one, so contributing has been fun.
Hey! Once again, you've got an interesting enhancement proposal. I don't take it any way as a discredit to Cova, just that you're looking to meet your own project requirements. On the contrary, I'm always on the lookout for feasible new ideas, so I appreciate it!
To the proposal itself, I'd like to clear up a few notions that might've been missed about Values and how they work first, then see if there's an area we can make this work.
The parse_fn doesn't require the input and output to be the same. It requires a string ([]const u8) as the input and the child Type of the Value as the output. This is because the parse_fn is called in place of Value.Typed.parse() as the first step of Value parsing to actually coerce end user argument tokens (which are the raw strings from the Argument Iterator) to the Value's child Type.
Conversely, the valid_fn takes the Value's child Type as an input and outputs a Boolean. This function is run after parse() as means of checking if the parsed data value of the child Type is actually valid per project requirements.
In your example, parsePath() should work just fine. What error are you seeing with that? Also, if I am understanding correctly, this specific use case might be better served as a valid_fn. It looks like you're checking if a string is an accessible filepath, which would fall more under validation since a string data value type doesn't actually need to be parsed.
There's also a chance I'm missing something while reading through it. If so, please help me understand better.
To me it sounds like the most usefully written parse_fn's should cover the needs of valid_fn, erroring out if the parsing fails or if the successfully parsed value is invalid. This seems better to me than separate functions.
That's fair. Thankfully, there's nothing stopping anyone from using parse_fn that way if they'd like to. If that function errors out, the parsing stops right there. So you can do both the parsing and validation logic together in a single function without issue.
More to your issue though, which part isn't working? To me, it looks like your example works as intended, but I'm sure I'm missing something.
I made that parsePath work that way because for some reason I thought input type needed to equal output type, so what I sent does work, but what I was trying to demonstrate is that I could do it better if the types could differ. Since they can, I'll update you when I've adjusted things.
Gotcha. I knew I was missing something. Please do let me know when you've tried again.
Separately, I've updated the mechanics for adding your own types as Value.Typed types. Looking at the way you're creating Boolean Options with boolOption made me realize I should make that an easier experience. Your method works perfectly well, so there's no need to change anything. Just figured I'd let you know since you inspired the change through this issue.
The details are in these two commits (which I added the wrong issue reference to):
You can see an example of overwriting bool and adding u1024 here
My only concern is that when I make the option's value type an OS specific file address (which is a number, not a path), I still want the option's help menu to say that they're supposed to provide a path and not the actual file address. The reason I have to do this at all is because Windows doesn't expose its equivalent to stdin/out/err as file accessible via a file path, such as is available in Unix systems; the only way to do this is to rely on that address I talked about before.
Just to make sure I'm tracking, you're saying you want the Option parsed differently depending on the OS? As in, Windows gets parsed into a Int while (presumably) Linux gets parsed to a String?
If so, you could probably use some compile time code to change that specific Option altogether depending on the targeted OS. Or, you could use the same technique after the fact during analysis, but it sounds like you're trying to do that during parsing. If this is a path you're interested in, I could try and create an example.
I still want the option's help menu to say that they're supposed to provide a path and not the actual file address.
Looking at it more carefully, I think this is the part you're addressing more here. With how things are currently set up, I would use a different valid_fn depending on the OS, and keep the Option's child Type to []const u8 (or whichever filepath abstraction you're using). That way, the user will always see the same type in the Usage/Help message, but you can ensure the value given actually meets your requirements. The downside here being that you'll have to do a cast during analysis for at least one of the OSs, but I think that would be an issue either way because Values can only hold one child Type.
Alternatively, you can make a custom usage_fn for that Option that will display the desired type.
No, I do not want it to handle anything differently based on OS. To clarify, what I want is for Cova to ask the user for a string (a path), and I want it to be able to parse that path from within my parse_fn and store the result for me not as a string, but as the host OS's address to that file. It's very important that stdin/out/err is handled correctly, and because Windows doesn't make its equivalent available via a path, I can only go by file descriptor (look I remembered the term).
Current function:
pub fn parsePath(arg: []const u8, _: mem.Allocator) ![]const u8 {
os.access(arg, os.F_OK) catch |err| {
// Windows doesn't make stdin/out/err available via system path,
// so this will have to be handled outside Cova
if (mem.eql(u8, arg, "-")) return arg;
return err;
};
return arg;
}I assumed you meant file descriptor or that file address was a related term I just hadn't heard yet, haha.
Ok, so using file descriptor regardless of OS. Tracking that part now. If I'm not mistaken, file descriptors are just UInts (or maybe Ints?) that are pointer casted to file locations. That means you're likely going to set up an Option with a U/Int child Type. The remaining issue seems like the Type hint from the Usage message. Since you can't change the actual Type hint with usage_fmt your next choice would be the Option.Config.usage_fn, but that affects all Options not just the file descriptor ones.
In that case, would it help to have an Option.Custom.usage_fn field so you can overwrite the message for only those Options?
I don't think that's a good solution, I think the simpler the solution the better.
There's gotta be a way to specify a different value from the outside than is stored on the inside.
And I'm only asking for this because I think a framework like this should be able to define every aspect of your CLI without having additional code related to it bleeding into your project. If cova wasn't so close to that already, I wouldn't be asking for this, is what I'm saying. Apologies for confusions.
No need to apologize, I enjoy the challenge!
An even simpler solution could be a type alias. Think Value.Typed.type_alias which would be used instead of Value.Typed.ChildT for Usage/Help messages. I've been toying with the idea of adding aliases in other areas, so it might make sense here as well.
Much simpler indeed, and ironically is a better description of what I was originally suggesting at the beginning of this issue!
How does Typed differ from the previously used system? How do I assign unique functions to each option (only because I need to have access to the default value if it exists)?
How does Typed differ from the previously used system?
Value.Typed is the original Value Type I came up with. Value.Generic is also the original abstraction I created for all Value.Typed variations. In order to allow library users to add their own variations of Value.Typed that could still be handled generically, I had to move all of the functions from the original Value.Generic into another wrapper Type which ended up being Value.Custom. That change is due to how reification works in Zig. Namely, reified (programmatically created) Types can't have declarations to include functions. The benefit here was that I was able to remodel Values to more closely resemble Commands and Options in terms of how their Types are set up and used.
How do I assign unique functions to each option (only because I need to have access to the default value if it exists)?
I can try and create an example later, but this comes down to either me adding usage_fn and help_fn directly to Option.Custom, or you handling Type variations based on Type Names in the usage_fn or help_fn provided to Option.Config. The former is probably preferred, but the latter is definitely possible.
I'm actually going to attempt the latter now that I've thought this through a little more; I'll let you know how it goes.
Sounds good! When I have more free time, I'll also look to make an example and possibly implement a few changes to callbacks to make things both more clear and flexible.