fix: implement full desktop spec

friedow commented 3 months ago

This PR implements a bunch of fixes around the applications plugin. This aligns the applications plugin closer with the freedesktop desktop entry specification. Furhtermore it fixes #110.

This PR includes:

Hiding desktop entries based on the following logic:
- Type has to be "Application"
- Hidden has to be false or unset
- NoDisplay has to be false or unset
- the XDG_CURRENT_DESKTOP has to be in OnlyShowIn and not in NotShowIn
Terminal desktop entries are now opened in the first terminal emulator that is found on the system (fixes things like btop not being able to launch)

colemickens commented 3 months ago

(random: I wonder if this code is useful to other projects. A rust "freedesktop-desktop-spec" crate or something for other launchers that do this? Curious if pop's launcher has similar code.)

Also, unfortunately I'm still seeing roughly the same result: screenshot-1711991260

friedow commented 3 months ago

Under the hhod we're using the https://crates.io/crates/freedesktop-desktop-entry crate here. This provides all of the parsing for .desktop entries and is actually kindly provided by pop ;). However, this library only contains the parsing of .desktop files and no utility functions for launchers like centerpiece.

I've actually tested this with the .desktop files you provided above. For me, centerpiece only shows the "Firefox Nightly Default" entry. Can you check if you have other .desktop entries for Firefox in your system?

Screen with your three .desktop files: 2024-04-01-195405_hyprshot

friedow commented 3 months ago

Some context here: centerpiece looks for .desktop files in all directories specified in $XDG_DATA_DIRS and $XDG_DATA_HOME. It will only search for desktop entries in the subfolder applications of these directories.

(Library code related to searching .desktop files: https://github.com/pop-os/freedesktop-desktop-entry/blob/main/src/lib.rs#L329-L333.)

colemickens commented 3 months ago

Yes, that's where other launchers like Sirula load from as well.

And indeed, that's where they are:

╭ zeph  ~  13ms
╰🡒  ls $"($env.XDG_DATA_HOME)/applications" | find firefox
────┬───────────────────────────────────────────────────────────────────────┬──────┬───────┬────────────
  # │                                 name                                  │ type │ size  │  modified
────┼───────────────────────────────────────────────────────────────────────┼──────┼───────┼────────────
  0 │ /home/cole/.local/share/applications/firefox-nightly-default.desktop  │ file │ 744 B │ 5 months
    │                                                                       │      │       │ ago
  1 │ /home/cole/.local/share/applications/firefox-nightly.desktop          │ file │ 752 B │ 5 months
    │                                                                       │      │       │ ago
  2 │ /home/cole/.local/share/applications/firefox.desktop                  │ file │ 696 B │ 2 months
    │                                                                       │      │       │ ago
────┴───────────────────────────────────────────────────────────────────────┴──────┴───────┴────────────

So, as you can see, the extra entries are being loaded, it just seems that there still isn't complete support for the spec - the Name overrides, the hiding via Hidden=true/NoDisplay=true, etc, is still not supported or working right.

friedow commented 3 months ago

Just for my sanity, can you please check whether you have other .desktop files for firefox in one of the directories under $XDG_DATA_DIRS.

Also: What is wrong with the Name overrides? To me, the names seem to be correct?

colemickens commented 3 months ago

I surely do have entries in the XDG_DATA_DIRS directories since those include all applications I have installed into my environment.

I'm pretty sure the XDG_DATA_HOME are meant to act as overrides if present in both.

My original issue shows Sirula correctly rendering the Firefox entries.

The three desktop files I supplied should result in two entries appearing:

Firefox should not appear (the entry should mask the one from XDG_DATA_DIRS and be suppressed since it has Hidden, NoDisplay)
DetSys should appear (Name overriden)
Firefox Nightly Default should appear (Name overridden)

Just to re-demonstrate, here's the only Firefox entries Sirula shows me: screenshot-1711650263 screenshot-1711650258

colemickens commented 3 months ago

Quite annoying the spec refers to XDG_DATA_HOME as being a place to install desktop entries, but then does not list it in the section for where to look for entries. Nor does it talk about precedence in such a case. Very annoying. Though I'm still pretty sure this is how most other launchers work. I would expect my setup to appear correctly with GNOME/Plasma.

a-kenji commented 3 months ago

I also find it quite annoying, but it is in a different spec:

https://specifications.freedesktop.org/menu-spec/latest/ar01s02.html

It would be nice, if they could cross reference.

friedow commented 3 months ago

I guess since all other launchers just use XDG_DATA_HOME as a sole source for .desktop files if it is defined, we should do the same then?

Also: Can you please post your .desktop files for firefox again? I can't seem to find the line of code where the Name is overridden with "DetSys".

a-kenji commented 3 months ago

From the link for the menu spec it seems that XDG_DATA_DIRS are used, which XDG_DATA_HOME is one part of:

$XDG_DATA_DIRS/applications/ This directory contains a .desktop file for each possible menu item. Each directory in the $XDG_DATA_DIRS search path should be used (i.e. desktop entries are collected from all of them, not just the first one that exists). When two desktop entries have the same name, the one appearing earlier in the path is used.

colemickens commented 3 months ago

Also: Can you please post your .desktop files for firefox again? I can't seem to find the line of code where the Name is overridden with "DetSys".

Oh, I'm so sorry, there's indeed a 4th desktop file for that profile:

> cat ~/.local/share/applications/detsys.desktop
[Desktop Entry]
Actions=new-private-window;new-window;profile-manager-window
Categories=Network;WebBrowser
Exec=firefox -P detsys --name firefox %U
GenericName=Web Browser
Icon=firefox
MimeType=text/html;text/xml;application/xhtml+xml;application/vnd.mozilla.xul+xml;x-scheme-handler/http;x-scheme-handler/https
Name=DetSys
StartupNotify=true
StartupWMClass=firefox-nightly
Terminal=false
Type=Application
Version=1.4

[Desktop Action new-private-window]
Exec=firefox-nightly --private-window %U
Name=New Private Window

[Desktop Action new-window]
Exec=firefox-nightly --new-window %U
Name=New Window

[Desktop Action profile-manager-window]
Exec=firefox-nightly --ProfileManager
Name=Profile Manager

friedow commented 3 months ago

After some offline discussion with @a-kenji, I think I got it right this time :D. Would you kindly test again @colemickens?

colemickens commented 3 months ago

Oh I feel like such a bother, but: nix run github:friedow/centerpiece?ref=49a2c91915d08ce1ca3196b13e1eb24d4df15095 resulted in the same view:

screenshot-1712094780

friedow commented 3 months ago

No worries at all :)!

I assume we are now running into your $XDG_DATA_DIRS having the wrong order of entries. To confirm this, please run the following:

echo $XDG_DATA_DIRS:$XDG_DATA_HOME: | sed 's/:/\/applications\n/g' | xargs -I{} find {} -name '*firefox*.desktop'

This should give us a list of firefox .desktop entries in any of the directories specified in $XDG_DATA_DIRS and $XDG_DATA_HOME. This list is actually already in the correct order. Meaning, that if there are two .desktop files in this list with the same Name property inside, the first one wins. To debug ths properly it would be nice if you could also show the contents of the found desktop files.

My guess is, that in this list there are firefox .desktop entries with have no Hidden and no NoDisplay property specified and those are ranked higher in priority (due to the order of $XDG_DATA_DIRS) then the ones you would actually like to see (or not to see rather :D).

colemickens commented 3 months ago

❯ echo $XDG_DATA_DIRS:$XDG_DATA_HOME: | sed 's/:/\/applications\n/g' | xargs -I{} find {} -name '*firefox*.desktop'
find: ‘/nix/store/b45sdskxbr4s71dklhd13skqvyhbzrl6-desktops/share/applications’: No such file or directory
find: ‘/home/cole/.nix-profile/share/applications’: No such file or directory
find: ‘/nix/profile/share/applications’: No such file or directory
find: ‘/home/cole/.local/state/nix/profile/share/applications’: No such file or directory
/etc/profiles/per-user/cole/share/applications/firefox.desktop
/etc/profiles/per-user/cole/share/applications/firefox-nightly.desktop
find: ‘/nix/var/nix/profiles/default/share/applications’: No such file or directory
/home/cole/.local/share/applications/firefox-nightly.desktop
/home/cole/.local/share/applications/firefox-nightly-default.desktop
/home/cole/.local/share/applications/firefox.desktop

Hm, you're right. I didn't check the spec, but XDG_DATA_DIRS normally contains system dirs, and XDG_DATA_HOME more containing user dirs, so I'm a bit surprised that XDG_DATA_HOME ones aren't considered to override or at a higher precedence.

friedow commented 3 months ago

Nice :), now that we have the root cause of this problem identified, this should be fixable.

