-
Notifications
You must be signed in to change notification settings - Fork 571
Add proxies #2846
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 proxies #2846
Conversation
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 we can use kindOf here instead, possibly.
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.
TypedValue True here?
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.
This doesn't matter too much, but I use this boolean to indicate "this should be checked", and then "this was checked", so it'd be good to be consistent is all.
|
Thanks! A few comments:
|
|
bc46882 to
19e856e
Compare
LiamGoodacre
left a comment
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.
Some GitHub projects I found with existing usages of proxy as an identifier:
- https://github.com/OutWatch/purescript-outwatch
- https://github.com/TylorS/purescript-cycle
- https://github.com/TylorS/purescript-stream
- https://github.com/Xandaros/purescript-opencomputers
- https://github.com/dariooddenino/cycle-ps
- https://github.com/garyb/purescript-express
- https://github.com/justinwoo/purescript-xstream
- https://github.com/polytypic/purescript-fix
- https://github.com/rintcius/purescript-mathbox
- https://github.com/slamdata/slamdata
examples/passing/KindedType.purs
Outdated
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.
If we've switched from Proxy to proxy then we don't need this change?
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.
Indeed. :) Will change later, working on something else atm.
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.
Can this go under reservedTypeNames?
paf31
left a comment
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'll merge this before the next major release if there are no objections between now and then.
|
A couple more thoughts:
|
That was one question I had, whether I could write something like this: prj
∷ ∀ sym f a r1 r2
. RowCons sym (proxy f) r1 r2
⇒ IsSymbol sym
⇒ proxy sym
→ VariantF r2 a
→ Maybe (f a)
prj p = on p Just (const Nothing)Note the use of |
I think so, because it's not like |
|
What was the reason for not using the symbol at the type level too? |
|
Sorry, which symbol again? :) |
|
So it's |
|
Bump for the |
|
I'm also a bit confused about that, I think it would be nice to use the same syntax for the type and the value if possible. |
|
I'd be fine with |
|
I'm fineish with I'll change it due to popular demand. We can always change it back. Fun fact: with this change, this PR will no longer be a breaking change. |
|
Travis says |
I think we should include this in 0.11.5 then, once the tests pass of course. |
e5109b5 to
1952202
Compare
|
Isn't it still a breaking change because @ can't be used as an operator? FWIW I think it was always dubious due to @ in patterns. |
|
Yes, actually, that's true. This currently works, for example: data Test a b = Test a b
infix 4 type Test as @
x :: forall a. a @ a -> a
x (Test a _) = a |
|
Now I guess the question is, would we rather reserve |
|
Using data Test a b = Test a b
infix 4 type Test as @
infix 4 Test as @
x :: forall a. a @ a -> a
x (a @ _) = a -- Type error, Test a0 a0 /~ a0, the compiler sees this as an @ binding, not `Test a _` |
|
Actually, given that, I think I do prefer |
|
Alright then, |
|
I guess the above parse does mean you can't use a proxy literal in a pattern, but then you don't really need to since they're singly-inhabited. |
|
@garyb the list of uses of |
1952202 to
219fec6
Compare
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.
"@"?
219fec6 to
64d1829
Compare
64d1829 to
ed07eb4
Compare
|
🎆 Thanks! |
This reverts commit 74bc4a9.
* Revert "pretty print proxy types when rendering docs (#3144)" This reverts commit 14227c7. * Revert "Print proxy type as an operator (#3124)" This reverts commit cfd1db3. * Revert "Fix proxies: synonyms, inference, traversals, instances (#3095)" This reverts commit fe0aa0d. * Revert "Add proxies (#2846)" This reverts commit 74bc4a9. * Remove one last occurrence of the ProxyType constructor * Update tests to not use new proxies
* Revert "pretty print proxy types when rendering docs (purescript#3144)" This reverts commit 14227c7. * Revert "Print proxy type as an operator (purescript#3124)" This reverts commit cfd1db3. * Revert "Fix proxies: synonyms, inference, traversals, instances (purescript#3095)" This reverts commit fe0aa0d. * Revert "Add proxies (purescript#2846)" This reverts commit 74bc4a9. * Remove one last occurrence of the ProxyType constructor * Update tests to not use new proxies
Fixes #2839.
This change is