nix-community / plasma-manager

Manage KDE Plasma with Home Manager
https://nix-community.github.io/plasma-manager/
MIT License
643 stars 74 forks source link

The `panels` option causes the file `.config/plasma-org.kde.plasma.desktop-appletsrc` to grow indefinitely #76

Closed FF-AntiK closed 8 months ago

FF-AntiK commented 8 months ago

The panels option creates new panels on every login (and removes the old ones beforehand). All the widgets inside the new panels will then be registered in plasma-org.kde.plasma.desktop-appletsrc. Unfortunately KDE Plasma doesn't cleanup the old entries, so the file will grow indefinitely.

I don't know if it would be safe to delete that file every time. KDE Plasma seems to automatically regenerate the file, but maybe there are some side-effects on the containment / widget numbering.

toast003 commented 8 months ago

I have noticed after a while it made the taskbar hang for a few seconds when plugging in a monitor, but apart from that it doesn't seem to cause any issues.

That said that's on a 7840U with a pretty decent SSD, and I see this being a problem on old/low power devices. We might want to add a warning about this on the readme

FF-AntiK commented 8 months ago

One problem that is caused by this issue, is how keyboard shortcuts for panel applets normally work on plasma. When you configure a keybinding on one of your panel applets (e.g. plasma-pass), you will notice that the applet will be registered with a (random?) containment and applet number inside plasma-org.kde.plasma.desktop-appletsrc. Even though the actual keybinding is configured in this file, the keybinding also needs to be in kglobalshortcutsrc. The way this works, is that inside kglobalshortcutsrc it refers to the applet number inside plasma-org.kde.plasma.desktop-appletsrc.

But if you now delete all you pannels, and regenerete new ones via plasma-manager's panels option on every login. These applet numbers are no longer vaild, because new ones were generated for the new panels.

toast003 commented 8 months ago

Did a bit of testing and I think we can fix this! Screenshot_20240228_104045 It also shows up with the same ID in kglobalshortcutsrc

toast003 commented 8 months ago

Might look into this but I need to finish the konsole module first

FF-AntiK commented 8 months ago

How would a possible fix look like? Add a script to panels.extraSettings which gets the new applet numbers and writes them into plasma-org.kde.plasma.desktop-appletsrc and kglobalshortcutsrc?

I'm still a bit concerned about the old entries. If you would want to remove the old entries from both files, you would need to know which widget ids are no longer valid.

Also I'm wondering what's the maximum number plasma does generate for these ids, and if it does search for a unused number, as soon as a certain limit is reached.

I think this wouldn't be something to worry about too much for a KDE dev, because how many panels will a user possibly create in his entire life? But again, with plasma-manager we are creating new panels on every login...

toast003 commented 8 months ago

We could save the IDs when we generate the panels so that we can clean up old entries. Btw that screenshot was talking about the keyboard shortcut thing you mentioned

I think this wouldn't be something to worry about too much for a KDE dev, because how many panels will a user possibly create in his entire life? But again, with plasma-manager we are creating new panels on every login...

Unless you have overrideConfig enabled we only run the panels script when there's a change, and if you have that enabled you're probs deleting both files anyways

FF-AntiK commented 8 months ago

Ah okay, thank you for clarification! I was under the impression the script will run on every startup. I've just lazily checked the number of lines in plasma-org.kde.plasma.desktop-appletsrc after every startup and it was growing everytime. But maybe this is caused by another issue. I will test this again, as soon as I have time.

toast003 commented 8 months ago

Hmm... Are you keeping last_run_layouts in .local/share/plasma-manager? If that's gone it will run the scripts every time

FF-AntiK commented 8 months ago

I've now tested it. The last_run_layouts is updated on every startup, so I guess the panel script runs on every startup in my case...

FF-AntiK commented 8 months ago

so, regarding the keyboard shortcut I've came up with this little plasma script:

      panels = [{
        extraSettings = ''
          panelIds.forEach((panel) => {
            panel = panelById(panel)
            panel.widgetIds.forEach((widget) => {
              widget = panel.widgetById(widget)
              if(!widget) return
              switch(widget.type) {
                case "org.kde.plasma.icontasks":
                  widget.currentConfigGroup = ["General"]
                  widget.writeConfig("launchers", "")
                  break;
                case "org.kde.plasma.systemtray":
                  systray = widget.readConfig("SystrayContainmentId")
                  systray = desktopById(systray)
                  systray.widgets().forEach((w) => {
                    if (w.type === "org.kde.plasma.pass") {
                      shortcut = "Meta+A"
                      w.currentConfigGroup = ["Shortcuts"]
                      w.writeConfig("global", shortcut)
                      config = ConfigFile("kglobalshortcutsrc")
                      config.group = "plasmashell"
                      config.writeEntry("activate widget " + w.id, shortcut)
                    }
                  })
              }
            })
          })
        '';

        /* ... */
      }];

But it won't work, because I would need to delete the old entry in kglobalshortcutsrc first (it won't allow you to have the same key-binding in multiple entries).

