-
Notifications
You must be signed in to change notification settings - Fork 39
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 context parameter to *Table types #101
Conversation
This allows to get rid of all the `Tag` nonsense and to further simplify the `Nullifiable` nonsense. Arguably we should have done it like this in the first place, as it brings a lot of nonsense that was hiding inside `Tag` out into the open. It was nice to not have an extra redundant-seeming type parameter (because the `a` already has a `Context`), but it isn't actually redundant. The proposed solution to the `Projection` problem will definitely need this extra `context` parameter and there are no tricks with `Tag` we can do to get around that.
@@ -3,7 +3,7 @@ | |||
{-# language LambdaCase #-} | |||
{-# language MultiParamTypeClasses #-} | |||
{-# language StandaloneKindSignatures #-} | |||
{-# language TypeFamilies #-} | |||
{-# language TypeFamilyDependencies #-} |
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.
I think you either forgot to use this - line 41 has no dependency
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.
It was great we got so far without this, but as we've both found now - there's a ton of value to be derived from having this in the type. So while it's a bit of a shame, I'm on board with this.
I dunno if you wanna confirm this actually unblocks the Projection stuff first, or maybe you've already checked that. Just thinking of avoiding an incomplete solution, but of course we could always revert if needed |
In this case, the extra `Context` parameter is actually 100% redundant as things stand, but it opens up the possibility of a `Prelude.Functor` instance for for `ListTable` further down the line, and at least remains "consistent" with the other `*Table` types.
e23d6e3
to
51fe694
Compare
This allows to get rid of all the
Tag
nonsense and to further simplify theNullifiable
nonsense.Arguably we should have done it like this in the first place, as it brings a lot of nonsense that was hiding inside
Tag
out into the open.It was nice to not have an extra redundant-seeming type parameter (because the
a
already has aContext
), but it isn't actually redundant. The proposed solution to theProjection
problem will need this extracontext
parameter and there are no tricks withTag
we can do to get around that.