This morning I actually found the section that specifies the hirarchy of these environment variables in the XDG specification. It can be found in the XDG Base Directory Specification.

TLDR:

The order of base directories denotes their importance; the first directory listed is the most important. When the same information is defined in multiple places the information defined relative to the more important base directory takes precedent. The base directory defined by $XDG_DATA_HOME is considered more important than any of the base directories defined by $XDG_DATA_DIRS.

This means that the order in which directories are read in the freedesktop-desktop-entry crate (https://github.com/pop-os/freedesktop-desktop-entry/blob/main/src/lib.rs#L329-L333) is wrong since it specifies $XDG_DATA_DIRS before $XDG_DATA_HOME.

I'll go and fix this in the freedesktop-desktop-entry crate and I'll hotfix this by overriding this function in centerpiece for now. Will ping you here once this is ready :).

friedow commented 3 months ago

Hey @colemickens, I've just pushed the hotfix prioritizing XDG_DATA_HOME over XDG_DATA_DIRS. Wanna test one more time :)?

friedow commented 3 months ago

I've also created a PR to the freedesktop-desktop-entry crate to upstream this fix: https://github.com/pop-os/freedesktop-desktop-entry/pull/10.

colemickens commented 3 months ago

I'm quite sad to report that my laptop decided to drink some coffee. I'm limping along from a non-Linux device, and giving it some time to air out. Hopefully, I can test next week, but it could be a month if it's kaput.

friedow commented 3 months ago

OH NO! I'm rooting for you that it turns on again!

I'll revert my hotfix here and update the https://github.com/pop-os/freedesktop-desktop-entry crate, since the upstream fix was already merged :). After that I'll merge this PR.

Feel free to test this and open a new issue whenever you're back to normal mode @colemickens :).

friedow commented 3 months ago

Actually, I noticed some negative performance impact in the plugin with the new functionality while doing my last tests. I guess I'll have to revise the implementation.

colemickens commented 2 months ago

The laptop, somehow, lives to fight more Nix and Rust (and Halo!). This latest version seems to behave as I was expecting. Thank you!!

friedow commented 2 months ago

Performance is improved, the plugin does not read desktop enties repeatedly anymore, time to merge this :>.

friedow / centerpiece

fix: implement full desktop spec #120