So, I guess it would be nice to have the old widget ids available in extraSettings. I think cleaning up the plasma-org.kde.plasma.desktop-appletsrc should preferably be done by plasma-manager internally.

I still have the issue of the script running on every startup. I will try to figure this out later.

FF-AntiK commented 8 months ago

I've now looked a little bit deeper into the "script runs on every startup" issue. The problem is, that .local/share/plasma-manager/data/layout.js always has the startup timestamp. The file itself is a link to /nix/store/...-home-manager-files/.local/share/plasma-manager/data/layout.js which is another link to /nix/store/...-hm_plasmamanagerdatalayout.js.

So, I assume that the timestamp layout.js has, is the timestamp of when the link was created, which I think is on every login.

I'm using home-manager as a nixos module, so I can build the whole system config including the home-manager config with a single nixos-rebuild. Could this kind of setup lead to the issue?

toast003 commented 8 months ago

I'm running home manager as a NixOS module too, and afaik I didn't have that happen ¯\_(ツ)_/¯

@FF-AntiK do you not have any last_run_* files in .local/share/plasma-manager?

FF-AntiK commented 8 months ago

The last_run_* files are present. But when the contents of last_run_layouts is compared with the timestamp of layout.js, it will always be older, because layout.js always has the current startup timestamp. I assume it is the time, when the link to the actual ...hm_plasmamanagerdatalayout.js was created, which seems to happen on every startup.

FF-AntiK commented 8 months ago

I cannot tell for sure, because the original file in the nix store has no timestamp (1. Jan 1970).

toast003 commented 8 months ago

It doesn't compare the timestamp against layout.js, it compares it against the last run files

Here's the script that applies theme settings:

#!/bin/sh
last_update=$(stat -c %Y "$0")
last_update_file=/home/toast/.local/share/plasma-manager/last_run_themes
stored_last_update=0
if [ -f "$last_update_file" ]; then
    stored_last_update=$(cat "$last_update_file")
fi

if [ $last_update -gt $stored_last_update ]; then
    success=1
    plasma-apply-lookandfeel -a Catppuccin-Mocha-Mauve || success=0
    plasma-apply-desktoptheme default || success=0
    plasma-apply-cursortheme Breeze_Snow || success=0
    plasma-apply-colorscheme CatppuccinMochaMauve || success=0
    /nix/store/9lfr9sqjnifnc51i1w3n6yxhkrlgxs28-plasma-workspace-5.27.10/libexec/plasma-changeicons breeze-dark || success=0

    [ $success = 1 ] && echo "$last_update" > "$last_update_file"
fi
FF-AntiK commented 8 months ago

Looking at the code, it does compare it to the timestamp of layout.js:

https://github.com/pjones/plasma-manager/blob/74fa336c22adf822c6ef13b21bfff18fbef66e9c/modules/panels.nix#L183

toast003 commented 8 months ago

:raised_eyebrow: Huh I guess I did miss that

toast003 commented 8 months ago

@magnouvean any ideas on what could be happening here?

magnouvean commented 8 months ago

I have noticed this too. I think the problem may have to do with how symlinks work with nix, and that they may get symlinked again after reboot. I think I though they worked differently when I wrote this, and didn't consider time-changes between reboots. On my machine at least it runs every time I reboot, but not when I log out and re-log back in. What I think we could do is to instead of checking the timing of the file, to just check the sha256-sum or something, and compare that instead. As far as solving this I think we can just delete this file before generating the plasma-layout. I can't see anything wrong with that tbh as that is what I actually thought was happening, and the file is not needed when all the panels are deleted anyway.

magnouvean commented 8 months ago

B.t.w. @FF-AntiK a much cleaner and simpler way of doing what you have done above would be something like:

panels = {
        widgets = [
          {
            name = "org.kde.plasma.icontasks";
            config = {
              General.launchers = [
                "applications:org.kde.dolphin.desktop"
               "enter more applications you want here"
              ];
            };
          }
        ];
};

See how you can specify what is written with writeConfig directly for each of the widgets via the config-option. I do see that this is not well documented at all. We really should work some more on documentation once we have gotten some more functionality in place as it is lagging quite behind I see. What is causing the problem might not be that there is a duplicated kglobalshortcuts entry or something, but one problem could be that writeConfig in general writes to [Containments][SomeID][Applets][AnotherID][Configuration], meaning what I would expect the above to do is write to [Containments][SomeID][Applets][AnotherID][Configuration][Shortcuts], but I seem to remember you saying the config needing to be in [Containments][SomeID][Applets][AnotherID][Shortcuts]. If this is the source of the problem there hopefully is a way in plasma-scripts to write also to such groups and that we can implement this as an option as well.

FF-AntiK commented 8 months ago

Deleting plasma-org.kde.plasma.desktop-appletsrc would also revert any desktop and panel changes that were made outside of plasma-manager. If this is desirable or not is an open question, I guess.

But also deleting the file wouldn't solve the problem of incrementing widget ids. As in my plasma-pass example, this will invalidate previously configured keyboard shortcuts. And who knows where else Plasma references panel widget ids.

@magnouvean thanks for the example. This is helpful for all the widgets directly in the panel. But it won't help for widgets inside the system-tray like plasma-pass. Also it won't help with the kglobalshortcutsrc problem, where we need to reference the correct widget id.

Regarding the sections in plasma-org.kde.plasma.desktop-appletsrc fortunately it works with and also without the [Configuration] part :-)

magnouvean commented 8 months ago

Ah alright, that's a good sign I guess. I would think the panels-option would revert all the panels already though seeing as it will delete all the panels and re-generate them on each startup. I see you are probably correct though with deleting the file wont solve much as I use overrideConfig (which deletes the file on every generation) and my plasma-org.kde.plasma.desktop-appletsrc is still much longer than what it probably should be (2000 lines or something). I got no idea what is happening there though, maybe something weird on the kde plasma side.

FF-AntiK commented 8 months ago

Yeah, I think it is an odd decision of the KDE devs to not automatically clean up plasma-org.kde.plasma.desktop-appletsrc (also I wish the filename was shorter ^.^)

I cannot speak for your overrideConfig setup, but I already manually deleted the file and didn't see any old entries. Deleting the file is like a "fresh start" for me. The only thing I see, is that plasma won't re-use any widget ids. It will always increment from the ids it used before, whether the file is deleted or not.

FF-AntiK commented 8 months ago

So far I'd vote for comparing checksums insted of timestamp, regarding the apply_layout script. This would at least prevent incrementing the widget ids too often.

Regarding the incrementing widget ids, I could live with that. In this case I would prefer plasma-manager deleting appletsrc file by default and mentioning in the docs that changes to the config outside of plasma-manager will not persist.

But I still would need a solution for my special case with keyboard shortcuts for widgets inside the systray. Maybe I will be able to add something to my extraSettings script, to delete all old entries of my shortcut in kglobalshortcutsrc.

I still think it's a little bit odd that I have to define the shortcut both in appletsrc and also kglobalshortcutsrc.

FF-AntiK commented 8 months ago

@magnouvean I've just checked the overrideConfig option you mentioned. plasma-org.kde.plasma.desktop-appletsrc is not in the default list of config files. Did you specify it in overrideConfigFiles?

magnouvean commented 8 months ago

I appears you are correct, nice spot. For some reason I always thought it was there. It definitely should be there though. I think that list was just gathered from the list of files tracked by rc2nix, but it is definitely due an update it looks like. There are probably even more files that belong there as well.

FF-AntiK commented 8 months ago

Since I enabled overrideConfig and added plasma-org.kde.plasma.desktop-appletsrc to overrideConfigFiles, I don't even get incremented widget numbers. It looks very smooth now to be honest.

The kglobalshortcutsrc doesn't behave like I'd expected though. I was hoping that deleting it would result in having no shortcuts at all (plus the ones configured in plasma-manager). But it will always have the default shortcuts. I still have to test what happens now, if I use some of the default shortcuts for other tasks. Will it override the default shortcut or not? I'll check this later.

FF-AntiK commented 8 months ago

It seems like default shortcuts will be overwritten automatically when using some of the default key-sequences for other tasks.

So all my issues are fixed now! :-) Do you want me to close the issue, or do you want to keep it open, until you've added some minor improvements (startup script compares checksum, plasma-org.kde.plasma.desktop-appletsrc added to the default config files, ...)?

magnouvean commented 8 months ago

Overrideconfig will essentially set all the settings not explicitly set in your nix config to the default ones, but the ones you set in your nix config should not be overridden. If you want to unset some of the settings (which is often needed when there are colliding keybindings for example) you may want to explicitly disable them in your config. In some cases adding "[$i]" at the end of your key can help settings not being overridden.

magnouvean commented 8 months ago

We can keep it open as it is still a problem when not using overrideConfig, and probably needs some minor tweaks

FF-AntiK commented 8 months ago

I've explicitly tested without "unsetting" colliding shortcuts. I don't know for sure, but I assume my config will apply first and the defaults will be added later, so they loose when there are colligind key-sequences.

I may be wrong though.

magnouvean commented 8 months ago

I think it is a bit unpredictable. In my experience it is needed in some cases (I need them for "Meta+1", "Meta+2", ... as far as I am aware), but other times it is fine. Nice that things have worked out!

magnouvean commented 8 months ago

As per #78 autostart scripts now use sha256sum instead of stat for checking for changes in panels/theme configurations. Also the plasma-org.kde.plasma.desktop-appletsrc should now be deleted before the panels are generated, which hopefully should somewhat solve the problem of the file growing indefinitely. Therefore I think I can close this issue now. One thing to note though is that I did find out that activations are not only ran at a new generation, but also at boot, which does mean that when using overrideConfig, all the files are deleted and regenerated on each boot. This also means that panels and themes will have to also be generated/applied on every boot as well. That might have been the reason why the stat command didn't work, I don't really know. I still think using a sha-sum is the better option though as it should compare the content of the file and not when the file was created.