weisJ / auto-dark-mode

IDEA plugin to automatically apply system theme settings on macOS and Windows.
https://plugins.jetbrains.com/plugin/14076-auto-dark-mode
MIT License
53 stars 14 forks source link

WIP: Settings DSL #9

Closed weisJ closed 4 years ago

weisJ commented 4 years ago

This PR implements a configuration DSL that is agnostic of any implementation details of the IntelliJ Plugin SDK.

The aim is to enable platform implementation modules to specify custom settings that appear in the plugin settings of the IDE (e.g. on Linux).

Settings are declared by creating a class that extends DefaultSettingsContainer (e.g. DarkModeThemeSettings) and is registered as a service for SettingsContainer.

In the current state it is capable of fully modelling the existing settings. Though changes may need to be done to accommodate more use cases (this needs to be tested after Linux support has been merged).

Note: The new settings format isn't compatible with the old one so any custom changes will need to be redone by the user (not a big problem for such a small plugin).

weisJ commented 4 years ago

@IncPlusPlus if you like you can work on the settings for gnome based on this branch. This way any missing api for the settings can be ironed out before merging.

IncPlusPlus commented 4 years ago

Sounds like a plan. I'll start familiarizing myself with these changes but I can't promise that I'll be able to make any commits today.

weisJ commented 4 years ago

Great! DarkModeThemeSettings.kt and Settings.kt should be a good starting point to get going.

IncPlusPlus commented 4 years ago

Alright. I've been picking apart the changes here and I understand what is happening. I'm working on an implementation now. I'll probably make GnomeThemeSettings.kt right next to DarkModeThemeSettings.kt.

IncPlusPlus commented 4 years ago

There are two requirements for making the Gnome module work without guessing. I'm quite stuck with both of these. I'm hoping that maybe you had some ideas on how to accomplish this.

  1. GnomeThemeSettings.kt needs to be able to retrieve the list of installed themes to present the drop-down list
    • What I intended to do was follow the structure of DarkModeThemeSettings.kt with GnomeThemeSettings.kt. I wanted to make use of persistentChoiceProperty so that users can pick which of their installed GTK themes they use as their light and dark themes. I made a utility function in IncPlusPlus/auto-dark-mode@30568b639c6f95e8fb45c079099766c38733bf50 which provides a list of the installed themes.
    • There's a slight problem with this. I need GnomeThemeSettings.kt to be able to access that method. However, I can't simply change implementation(project(":auto-dark-mode-linux-gnome")) in linux/build.gradle.kts to api(project(":auto-dark-mode-linux-gnome")) and make the method public as well. This always causes an UnsatisfiedLinkError upon calling it.
    • I have tested that calling this function works by printing the output of GnomeThemeUtils.getInstalledThemes() whenever GnomeThemeMonitorService#isDarkThemeEnabled is called. The method works when isDarkThemeEnabled is called from AutoDarkMode.kt but not when called from GnomeThemeSettings.kt.
    • I tried using ServiceManager.getService(ThemeMonitorService::class.java) as LinuxThemeMonitorService and nabbing the GnomeThemeMonitorService with some additional code but this still led to the linking error. I'm not sure what's wrong. Are there separate JVMs running for each of these modules?
  2. GnomeThemeMonitorService needs to know what the user has set in their preferences as far as which of their GTK themes are their light and dark themes.
    • I thought maybe we could have ThemeMonitorService#install accept an Object that could provide any information that a ThemeMonitorService implementation could need. I scrapped this idea as it threw type-safety to the wind.
    • I tried a generic solution that also passed an object through the install method. However, this quickly got overcomplicated
weisJ commented 4 years ago

I think I wasn't quite clear enough in my explanation with the purpose of this PR, that's my fault.

  1. GnomeThemeSettings.kt needs to be able to retrieve the list of installed themes to present the drop-down list ...
  2. GnomeThemeMonitorService needs to know what the user has set in their preferences as far as which of their GTK themes are their light and dark themes.

The way the new approach to settings is structured, settings are declared at the use site, e.g. for Gnome that would be inside the linux-gnome package. Marking the SettingsContainer with @AutoService(SettingsContainer::class) will automatically generate the necessary service files for them. For your case the resulting declaration would be something like this:

@AutoService(SettingsContainer::class)
class GnomeSettingsProxy : SettingsContainer by GnomeSettings

object GnomeSettings : DefaultSettingsContainer() {
    ...
    var systemLightTheme : String = ""

    init {
        group("Gnome") {
            persistentChoiceProperty(
               value = ::systemLightTheme,
               ...
           ) {
               choices = GnomeThemeUtils.getInstalledThemes()
           }
        }
        ...
    }
}

As you can see GnomeSettings doesn't need to know how the settings panel is actually implemented or how the values are persisted. All necessary dependencies are already contained in auto-dark-mode-base.

IncPlusPlus commented 4 years ago

Ahhhhhh. That should make things a lot simpler. I didn't realize that @AutoService would work outside of the plugin module. I'll give it another shot.

weisJ commented 4 years ago

You will need to include the dependencies on com.google.auto:auto-service

weisJ commented 4 years ago

I think just for ease of use the guessing mechanism could be offered as an alternative to the exact provided values. Ideally this would be enabled the first time the plugin is loaded, with a popup notification that one should change the settings to not rely on guessing. This would require:

  1. A way for settings to be enabled/disabled (Should be quite straightforward to add a flag to the ValueProperty class).
  2. A way for implementation modules to dispatch notifications, without depending on the intellij sdk.

The first point would be part of this PR. The second more likely something for later.

IncPlusPlus commented 4 years ago

I agree. The guessing mechanism is a very nice default. I'll add that.

By dispatching notifications, are you talking about items that would appear in the Event Log (and possibly show a balloon too) or are you thinking about logging?

weisJ commented 4 years ago

I'm thinking specifically about events that appear in a balloon message.

IncPlusPlus commented 4 years ago

I've moved GnomeSettings added the following dependencies to linux/gnome/build.kts.gradle:

kapt(platform(project(":auto-dark-mode-dependencies-bom")))
kapt("com.google.auto.service:auto-service")
compileOnly("com.google.auto.service:auto-service-annotations")

I also added kotlin("kapt") to the plugins. For some reason, I still get UnsatisfiedLinkError. GnomeSettings.kt.txt frustration.txt

Is there something weird about the way it's being invoked?

weisJ commented 4 years ago

The settings are created before the theme monitor. You‘ll have to make sure the native library is loaded before using any methods of it.

IncPlusPlus commented 4 years ago

Gotcha

IncPlusPlus commented 4 years ago

Seems like .github/workflows/codeql-analysis.yml needs libgtk-3-dev.

weisJ commented 4 years ago

Seems like .github/workflows/codeql-analysis.yml needs libgtk-3-dev.

Yes. I can't modify it now, as the file isn't present in this branch.

@IncPlusPlus I have added a baloon notification regarding the guessing mechanism. Could you check that it shows up on your end? (You may have to first open a project, before the message shows up). Also I just quickly threw the notification message together, so there is room for improvement there.

IncPlusPlus commented 4 years ago

image Here's what I get when I first open the IDE.

IncPlusPlus commented 4 years ago

Updated to 8cc00d3ae38af5557f6df5d6b626127d79be6c34.

Here's what it looks like when you open the IDE and the message is displayed. image Here's what it looks like when you open a Project and are notified. image

Looking good!

weisJ commented 4 years ago

And the message is only shown, the first time you open the ide?

IncPlusPlus commented 4 years ago

Yep! Just double-checked with a clean environment. First time opening shows the message. After closing and reopening the IDE, the message is not shown again.