Skip to content
This repository has been archived by the owner on Jul 16, 2019. It is now read-only.

Fixes for Windows support #58

Merged
merged 2 commits into from
Dec 28, 2017
Merged

Conversation

janicduplessis
Copy link
Contributor

Fixes a few things needed for windows support. Flow is pretty strict about what it expect to work, ideally this would also be fixed in Flow but for now we can workaround those things here.

  • For some reason vscode-uri normalizes drive letter to lowercase here https://github.com/Microsoft/vscode-uri/blob/master/lib/index.ts#L131 and this causes issues because Flow uses case sensitive paths even on Windows where it is not. As a workaround I implemented a super simple file uri parsing function that works for our use case and handles windows paths without lowercasing drive letter. Ideally this could be fixed upstream in vscode-uri but there might be a reason why they do this and will be a breaking change so doing this is a lot easier and less disruptive.

  • The flow binary under node_modules/.bin is actually flow.cmd and now flow.exe

  • The project root returned by the language server spec has a trailing slash and flow trips on it on Windows. path.resolve gets rid of it.

Tested that the ide-flowtype extension using this patched version of flow-language-server works on windows and mac os (don't have linux setup but it should be different than mac os).

@sguergachi
Copy link

Is this going to be merged any time soon? It's been a month since this pull request was filed...

@hansonw
Copy link
Contributor

hansonw commented Dec 28, 2017

Thanks for this - looks great! I'll just make a few fixes on top and this will be good to go. Note that Flow will very soon have native support for the LSP so this can hopefully go away :) cc @ljw1004

- work with both flow.exe and flow.cmd
- only avoid vscode-uri on Windows
- check for existence of trailing slash
@hansonw hansonw merged commit 57d0735 into facebookarchive:master Dec 28, 2017
@wbinnssmith
Copy link
Contributor

I just wanted to drop by and say thank you so much for this @janicduplessis!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants