-
Notifications
You must be signed in to change notification settings - Fork 20
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 named field construction #16
Conversation
Ah, need to fix the docs... |
The `named` library seems to make a lot of this look much prettier, so I'm very excited about this change!
src/Data/Generic/HKD/Named.hs
Outdated
instance Append (S1 m h) t (S1 m h :*: t) | ||
instance Append x y z => Append (w :*: x) y (w :*: z) | ||
|
||
class Rearrange (i :: Type -> Type) (o :: Type -> Type) | i -> o |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this a class if it has no methods? Shouldn't it be a type family?
type family Rearrange (i :: Type -> Type) = (o :: Type -> Type) where
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/Data/Generic/HKD/Named.hs
Outdated
import GHC.Generics | ||
import Named ((:!), NamedF (..)) | ||
|
||
class Append (xs :: Type -> Type) (ys :: Type -> Type) (zs :: Type -> Type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this a class if it has no methods? Shouldn't it be a type family?
type family Append (xs :: Type -> Type) (ys :: Type -> Type) = (zs :: Type -> Type) where
...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point; I think I'd set off with the intention of making this much more interestingly injective, but got lazy a bit too quick 😅
src/Data/Generic/HKD/Named.hs
Outdated
-- | ||
-- >>> :{ | ||
-- data User | ||
-- = User { name :: String, age :: Int, likesDogs :: Bool } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the example use fields of the same type but with different names to follow the motivation above?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, probably clearer!
-- ... standing for ...("name" :! f [Char]) | ||
-- ... -> ("age" :! f Int) -> ("likesDogs" :! f Bool) -> HKD User f... | ||
-- ... | ||
class Record (structure :: Type) (f :: Type -> Type) (k :: Type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The third parameter here can be replaced with an associated type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The errors are never as useful, though :/
I'd be interested in exploiting some stuckness eventually to have an error if one forgets the type application, but it's probably out-of-scope for this change, so I'll leave it as-is :)
src/Data/Generic/HKD/Named.hs
Outdated
grecord fill = \(Arg inner) -> fill (M1 (K1 inner)) | ||
|
||
instance (GRecord right f structure k, rec ~ Rec0 x) | ||
=> GRecord (S1 ('MetaSel ('Just name) i d c) rec :*: right) f structure (name :! x -> k) where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no other instance matching on :*:
, so better type errors / type inference is possible if the instance is defined as
left ~ ... => GRecord (left :*: right) f structure ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite right; I'd intended for this behaviour to be introduced straight to Build.hs
for any named-field record, but (having discussed with @kcsongor) the generic-lens
upcasting doesn't work with unnamed fields yet, so it got split out. Good spot - I hadn't gone back and tidied up carefully enough 😇
In general, this looks great, thanks for taking the time to make |
@@ -72,7 +75,7 @@ data User | |||
deriving (Generic, Show) | |||
|
|||
user :: User | |||
user = User "Tom" 25 True | |||
user = User "Tom" 26 True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙂
README.md
Outdated
@@ -184,13 +187,30 @@ eg7 = eg6 [1] [] ["Tom", "Tim"] | |||
-- Triple [1] [] ["Tom","Tim"] | |||
``` | |||
|
|||
Should we need to work with records, we can exploit the label trickery of the | |||
[`named`](http://hackage.haskell.org/package/named) package. The `record` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[`named`](http://hackage.haskell.org/package/named) package. The `record` | |
[`named`](https://hackage.haskell.org/package/named) package. The `record` |
README.md
Outdated
eg8 = record @User | ||
|
||
eg9 :: HKD User Maybe | ||
eg9 = eg8 ! #name (Just "Tom") ! #likesDogs (Just True) ! #age (Just 26) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps break this into several lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks so much @int-index for taking a look, some really great spots :) I'll commit, push, bump versions, and close up!
src/Data/Generic/HKD/Named.hs
Outdated
import GHC.Generics | ||
import Named ((:!), NamedF (..)) | ||
|
||
class Append (xs :: Type -> Type) (ys :: Type -> Type) (zs :: Type -> Type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point; I think I'd set off with the intention of making this much more interestingly injective, but got lazy a bit too quick 😅
src/Data/Generic/HKD/Named.hs
Outdated
instance Append (S1 m h) t (S1 m h :*: t) | ||
instance Append x y z => Append (w :*: x) y (w :*: z) | ||
|
||
class Rearrange (i :: Type -> Type) (o :: Type -> Type) | i -> o |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
src/Data/Generic/HKD/Named.hs
Outdated
-- | ||
-- >>> :{ | ||
-- data User | ||
-- = User { name :: String, age :: Int, likesDogs :: Bool } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, probably clearer!
-- ... standing for ...("name" :! f [Char]) | ||
-- ... -> ("age" :! f Int) -> ("likesDogs" :! f Bool) -> HKD User f... | ||
-- ... | ||
class Record (structure :: Type) (f :: Type -> Type) (k :: Type) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The errors are never as useful, though :/
I'd be interested in exploiting some stuckness eventually to have an error if one forgets the type application, but it's probably out-of-scope for this change, so I'll leave it as-is :)
src/Data/Generic/HKD/Named.hs
Outdated
grecord fill = \(Arg inner) -> fill (M1 (K1 inner)) | ||
|
||
instance (GRecord right f structure k, rec ~ Rec0 x) | ||
=> GRecord (S1 ('MetaSel ('Just name) i d c) rec :*: right) f structure (name :! x -> k) where |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Quite right; I'd intended for this behaviour to be introduced straight to Build.hs
for any named-field record, but (having discussed with @kcsongor) the generic-lens
upcasting doesn't work with unnamed fields yet, so it got split out. Good spot - I hadn't gone back and tidied up carefully enough 😇
Thanks @neongreen and @int-index!
@i-am-tom ping |
Thanks! Can you also ping me when a new version is up on Hackage? |
Ping :) Thanks so much for your incredible patience - it has been a hectic few weeks, and your nudges are appreciated! |
The
named
library seems to make a lot of this look much prettier, so I'm very excited about this change!Hoping @neongreen can give this change a quick look before I commit it to master :)