eclipse / aCute

Eclipse aCute - C# edition in Eclipse IDE
https://projects.eclipse.org/projects/tools.acute
Eclipse Public License 2.0
80 stars 33 forks source link

Dotnet path not seen in 64bits windows #115

Closed JustJavaConsultancy closed 6 years ago

JustJavaConsultancy commented 6 years ago

It looks like DotnetVersionUtil only checks for 32bits Windows and doesn't do an appropriate check for 64bits Windows. It keeps failing after entering the correct path in Preference window.

mickaelistria commented 6 years ago

I don't think any of the current contributors do use Windows. So if you could add technical details about how checks should be done on Windows 64bits or even submit a PR, that'd be very helpful. Code is at https://github.com/eclipse/aCute/blob/master/org.eclipse.acute/src/org/eclipse/acute/AcutePreferenceInitializer.java#L32 mostly.

JustJavaConsultancy commented 6 years ago

Thanks a lot michaelistria, instead of using Platform.getOS what about using apache commons lang which has a class SystemUtils.java and we can then use :

SystemUtils.IS_OS_LINUX SystemUtils.IS_OS_WINDOWS

Thanks

bkalbfus commented 6 years ago

I don't think the issue is in the getBestDotnetPathGuess() method. Entering the correct path gives the error: dotnet --version failed to return a version

image

It is finding the executable. Not finding a path gives a different error. I am using Eclipse Oxygen on 64-bit Windows 7. dotnet --version returns the version (2.1.3 for me) when I run it from the command line.

Thank you, Brian

JustJavaConsultancy commented 6 years ago

Hi mickaelistria, I just submitted a PR as requested to address the windows OS issue. Thanks

bkalbfus commented 6 years ago

Please review my PR. It fixes the issue on my end.

mickaelistria commented 6 years ago

instead of using Platform.getOS what about using apache commons lang

In Eclipse Platform, using Platform.getOS() is very reliable and integrated well with many corner cases. I don't see any value in changing.

mickaelistria commented 6 years ago

A new build including #117 from @bkalbfus was produced. Please try it at http://download.eclipse.org/acute/snapshots/ and report whether it works for you.

mickaelistria commented 6 years ago

Hearing no objection, I assume #117 fixed this issue.