albertosottile / darkdetect

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

Implement `listener` for macOS #25

Closed albertosottile closed 1 year ago

albertosottile commented 2 years ago

A listener function for macOS is currently missing. As stated in the README, calling this function on this platform raises NotImplementedError.

EDIT:

Implementation requirements

The listener function must:

The listener function should, if possible:

Related: #14

albertosottile commented 2 years ago

I invested some time into trying to implement this, but so far I had lo luck.

If anyone is willing to pick up this task, further information on the most severe issues can be found here: https://stackoverflow.com/questions/72982210/swift-and-python-get-a-runloop-that-can-run-in-threading and here: https://developer.apple.com/forums/thread/710135?login=true&page=1&r_s_legacy=true#720551022

I pushed the library implementations that I tried in the macos_listener_attempts branch, but none of them is functional from a secondary thread (a fundamental requirement for this package).

zwimer commented 1 year ago

Is there a reason something akin to this would not work?

def listener(callback):
  old = isDark()
  while True:
    x = isDark()
    if x != old:
      old = x
      callback(x)
    time.sleep(.01)

def listen(callback):
  threading.Thread(target=listener, args=(callback,)).start()

This seemed to work for me, though maybe you'd want theme() instead of isDark?

I realize this might not be an ideal listener, but given the current lack of any, might this be acceptable in the meantime?

zwimer commented 1 year ago

A PR of the above: https://github.com/albertosottile/darkdetect/pull/27

albertosottile commented 1 year ago

@zwimer Thanks for your contribution. I apologize if I did not reply sooner, but I think this approach is not truly viable. This thread will constantly call sleep and the ctypes reading code, which will have a CPU impact on the host machine and an impact on the other threads due to the constant GIL lock/release. I did not test this, but I assume the effect on e.g. laptops battery life would be significant.

Therefore, I would be against integrating this in the darkdetect codebase, as any programmer would expect that the listener function would not behave like this. The general, and correct, expectation would be that the spawned thread does not consume any constant resources and only fires the callback when needed. The approach proposed here does not allow that.

As you mentioned, though, as a last resort measure any developer can implement a similar approach in their own codebase, where they have full control on the secondary effects. But I would rather not add this in the codebase, especially hidden under a listener API that, as mentioned before, carries a different set of expectations..

zwimer commented 1 year ago

@albertosottile I implemented a proper listener here: https://github.com/albertosottile/darkdetect/pull/30

albertosottile commented 1 year ago

Closed in #30, thanks to @zwimer.