rharish101 / ReGreet

Clean and customizable greeter for greetd
GNU General Public License v3.0
348 stars 16 forks source link

Respect precedence of found desktop files and Hidden/NoDisplay #41

Closed otaj closed 8 months ago

otaj commented 9 months ago

Hi,

this PR implements precedence of desktop files (only first one with the same file name is used). It also respects Hidden/NoDisplay values of desktop entries.

The motivation for this is that I have my own modified versions of desktop files located in /usr/local/share/wayland-session which are using my own wrapper script, which are different than the ones which are installed by package manager.

It also respects Hidden=true and NoDisplay=true.

I've tested this in my system and it seems to work, however, this my first code in Rust, so it definitely might use some improvements.

Cheers

rharish101 commented 9 months ago

I also have another concern: would this break existing desktops that have both X11 and Wayland sessions, but use the same file stem?

otaj commented 9 months ago

Looks good. I'd prefer if you could split the commit into two, one for the precedence and the other for Hidden/NoDisplay, but it's not a hard requirement.

I can do that, no problem.

Also, could you document the file stem behaviour in the README?

Will do as well.

I also have another concern: would this break existing desktops that have both X11 and Wayland sessions, but use the same file stem?

I don't think this happens though. Gnome, Plasma and Qtile have all different stems for Wayland and X11 sessions and I can't think of any more, that have both sessions. To be on a safe side, we can store something like parent directory + stem?

I will get to coding in couple days

rharish101 commented 9 months ago

I don't think this happens though. Gnome, Plasma and Qtile have all different stems for Wayland and X11 sessions and I can't think of any more, that have both sessions.

In that case, it's fine.

To be on a safe side, we can store something like parent directory + stem?

Well, that would be less convenient for your use case, right? Because you would need the parent directory for your custom files to match too. I think an easier and safer option would be a config option to enable/disable this behaviour. What do you think?

otaj commented 9 months ago

I don't think this happens though. Gnome, Plasma and Qtile have all different stems for Wayland and X11 sessions and I can't think of any more, that have both sessions.

In that case, it's fine.

To be on a safe side, we can store something like parent directory + stem?

Well, that would be less convenient for your use case, right? Because you would need the parent directory for your custom files to match too. I think an easier and safer option would be a config option to enable/disable this behaviour. What do you think?

I wrangled with the code for a bit. Now I'm storing OsStr, which contains the immediate prefix (xsessions or wayland-sessions). I was not able to easily split the commits as I already merged the updates to your upstream and my git-fu is not strong enough.

otaj commented 8 months ago

Apart from one tiny change, all that's missing is documenting this behaviour in the README.

Done

rharish101 commented 8 months ago

LGTM!