feat(core): add fixedTo.{atLeast,exactly,upTo}#340
Conversation
|
Hey @DylanRJohnston you are awesome, I like this very much! Indeed, adding all these parametrics makes sense, thanks for taking the time to make this PR. Some notes:
I won't have internet today and maybe also not tomorrow. (My dad is having a surgery today and I'll be at the hospital, and tomorrow I get baptized at church -- so maybe I can get back to you on this until sunday). Thanks for all your effort on this :) Btw, if you can experiment and switch our default functor to be |
# Conflicts: # nix/lib/can-take.nix # nix/lib/default.nix # nix/lib/parametric.nix
|
Hey @DylanRJohnston, I'm trying to make some progress on this, starting with merging |
# Conflicts: # nix/lib/take.nix
…an.johnston/fixedTo
| satisfied = valid && builtins.all (n: params ? ${n}) required; | ||
| exactly = valid && required == builtins.attrNames params; | ||
| upTo = valid && intersect != { }; | ||
| upTo = satisfied && intersect != { }; |
There was a problem hiding this comment.
upTo shouldn't return true for functions that require more than you're able to provide, the previous implementation returned true for canTake { host = true } ({ host, user }: {})
There was a problem hiding this comment.
Good catch, I have some usage of upTo in den core, lets see if tests pass.
| deepRecurse = | ||
| functor: ctx: provided: | ||
| provided | ||
| mapIncludes = | ||
| branch: leaf: aspect: | ||
| aspect | ||
| // { | ||
| includes = map ( | ||
| include: | ||
| if include ? includes then | ||
| if include ? __functor then include else parametric.deep functor ctx include | ||
| else | ||
| include | ||
| ) (provided.includes or [ ]); | ||
| include: if include ? includes && !include ? __functor then branch include else leaf include | ||
| ) (aspect.includes or [ ]); | ||
| }; |
There was a problem hiding this comment.
The name change comes from the fact that this function isn't recursive anymore, it just applies two different functions to one level of the include tree, one for branches, and one for leaves.
| parametric.deepOwned = functor: deepRecurse includeOwnedAndStatics functor lib.id; | ||
| parametric.deepParametrics = functor: deepRecurse includeNothing lib.id functor; | ||
|
|
||
| parametric.fixedTo.__functor = _: attrs: parametric.deepOwned (lib.flip parametric.atLeast attrs); | ||
| parametric.fixedTo.exactly = attrs: parametric.deepParametrics (lib.flip take.exactly attrs); | ||
| parametric.fixedTo.atLeast = attrs: parametric.deepParametrics (lib.flip take.atLeast attrs); | ||
| parametric.fixedTo.upTo = attrs: parametric.deepParametrics (lib.flip take.upTo attrs); |
There was a problem hiding this comment.
When implementations unify it's usually a good sign you're on the right track. I found I was able to unify the recursion in my implementation with the recursion in parametric.deep. Interestingly one applies the functor on the node in the include tree before it gets traversed, the other applies the functor at the leaf nodes. Not 100% sure why the difference is required or if one could be massaged into the other.
I found it particularly interesting that attrs / ctx could be factored out right to the top level and is no longer interleaved through the include tree traversal but instead is only partially applied to the functor.
DylanRJohnston
changed the title
|
Awesome @DylanRJohnston everything is green, shall we merge ? |
|
|
I like much more the names you use than mine :D. I will merge, infinite thanks! Hopefully later we can make your wish come true, make upTo the default parametric. |
2 participants
Overview
Adds new features, early draft, wanted to get your opinion on the design before committing to cleaning it up and potentially breaking it into smaller focused PRs. Needs documentation update too.
den.lib.take.upToUses the same predicate as
atLeastto decide whether to invoke a function, but calls the function with only the arguments it supports unlikeatLeastwhich will call the function with extra arguments causing an error unless the function signature uses.... In my opinion this is howatLeastshould function, but I understand that this is a breaking change if anyone is usingargs@{ ctxKey, ... }to capture the full context. So I've introduced this as a seperate parametric type.den.lib.canTake.upToAn alias of
den.lib.canTake.atLeastprovided for API symmetry betweenden.lib.canTakeandden.lib.takeden.lib.recursiveFunctorMuch like
den.lib.functorthis takes anapplyfunction and an aspect and applies it to each include in the aspectsincludeslist. However, this also recurses into the include list and applies the function down the include tree. Much likefixedTodoes but without owned or static aspects.Caveat: This currently doesn't resolve static or parametric includes and then recurse into their includes if they exist, should it? I'm not sure why you would write aspects with this configuration, but it's theoretically possible. I originally didn't implement this because I thought it would be too complicated, but I think you can just call the aspects with ctx and static ctx and then recurse into them to get at them?
den.lib.parametric.fixedTo.{exactly,atLeast,upTo}This family of
fixedTovariants, followfixedTo's ability to recurse into the import tree, but do not evaluate owned or static aspects. This reflects the relationship betweenparametric.__functorandparametric.{exactly,atLeast,upTo}.