search

SciNim / nim-plotly

plotly wrapper for nim-lang
https://scinim.github.io/nim-plotly/
MIT License
179 stars 15 forks source link

fix: WSL check failing on mac #65

Closed danwhitford closed 3 years ago

danwhitford commented 3 years ago

Problem I had a couple of issues with the check for running under WSL

  1. The /proc/version file does not exist on all Posixes, notably not on MacOS. This was causing the program to crash with an OSError
  2. Windows release was lower case in my WSL version (running Debian under WSL2)

Fix Using uname() from posix_tools should be more portable for other Posixes and will not crash if the file doesn't exist. Also added check for windows and Windows!

I hope it's OK to create a PR without an issue first. Happy to create one if you prefer.

Vindaar commented 3 years ago

Oh, that's a great point. The only thing I'm wondering about is whether import posix_utils should be behind a when defined(posix) (I suppose the module is just empty on non posix systems, but I haven't checked).

Otherwise, thanks a lot for this!

edit: I should add: I fear the CI might still be broken on devel, cause choosenim was/is stuck on an old Nim devel version. I kept wanting to go and replace choosenim, but haven't done so so far unfortunately.

danwhitford commented 3 years ago

Oh, that's a great point. The only thing I'm wondering about is whether import posix_utils should be behind a when defined(posix) (I suppose the module is just empty on non posix systems, but I haven't checked).

That makes sense, I'm pretty new to nim and hadn't seen that pattern before but looks sensible. I have updated.

Vindaar commented 3 years ago

Just checked travis btw and it seems like choosenim is finally pulling a devel version with the infinite recursion bug fixed. So CI passes for this PR now.