idanarye / rust-typed-builder

Compile-time type-checked builder derive
https://crates.io/crates/typed-builder
Apache License 2.0
904 stars 52 forks source link

Change `AttrArg` workflow - match on name first, type second #121

Closed idanarye closed 11 months ago

idanarye commented 11 months ago

@ModProg - I've taken your AttrArg idea and modified it so that the name will be matched first. This means that if a setting is used with the wrong style, instead of getting an error that it does not exist we'd get an error that it is of the wrong type. Currently this error does not specify what the correct type is, but that can be added in the future.

idanarye commented 11 months ago

I'm also considering adding helper functions like iter_if_sub for the other style, but I'm not 100% sure yet if iter_if_sub is the correct abstraction...

ModProg commented 11 months ago

I think I'd add:

impl AttrArg{
   try_get_value(self) -> Result<Expr, /*Error that says a key value was expected*/>;
   try_get_subcommand<T: Parse>(self) -> Result<impl Iterator<T>, /*Error that says a subcommand was expected*/>;
   try_get_flag(self) -> Result<bool, /*Error that says a flag was expected*/>;
}

as AttrArg knows the ident, it can also set the correct span easily.

idanarye commented 11 months ago

My iter_if_sub is similar to your try_get_subattr, but now that I think about it,

for arg in expr.iter_if_sub()? {

is not that much simpler over

for arg in expr.subattr()?.args()? {

So maybe I should just have flag(), keyvalue(), and subattr() methods? The naming convention comes from how Result has ok() and err() methods. The semantics are a bit different (they return Option while I return Result, but the idea is similar.

I also want to have _or_nullify() version of all these methods, which will accept a &mut Option<T> and handle AttrArg::Not.

idanarye commented 11 months ago

BTW - I'm thinking that nested would be a better name than sub. I'll keep it sub for now since it's internal API, but I might rename it after #116 gets in.

idanarye commented 11 months ago

Actually, _or_nullify is a problem because it must still return a value (which does not exist) or an error. Maybe or_not(), which would return and Option?

ModProg commented 11 months ago

Actually, _or_nullify is a problem because it must still return a value (which does not exist) or an error. Maybe or_not(), which would return and Option?

that makes more sense, I find apis taking &mut to be a bit clunky