Skip to content

+ Map and Return methods to make it more F#+ friendly #618

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

Merged
merged 5 commits into from
Oct 6, 2022

Conversation

gusty
Copy link
Contributor

@gusty gusty commented Oct 4, 2022

I propose to add this overload to make it F#+ friendly.

F#+ (as many other libs) defines also <!>, <*> and >>=, but in a generic way and actually it doesn't cause any problem with <*> and >>=, but for <!> it expects a Map static member.

This is a small adjustment, backwards compatible and it doesn't cause any problem here.

Eventually if you don't agree to expose a Map static member, I can hide it with [<EditorBrowsable(EditorBrowsableState.Never)>]

EDIT: I added also Return which might be useful too when interoperating with F#+ of course it can also be hidden with the attribute.

gusty added 3 commits October 4, 2022 14:26

Unverified

The signing certificate or its chain could not be verified.
@kurtschelfthout
Copy link
Member

No hard objections, just wondering if some of this won't clash with Gen.map on the Gen module which has CompiledName("Map")? Or maybe F#+ can leverage that?

Also please add a comment to clarify what the intent is of these methods, i.e. to support F#+, as I doubt anyone will remember after some time and they might get removed inadvertently....

@gusty
Copy link
Contributor Author

gusty commented Oct 5, 2022

Hi Kurt, thanks for your feedback.

No hard objections, just wondering if some of this won't clash with Gen.map on the Gen module which has CompiledName("Map")? Or maybe F#+ can leverage that?

Let me give it a try first, in order to make sure the interaction is as smooth as possible.

@gusty
Copy link
Contributor Author

gusty commented Oct 6, 2022

I tried it and it works perfectly.
It's true that before applying this, type inference gets confused and shows an accessibility error mentioning a private Map implementation (possibly from IGen) which is definitely misleading, since the compiled names are not taken into account for trait-calls, but the F# ones.
It seems that the F# compiler should be adjusted in the regard.

I left Return hidden for tooling as probably could be confusing for the user, can do the same with Map if you want, as there's already a .map function.

@gusty gusty changed the title + Map overload to make it F#+ friendly + Map and Return methods to make it more F#+ friendly Oct 6, 2022
@kurtschelfthout
Copy link
Member

Thanks!

I left Return hidden for tooling as probably could be confusing for the user, can do the same with Map if you want, as there's already a .map function.

Yes, please.

@gusty
Copy link
Contributor Author

gusty commented Oct 6, 2022

Done !

@kurtschelfthout kurtschelfthout merged commit 5b6f9a1 into fscheck:master Oct 6, 2022
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.

None yet

2 participants