auto-dark-light@gihaume: v2.0.0

[guillaume-mueller]

• Closes [auto-dark-light@gihaume] Custom Time For Automatic Theme Change #6758
• Closes [auto-dark-light@gihaume] Want to propose +/- min delay settings #7247
• Closes Customizable Keyboard shortcuts to change dark/light mode #7923

Note that I used JS minification so the applet loads faster. I understand it looks like compiled binary (that you avoid) but unlike the latter it should be fully reproducible on any system. I added an automatic checksum so anyone can verify the integrity easily.

[guillaume-mueller]

@rcalixte What do you mean by marking the PR as draft?

[rcalixte]

@rcalixte What do you mean by marking the PR as draft?

User-facing code should be readable. The benefits for minification in JavaScript are related to download times not execution times. As the Spices are distributed as compressed files, this isn't a necessary or gainful modification, reproducible or not. Hopefully this makes sense.

[guillaume-mueller]

I wasn't talking about runtime performance but loading time, but anyway it was like 50 ms versus 100 ms, so no problem.

[mtwebster]

Hi,

I'm going to close this, at least for now.

I don't want to let this applet get any more out of hand and bloated than it already is. Looking at the current version there's already a lot of functionality overlap and reinventing the wheel. More effort needs to be made to utilize existing API and patterns common to spices and Cinnamon.

• You don't need a timezone and (manual) time change listener. Set up a periodic timeout to check things. You can retrieve and compare details using normal GLib (GDateTime, GTimeZone ...) functionality. Remove the compiled binary.
• You can check and monitor the screensaver and session presence using native Cinnamon interfaces (see https://github.com/linuxmint/Cinnamon/blob/master/js/misc/gnomeSession.js and https://github.com/linuxmint/Cinnamon/blob/master/js/misc/screenSaver.js)
• No minified code, for reasons above, but also, a lot of Cinnamon contributors and Spice devs got their start messing around with an existing applet or desklet - it's nice to have this stuff accessible for modification. I'm not a fan of typescript here for the same reason, though it doesn't completely limit tinkering.

Keep in mind that Cinnamon is a single-threaded application - spices all run in this same thread, and can cause side effects/bugs during normal operations if they're ill-behaved.

[guillaume-mueller]

I'm going to close this, at least for now.

Why closing? We could continue to work on this PR, getting the discussion and requested changes stay in one place.

Looking at the current version there's already a lot of functionality overlap and reinventing the wheel. More effort needs to be made to utilize existing API and patterns common to spices and Cinnamon.

I'm fully in for utilizing existing API, but I'd need help for pointing what. I tried to do it as much as I could, then finding other ways for what I could not find.

You don't need a timezone and (manual) time change listener. Set up a periodic timeout to check things. You can retrieve and compare details using normal GLib (GDateTime, GTimeZone ...) functionality. Remove the compiled binary.

"a periodic timeout to check things" is exactly the opposite of the philosophy of this applet in order to be not based on any periodic polling at all. It only reacts when anything relevant for it has changed. I think that active polling should be reserved to low/mid level services and not apps and that the latter should subscribe listeners to those centralized lower level pollers.

I used to use QRedShift and could note the terrible impact of naive active polling on fluidity. One day seeing a movie on VLC I noticed that every n seconds, there was a stutter. I was thinking "WTF is wrong with this OS" after finally realizing that it was QRedShift's fault doing an update of all its values. We've been lucky I have not stopped at the WTF and told the world that Linux Mint Cinnamon is natively bloated. This is exactly what you want to avoid and I worked hard on this applet to make it fully reaction based so the user only pays when and for what it is relevant to do so.

Do you understand what the "time change listener" does? It doesn't react each second or minute when the clock naturally updates, but only when the user or the NTP service modifies it. If there's an existing service to subscribe to it, I would use it, but I haven't found any.

Regarding the timezone change listener, I also don't get why it would be wrong.

You can check and monitor the screensaver and session presence using native Cinnamon interfaces (see https://github.com/linuxmint/Cinnamon/blob/master/js/misc/gnomeSession.js and https://github.com/linuxmint/Cinnamon/blob/master/js/misc/screenSaver.js)

I will have a look on this and come back.

Keep in mind that Cinnamon is a single-threaded application - spices all run in this same thread, and can cause side effects/bugs during normal operations if they're ill-behaved.

I don't get the point. I worked so thoughtfully doing a fully reaction based applet that does a very low impact on the performance of Cinnamon, as explained above and more. And I think I've succeed to do so.

I don't want to let this applet get any more out of hand and bloated than it already is.

Now this new version doesn't add anything more connected to Cinnamon than the previous one. The change is mainly the add of MobX to get a reactive paradigm helping the UI state management to be declarative, light, fine-grained, robust and extendable. And it works so well that it has been a key point in my ability to add the features the users asked me to.

I’d appreciate your feedback on these points, since I feel my perspective may currently be misunderstood.

[mtwebster]

I don't know what QRedshift was doing to cause such an impact, but if it was doing that it was simply very poorly written. You could WTF as much as you want, that's not a native applet. This is one of the drawbacks of supporting third-party applets. We try to watch for bad practices and traps but it's not easy to catch, especially when it happens gradually.

I realize our documentation isn't as great as it could be - unfortunately the best documentation is looking at Cinnamon's core ui code, and its applets and desklets. We try to make getting at system services and states pretty easy, and if it makes sense, we'll make it so it's available for spices to use (like the screensaver or session state). Questions like 'How can I...' brought up in Github discussions I'll usually try to answer also.

I did look at the time change listener, I realize it only triggers for that specific event. I think this is too far to go for efficiency. An extra process running on the remote (or at least, fairly uncommon) chance that the time gets updated. 99% of the time if this triggers, it will be because your timezone changed (which you're also listening separately for). Very likely the timezone monitor is enough.

GLib.timeout_add_seconds(GLib.PRIORITY_DEFAULT, 60 * 30, this._periodic_update.bind(this));
....
....
_periodic_update() {
    if (check_the_current_time_against_our_schedule()) {
        /* Do work */
    }

    return GLib.SOURCE_CONTINUE;
}

Doing something as simple as comparing times every 30 minutes (or even every 5) will not negatively impact your desktop.

[guillaume-mueller]

OK, thanks. I will work on these.