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

Add look and feel checks for WSL. fix #26 #27

Closed sadphi closed 3 years ago

sadphi commented 3 years ago

This hopefully fixes the issue on WSL where the incorrect look and feel is selected (GTK+), when it should be the same as on Windows.

bjornregnell commented 3 years ago

Thanks!! I think I'd prefer giving both the options "-q" and "-s" separately to shut up file not found and then also skip the macOs test inside isInProc. Feels more straight forward and simpler to read and understand. Could you fix that @sadphi 

scala> def isInProc(s: String): Boolean =                                                                                                   
     |   val cmd = Seq("grep", "-s", "-q", s, "/proc/version")
     |   val p = sys.process.Process(cmd)
     |   util.Try(p.! == 0).getOrElse(false)
bjornregnell commented 3 years ago

Also we would need volunteers for each and every OS to test Linux/Windows/MacOS/WSL/WSL2 etc. The more the merrier. Reports here are welcome, if you want to try out the fork of @sadphi

sadphi commented 3 years ago

I've updated the code based on your comments. I've tested it on vanilla Windows and found no issues, and it also seems to be working now on WSL as well when using GWSL.

bjornregnell commented 3 years ago

OK nice. But wasn't it InProg("Microsoft") as well a possible alternative to look for to detect WSL? https://github.com/microsoft/WSL/issues/4071#issuecomment-496737093

sadphi commented 3 years ago

Hmm, if I recall correctly /proc/version only showed windows on the machine I tested.

But based on that issue I guess it's just safer to include microsoft as well, because it seems to vary from system to system, even with similar setups.

I can add this tomorrow.

sadphi commented 3 years ago

I added microsoft as a search string in the latest commit, but I also rewrote isInProc() to instead take repeated parameters, so that grep can search for all of them at the same time using regexp.

bjornregnell commented 3 years ago

Cool! Could you also try to elicit some test reports that are given here by one or two people running some specified setup (WSL1|WSL2 & GWSL version ? & Windows 10 | Windows 11 if possible).

bjornregnell commented 3 years ago

I'll merge this now and soon make a release here on github for more people to test it before we make a release on maven central.

sadphi commented 3 years ago

Great! I'll report if I find any issues

bjornregnell commented 3 years ago

It just struck me that we should use java File instead -- it's kind of 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".

bjornregnell commented 3 years ago

New issue here now: https://github.com/lunduniversity/introprog-scalalib/issues/29