Skip to content
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

network-simple for SockAddr #5

Open
jdnavarro opened this issue Mar 4, 2014 · 3 comments
Open

network-simple for SockAddr #5

jdnavarro opened this issue Mar 4, 2014 · 3 comments

Comments

@jdnavarro
Copy link
Contributor

I made network-simple work with resolved SockAddr here: https://github.com/jdnavarro/network-simple-sockaddr

Notice that it also works with IPv6 and Unix Sockets.

Do you think it could be included somehow in network-simple? Perhaps in a different module and make the current `Network.Simple.TCP depend on it and just focus on resolving addresses?

If you are in favor of this idea, I could do all the integration work.

@k0001
Copy link
Owner

k0001 commented Mar 4, 2014

Ah, this is a good idea! Yes, this definitely has a home within network-simple. Thanks for doing this.

I'm not sure if Network.Simple.TCP should just focus on resolving addresses, maybe it should continue to export the functions it currently exports as a convenience (so that users don't need to worry about SockAddr if they don't care), but I agree that it should rely on the SockAddr support when possible.

I haven't looked at your changes in great detail yet, I'll do it later.

@jdnavarro
Copy link
Contributor Author

That's great to hear.

Sorry, I didn't explain myself clearly. I tried to say that if Network.Simple.TCP could depend on Network.Simple.SockAddr, the only code specific to Network.Simple.TCP, while maintaining the same API, would be related to resolving addresses.

I'll be easier to understand once I have some integrated code. I'll let you know then.

jdnavarro added a commit to jdnavarro/network-simple that referenced this issue Mar 10, 2014
jdnavarro added a commit to jdnavarro/network-simple that referenced this issue Mar 10, 2014
@jdnavarro
Copy link
Contributor Author

There is still a lot to polish, but now with those commits referenced above you can get a better idea of the changes I propose.

  • Network.Simple.SockAddr will work with SockAddres only.
  • Network.Simple will work generically with TCP/IPv4, TCP/IPv6 and Unix Socket domains. I don't know if it makes sense to try to include UDP here later, haven't thought about it yet but it's a possibility.
  • Network.Simple.TCP will be like Network.Simple but making sure the sockets are TCP only. The API should stay the same as before.
  • Network.Simple.Unix the same as TCP for Unix sockets.
  • Network.Simple.UDP ...

The only real breaking API change I'd like to propose is to use MonadIO m instead of IO everywhere, including the callback functions. But if you don't want it (the type signatures will become messy and we'd need to use RankNTypes), it's OK, it's not so crucial for me, it'd be just nice for me to have.

I'll wait for your input before completing the changes and submitting a proper pull request.

Please don't feel pressed to accept these changes, if you think these changes are too drastic, there is no problem, I can keep using the network-simple-sockaddr fork but I thought it'd be nice to have it all in a single package.

jdnavarro added a commit to jdnavarro/network-simple that referenced this issue Mar 13, 2014
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

No branches or pull requests

2 participants