search

lunduniversity / introprog-scalalib

Scala library with simple-to-use utilites for students of introductory programming. http://cs.lth.se/pgk/api
BSD 2-Clause "Simplified" License
60 stars 14 forks source link

Refactor current detection of OS including WSL #29

Closed bjornregnell closed 3 years ago

bjornregnell commented 3 years ago

This PR https://github.com/lunduniversity/introprog-scalalib/pull/27 includes a hopefully feasible way to detect OS and set the right look and feel so that window stuff does not crash on WSL + GWSL, and we should test that in as many setups as possible including Windows 11.

However, the code could be refactored to be more elegant and easier to read and also more effective:

We should use java's File stuff instead -- it's unnecessarily heavy to fire up a whole new OS process just to check what's in a file. I did not question the tip on how to detect WSL. So before we make an ordinary release to Maven central 1.3.1 we should fix this. And the isInProc is an implementation detail that should not be visible outside, and actually a part of isOS so that method should use a private isInProc. If so, the code would feel more "natural".

sadphi commented 3 years ago

I agree, that's a better way to do it. I can take a look at this later today.

bjornregnell commented 3 years ago

Ok good. I just made one refactoring here: https://github.com/lunduniversity/introprog-scalalib/commit/ff3806a61a58ef935b713550b78651a65c17c387 But haven't touched the process -> file-check stuff so feel free to do that. Perhaps there are stuff in introprog.IO that can be used for this....

sadphi commented 3 years ago

I've now created a pull request with a changed isInProc(), see #30