Skip to content

Conversation

@JordanMartinez
Copy link
Contributor

Since SProxy is defined in Prelude, we need to port purescript-proxy's Proxy type to here instead now that it can be polykinded.

Note: I assume that IsSymbol should use Proxy now rather than SProxy.

@JordanMartinez
Copy link
Contributor Author

JordanMartinez commented Oct 6, 2020

I'll have to get to this later:

  • Update all kind-specific proxy types to use forall proxy. proxy kindValue


reifySymbol :: forall r. String -> (forall sym. IsSymbol sym => SProxy sym -> r) -> r
reifySymbol s f = coerce f { reflectSymbol: \_ -> s } SProxy where
reifySymbol :: forall r. String -> (forall sym. IsSymbol sym => Proxy sym -> r) -> r
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should use a type variable proxy instead of Proxy, for the same reason as reflectProxy - that is, so that it works with both Proxy and SProxy until we update it again later.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! I've fixed that in latest commits.

@JordanMartinez
Copy link
Contributor Author

Should the docs for Proxy be updated at all? I think right now it only explains how to use it as a proxy for kind Type, not for any kinds.

@hdgarrood
Copy link
Contributor

I don't think that's necessary; it will be clear that Proxy can be used for any kind once someone has come across the idea of polymorphic kinds, and I think the documentation for polykinds would be a better place to introduce things like the fact that you can use Proxy with any kind.

@JordanMartinez
Copy link
Contributor Author

Ok. Then I think this PR is ready to be merged if there are no other changes I need to make.

@hdgarrood hdgarrood merged commit 212e486 into master Oct 10, 2020
@hdgarrood hdgarrood deleted the addProxy branch October 10, 2020 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants