facebookarchive / ide-flowtype

Flow support for Atom IDE
Other
178 stars 17 forks source link

Windows support #42

Closed Arcanemagus closed 6 years ago

Arcanemagus commented 6 years ago

I'm filing this to have a single place to track getting support for Windows here, as I don't see an issue specifically about this already filed.

@wbinnssmith Previously mentioned that there are a "few issues" in https://github.com/flowtype/ide-flowtype/issues/17#issuecomment-329042754 and updated the readme to reflect that it wasn't "fully supported" after https://github.com/flowtype/ide-flowtype/issues/10#issuecomment-329873530.

If there is anything you need help testing please let me know!

mikecousins commented 6 years ago

What does a few issues mean? I'm currently seeing the outline view and nothing else. Is that what I should be seeing on Windows or are some of you in a more functional state than me?

Arcanemagus commented 6 years ago

I'm not sure what "a few issues" means, personally it's completely broken as the LSP only responds to the outline view requests, every other part of it returns null.

HyperBrain commented 6 years ago

For me it means that it is not usable at all in the current state 😞 - I only have the outline, but no errors show up.

jcowgar commented 6 years ago

This makes it unusable for my team as well. Is there something we can do to help? I have not done any Atom development in the past, but would really like to give this a go, but we use Flow, and I'm on Windows.

sguergachi commented 6 years ago

There's a pull request here that apparently adds Windows support to the flow-language-server but hasn't been merged for over a month...

hansonw commented 6 years ago

Just published flow-language-server@0.2.4 with @janicduplessis's fixes. 1e93e38a540bcc061243d1bfdcbb97dc73045288 has the version bump so I'd appreciate it if any Windows users here were able to test it out, as I haven't had a chance to do that yet!

Arcanemagus commented 6 years ago

I'll give it a try 😉.

Arcanemagus commented 6 years ago

Looks like it's finally working for me! 🎉

image

jcowgar commented 6 years ago

I'm a bit lost. On my system, I simply install flow-bin from npm. What do I have to do to make use of this new commit?

Arcanemagus commented 6 years ago

@jcowgar The simplest way would be to close all instances of Atom, and then run the following:

apm uninstall ide-flowtype
apm install ide-flowtype

This should make it re-download the dependencies, and since flow-language-server@0.2.4 satisfies the flow-language-server@^0.2.3 requirement of the released version, you should get the latest version installed for you. The https://github.com/flowtype/ide-flowtype/commit/1e93e38a540bcc061243d1bfdcbb97dc73045288 commit simply forces that version as the minimum allowed.

hansonw commented 6 years ago

Awesome! I'll go ahead and publish a new release to bump everyone's versions.

jcowgar commented 6 years ago

@Arcanemagus Thanks, I didn't realize it was so simple. I followed your instructions and I have flow errors! Didn't think I'd be happy to see them!

So, it's working here!

Thanks @hansonw!

hansonw commented 6 years ago

Glad to hear, happy holidays! :) Closing this out for now, please let us know if you find any other specific problems on Windows.

janicduplessis commented 6 years ago

@hansonw I've been using this for a while now, it's not perfect but usable. The main issue left is caused by vscode-uri because it lowercases drive letters and path comparaisons in atom are case sensitive. I opened an issue on vscode but I'm not sure how likely it is to get fixed upstream. It's not an issue for vs-code because they do other kind of path normalization that lowercases paths on Windows.

Arcanemagus commented 6 years ago

@janicduplessis There were several changes made to your https://github.com/flowtype/flow-language-server/pull/58 PR, maybe those remaining issues you had are fixed there?

jcowgar commented 6 years ago

Current File Only switch does not work. It does not show any results regardless of the current file you are editing. I notice what @janicduplessis mentioned. In the "Path" column of the "Diagnostics" pane, it shows a lowercase drive letter, while hovering over the file tab shows an uppercase drive letter. I am not sure if that is the problem, but seems likely w/the error reported above.

I did upgrade to the version you pushed out in Atom.

hansonw commented 6 years ago

Ah... it must be that vscode-uri does the same lowercase normalization while creating file URIs, not just when parsing it. I'll have to verify this but it'll be a simple fix if that's the case.

On Thu, Dec 28, 2017 at 8:34 PM Jeremy Cowgar notifications@github.com wrote:

Current File Only switch does not work. It does not show any results regardless of the current file you are editing. I notice what @janicduplessis https://github.com/janicduplessis mentioned. In the "Path" column of the "Diagnostics" pane, it shows a lowercase drive letter, while hovering over the file tab shows an uppercase drive letter. I am not sure if that is the problem, but seems likely w/the error reported above.

I did upgrade to the version you pushed out in Atom.

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/flowtype/ide-flowtype/issues/42#issuecomment-354396716, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjPYtP4bagXdCpM_PxkGc7ksBIvhQc1ks5tFGvJgaJpZM4QflB8 .

janicduplessis commented 6 years ago

@hansonw Yes it's possible, if I remember correctly the issue was that errors did show up in the diagnostics panel, but not underlined in the code because atom would not recognize that it's in the opened file because of different casing on the driver letter.

Also here's the issue I opened on vscode about that for more context https://github.com/Microsoft/vscode/issues/40057

janicduplessis commented 6 years ago

@hansonw Did a quick investigation and the problem happens here https://github.com/flowtype/flow-language-server/blob/master/src/Diagnostics.js#L78 when we create the URI. We could use https://github.com/flowtype/flow-language-server/blob/master/src/utils/util.js#L40 and patch it for windows but creating a uri object is more complex (It seems like we use only toString() so we could juste change the util method to return a string instead of the full uri object).

Another alternative that might be better is to fork or vendor in vscode-uri and remove the code that lower cases driver letters.

hansonw commented 6 years ago

Thanks again, @janicduplessis! I've pushed a new flow-language-server version and ide-flowtype version so hopefully this should be fixed now.

jcowgar commented 6 years ago

@hansonw @janicduplessis "Current File Only" now works on Windows!

sguergachi commented 6 years ago

I'm still seeing this problem on Windows, when I open a file I see this. Tried re-installing the package and it didn't help. image

Arcanemagus commented 6 years ago

@sguergachi That's a separate issue (the outline was actually the only thing that did work on Windows!), there are a few issues already filed here about this but unfortunately I don't remember the solution off the top of my head.

thirafidide commented 6 years ago

I still can't make it work. I tried to reinstall everything (even the Atom itself) and still got no flow error message, nor autocomplete, hints, etc. The Outline working just fine though

notworking

Did i missing something? Also i don't get any message in the console (from Ctrl+Shift+I), probably the ide didn't fire properly?

Arcanemagus commented 6 years ago

@iamn00b The debug messages shown above are a hidden setting, if you want to enable them you need to type atom.config.set('core.debugLSP', true) in the console.

Does flow work in the CLI for you? (npx flow check)

thirafidide commented 6 years ago

CLI runs fine and shows the error.

Some errors do show up though after i enabled atom.config.set('core.debugLSP', true)

error

hansonw commented 6 years ago

@iamn00b using flow-bin from node_modules is not enabled by default. Go into ide-flowtype's settings and enable "Use flow-bin from node_modules when available" to see if that works for you.

On Fri, Feb 2, 2018 at 10:35 AM, Landon Abney notifications@github.com wrote:

@iamn00b https://github.com/iamn00b The debug messages shown above are a hidden setting, if you want to enable them you need to type atom.config.set('core.debugLSP', true) in the console.

Does flow work in the CLI for you? (npx flow check)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/flowtype/ide-flowtype/issues/42#issuecomment-362667928, or mute the thread https://github.com/notifications/unsubscribe-auth/AAjPYle7lTLpOvKC0Rp9hTZb-tvh73GDks5tQ1V6gaJpZM4QflB8 .

Arcanemagus commented 6 years ago

Looks like that's already enabled, and their node_modules flow-bin is crashing with a code of 12. Since this is something different from basic Windows support I'd file that as a new issue @iamn00b.

thirafidide commented 6 years ago

@Arcanemagus @hansonw yep, it's enabled. Disable it doesn't help though.

I see, i'll file this as a separate issue. thanks

sguergachi commented 6 years ago

Seeing the same issue, (no diagnostics) though I don't see an error with code 12: image

Arcanemagus commented 6 years ago

@sguergachi Your screenshot shows diagnostics being pushed back for two files, so your problem is in the interface somewhere.

sguergachi commented 6 years ago

@Arcanemagus Interface meaning ide-flowtype package or atom-ide package?

Arcanemagus commented 6 years ago

atom-ide-ui's diagnostics, or linter + linter-ui-default, whichever you are using to display those messages.