albertosottile / darkdetect

Detect OS Dark Mode from Python
Other
178 stars 18 forks source link

Check compatibility with Windows 11 #13

Closed goanpeca closed 2 years ago

goanpeca commented 3 years ago

Hi @albertosottile thanks for working on this :)

I had a question:

Is this ifelse code https://github.com/albertosottile/darkdetect/blob/master/darkdetect/__init__.py#L19 working as expected?

Will it not work for other versions of windows besides 10 ?

thanks!

albertosottile commented 3 years ago

I had to go to the mentioned PR to understand exactly what this issue was about, as I am not yet in the right mindset for Windows 11. Short answer: I have no idea if this package works under Windows 11, nor I have a way to test it, nor I am sure on how to fix a potential failure.

@goanpeca Would you like to open a PR with a fix? Maybe is it as easy as changing == with >=...

Side note: this is not as trivial as it might seem, as e.g. macOS 11.0 still has platform.mac_ver() = '10.16'. Hence, for whatever PR/solution, I would recommend some testing on the target system...

albertosottile commented 2 years ago

I renamed the issue. I would really appreciate if anyone could test if darkdetect works as expected on Windows 11, so we can close this issue or apply the necessary fixes.

GeoffIX commented 2 years ago

I have found a modification to the init.py which allows Windows 11 to be detected correctly. I am running Windows 10 and Windows 11 in Parallels Desktop 17 virtual machines under macOS Big Sur. I first confirmed that both Windows installs correctly responded to the windows registry commands to discover the current display theme. That led me to look at the version check being used.

Windows 11 still returns '10' from platform.release(), so that part is fine. [Edited to clarify: I'm using Python 3.7.9 as the internal scripting language within Poser 12 software, which I am beta testing on both Windows and macOS. So the caveat is that the output of the platform package may see revision in later versions of Python - who knows? I originally thought that changing the platform.release() test to >= '10' or forgoing the build version test if > '10' would be applicable, but Windows 11 still reports release as '10', so presumably someone decided they didn't want to break python scripts, or else older platform packages have no facility for supporting future windows releases. Shrug.]

Instead of deriving winver from sys.getwindowsversion(), which was reporting 9200 on both win10 and win11 systems, I used the second component of platform.version() which gave me 19041 on win10 and 22000 on win11, after splitting the '10.X.XXXXX' platform.version() output

#winver = int(sys.getwindowsversion()[2])
winver = int(platform.version().split('.')[2]) # This gives better build numbers in Windows 11
sayyid5416 commented 2 years ago

I will check it tomorrow

sayyid5416 commented 2 years ago

@albertosottile I checked this issue with windows 11. Check the screenshot for the reason. image

Yeah! Code still works for windows 11.

sayyid5416 commented 2 years ago

As a precaution, I created a pull request regarding this #16

albertosottile commented 2 years ago

Closed by #16