Closed ExpHP closed 6 years ago
Looks great far so good to me; my one question ATM is with the moving of doc-tests for HNil and HCons; do these run fine?
It looks like they do on Travis, just double checking:
HCons
's len: https://travis-ci.org/lloydmeta/frunk/jobs/357234016#L784HNil
's len: https://travis-ci.org/lloydmeta/frunk/jobs/357234016#L815They do run, although they are currently duplicates of each other.
Fun fact about doc comments: They get transformed into #[doc = ... ]
annotations before the token stream is given to macros, so we can even match them explicitly in the macro. I plan to use this in the future to give different docstrings to some of the HNil functions.
Edit: well okay, doing that for only 5~6 methods would be overkill, but it is something that could be done if things ever got out of hand.
Fun fact about doc comments: They get transformed into
#[doc = ... ]
annotations before the token stream is given to macros, so we can even match them explicitly in the macro. I plan to use this in the future to give different docstrings to some of the HNil functions.
Interesting. TIL!
@Centril I get the impression that you generally prefer having a few general interfaces over lots of little feature-specific interfaces, so I want to make sure you are okay with the reduced roles I am giving each trait.
(P.S. not all of these are done in the PR yet. These are just my plans)
The following traits will be unaffected by my PR because I am not giving them inherent methods:
hlist::LiftFrom
, hlist::LiftInto
HMappable
, HFoldLeftable
, HFoldRightable
CoproductFoldable
The following traits receive inherent methods but still describe themselves as being intended for general types. (I.e. not necessarily specific to HLists or Coproducts)
hlist::IntoReverse
The following traits describe themselves as a bound for using a method of HList (or Coproduct) in generic code:
hlist::Selector
hlist::Plucker
hlist::Sculptor
CoprodInjector
CoprodUninjector
CoproductSelector
CoproductTaker
CoproductSubsetter
CoproductEmbedder
The following traits now describe themselves as purely implementation detail of a method on HList or Coproduct. There is no good reason to ever need the trait in generic code.
hlist::IntoTuple2
(this trait formerly described itself as being for general types, but I feel that its semantics of producing a nested tuple are far too specific to hlist, and its treatment of the last two items destroys any utility it has in generic code)It sounds good in general 👍
A small thought...
The following traits describe themselves as a bound for using a method of HList (or Coproduct) in generic code:
To increase the usefulness of frunk, we could also let some of these traits operate on other product types such as built in tuples. I agree that we should mainly consider / describe the traits as bounds for hlists, etc., but we could also consider them more generally? Tho the Generic
trait allows you to go back and forth between tuples and hlists.
IMHO, I'd prefer to have general functionality like that on a separate trait, one which is designed to be imported. That trait can then be impl
'd on HCons
where Self: hlist::Selector
(as an example), and use different bounds on other types where appropriate. (or maybe it can be impl'd on everything where Self: Generic
).
This way, the design of Selector
can remain optimized for the implementation of HList::get
, without potentially needing to change to accommodate similar methods on other types.
Testing out T: Generic<Repr = some_hlist>
sounds interesting; For now, I'm comfortable with your general plan. If we can add more impls on tuples that do not hurt type inference later, that could be nice but not strictly necessary.
I was really afraid of how these signatures would look in documentation, so I'd just like to take a moment to thank rustdoc for being goddamn awesome!
pub fn subset<Targets, Indices>(self) // v--- yikes!
-> Result<Targets, <Self as CoproductSubsetter<Targets, Indices>>::Remainder>
where
Self: CoproductSubsetter<Targets, Indices>,
We probably want to run some rustfmt on all the code later ;)
@lloydmeta I think this is ready :slightly_smiling_face:
Also, when should we release this ? Do we want to start planning release milestones? Just an open ended question; we can maybe open an issue to discuss this as needed if the answer is too long :)
My kind of indirect answer to this is #101, though not everything on the list needs to be done before an initial release. (I know, that isn't saying much that hasn't already been said!)
I guess that really, we can choose to shoot for a release at any time. But definitely, the primary documentation (e.g. README.md) needs to be updated to take advantage of the new features before making the release.
Okay, I think this is ready.
Todo:
Original text
This currently just covers HList because I wanted to know if there is any feedback before continuing with Coproduct. The actual addition of the inherent methods is only a small part of this. By far the biggest aspect of such a feature is documentation, because without good documentation, having both the trait methods and the inherent methods will create confusion. * Docs on traits were moved to the new methods where appropriate. * Examples in `hlist.rs` are adjusted to show turbofish * Most trait docs have significantly changed. * `Selector`, `Sculptor`, `Plucker` all direct the reader's attention toward the inherent method, leaving the trait for just generic code. * IntoTuple2 says "you should never need to import this trait" because it is not useful in generic code. * IntoReverse was allowed to keep its headline. (I can see other types implementing it in the future.) It just casually remarks "This functionality is also provided as an inherent method."