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

Ported to Win32 #15

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Ported to Win32 #15

wants to merge 9 commits into from

Conversation

mvoidex
Copy link

@mvoidex mvoidex commented Mar 16, 2013

Port is used instead of unix socket name, but it works.

@mvoidex
Copy link
Author

mvoidex commented Mar 16, 2013

This pull request is imported for me, I'm using it in SublimeHaskell plugin for Sublime Text 2/3.

@mvoidex
Copy link
Author

mvoidex commented Mar 28, 2013

@bitc ?

@bitc
Copy link
Owner

bitc commented May 16, 2013

Hi, first of all, thanks for submitting this patch, and I apologize for taking a while to respond.

First of all, Windows support is definitely a great feature to have and I support the effort. I'm just a bit worried about your implementation, mostly regarding using regular port-number based sockets.

I intentionally used unix socket files, placed in the current directory, since it makes it clear that local filepaths are important, and it makes things automatically work with running multiple hdevtools servers at the same time for different projects.

Another thing that slightly bothers me about the patch is the heavy use of preprocessor guards, I generally feel more comfortable trying to minimize their usage (although they are fine when necessary -- hdevtools uses preprocessor guards in several places to support multiple GHC versions)

So for now I'm just going to leave this pull request as is, without merging it. I'm not quite sure what the best way to move forward is regarding windows support. I would be happy to hear some input from the community: if there is a consensus that your approach is the best fit for the windows platform then I would be willing to listen.

In the mean time, this windows branch of yours is still currently up to date with regards to hdevtools master, so those who need windows support now are welcome to use your branch.

Thanks!

@mvoidex
Copy link
Author

mvoidex commented May 16, 2013

I don't like this implementation too and I agree with your decision. Thanks for response.

@LeDominik
Copy link

Having some win32 API experience from the past the closest you get to a Unix socket on win32 is a named pipe... I have however not yet checked if there's a suitable Haskell-API available on hackage...

@Zane-XY
Copy link

Zane-XY commented Apr 25, 2014

I built the Windows fork on my local, and it doesn't work:

hdevtools: connect: failed (Connection refused (WSAECONNREFUSED))

Does the vim-hdevtools still work for Windows? How can I start it from command line?

@mvoidex
Copy link
Author

mvoidex commented Apr 25, 2014

@Zane-XY, have you started hdevtools server?

@Zane-XY
Copy link

Zane-XY commented Apr 25, 2014

@mvoidex Thanks for the quick reply, I use it with the vim-hdevtools plugin, I think it will start the server when I open a Haskell file, should I start the server manually? How can I start the server?

@mvoidex
Copy link
Author

mvoidex commented Apr 25, 2014

@Zane-XY sorry, but I don't know anything about vim-hdevtools

@Zane-XY
Copy link

Zane-XY commented Apr 25, 2014

@mvoidex when you integrate with sublime, how do you start the server?

@mvoidex
Copy link
Author

mvoidex commented Apr 25, 2014

@Zane-XY Sublime starts it with hdevtools --start-server

@ulidtko
Copy link

ulidtko commented Nov 24, 2014

Hi all. Consider an idea.

Separate branch

@bitc would you like to merge this in as a separate branch? A windows-port along with master would be great, until the integration becomes good enough code-wise.

In my view, it's a concern that the windows port is a tad bit hard to find among the 40 present forks, and it'd be great to see an appropriate branch right in this repo.

Well... I'll make another PR just for this.

@ulidtko
Copy link

ulidtko commented Nov 24, 2014

Well, I couldn't figure out how to offer a brand new branch via PR.

Anyway. My agreement goes to the points that:

  • preprocessor conditionals are definitely overused here;
  • it'd be excellent to use win32 named pipes in this case — but, it seems, nobody bothered to write bindings for that. Perhaps it's not so hard to use FFI directly.

Although still, the windows code of @mvoidex is functional and will definitely be useful to some. If not a branch, than at least a link from readme is needed.

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