Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding hyprland to available desktops and configs *i also fixed the logout button* #336

Closed
wants to merge 41 commits into from

Conversation

pyclicker
Copy link

@pyclicker pyclicker commented Oct 27, 2024

Pull request

Description

Please provide a brief description of the changes made in this pull request. Include any relevant context or background information.

Related Issue

If this pull request addresses an issue, please link to it here. For example:

  • Closes #(issue number)
  • Fixes #(issue number)

Changes Made

  • Added new feature: Describe the feature.
  • Fixed bug: Describe the bug and how it was fixed.
  • Updated documentation: Describe what was updated.
  • Other: Describe any other changes made.

Screenshots (if applicable)

If your changes include visual modifications, please include screenshots to demonstrate the changes.

Testing

Select the installation methods on which you tested your changes:

  • Flatpak
  • Snap
  • Native version from this repository

Checklist

  • I have read the contributing guidelines.
  • I have performed a self-review of my code.
  • I have commented my code where necessary.
  • I have updated the documentation accordingly.
  • My changes do not introduce any new warnings.

Additional Notes

src/config.py Outdated
# for package in available_packages:
# if package in str(name):
# packages.append(pathto(package))

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these lines are commented out, remove them.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure i will

@vikdevelop
Copy link
Owner

Hello, thank you very much for creating this PR. However, I am not satisfied with a number of things:

  1. You want to add (probably accidentally) the .flatpak-builder directory to the repository, but it contains unnecessary files that were created while building the application with Flatpak Builder. So remove this directory.
  2. For some reason, your PR is removing the contents of the native/install_native.sh file, which makes it impossible to install SaveDesktop in the desired directories in the home folder, and thus to install it. So revert its content.
  3. I deal with other changes in separate files.

src/config.py Outdated
elif os.getenv('XDG_CURRENT_DESKTOP') == 'Hyprland':
environment = 'Hyprland'
#else:
# from tty_environments import *
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tty_environments.py file is used to detect DE if SaveDesktop is running in TTY mode (i.e. pure CLI without graphical environment or tile WM)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i do not know why i deleted that ......

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -1612,6 +1614,8 @@ def logout(self, action, param):
os.system("dbus-send --print-reply --session --dest=org.kde.LogoutPrompt /LogoutPrompt org.kde.LogoutPrompt.promptLogout")
elif self.win.environment == 'COSMIC (New)':
os.system("dbus-send --print-reply --session --dest=com.system76.CosmicSession --type=method_call /com/system76/CosmicSession com.system76.CosmicSession.Exit")
elif self.win.environment == 'Hyprland':
os.system("hyprctl dispatch exit")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, the hyprctl command doesn't work in Flatpak, so you probably need to use dbus to log out the system, or use other options.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh thanks for the feedback i will do that one later because i do not know how to kill hyprland or logout hyprland with a systemcall

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commented out and wrote "needs to be fixed but i do not know how to make this call on flatpak via python"

src/config.py Outdated

# add command-line arguments
from localization import _, CACHE, DATA, system_dir, flatpak, snap, settings
home = os.getenv('HOME')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The home variable is defined in the src/localization.py file, so it is not necessary to use a new, same variable in this file. This is because Snap requires a custom home directory definition.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i will remove that line thanks for the feedback on that one i did not know the localization.py did that

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

src/config.py Outdated
elif environment == None:
elif environment == 'Hyprland':
os.system(f"cp -aur ./hypr {home}/.config/ -v")
elif environment == None:
Copy link
Owner

@vikdevelop vikdevelop Oct 27, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
elif environment == None:
elif environment == None:

This row is not aligned correctly, which generates a TabError.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

src/config.py Outdated Show resolved Hide resolved
else:
print(f'{self.win.environment} is not found in our database it would be of great value if you would leave a issue on our github page: https://github.com/vikdevelop/SaveDesktop')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
print(f'{self.win.environment} is not found in our database it would be of great value if you would leave a issue on our github page: https://github.com/vikdevelop/SaveDesktop')

Since most DEs except KDE, Cosmic (Rust version) and Xfce use org.gnome.SessionManager, there is no need to add this message here.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay i will remove it in my next commit

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -1612,7 +1613,11 @@ def logout(self, action, param):
os.system("dbus-send --print-reply --session --dest=org.kde.LogoutPrompt /LogoutPrompt org.kde.LogoutPrompt.promptLogout")
elif self.win.environment == 'COSMIC (New)':
os.system("dbus-send --print-reply --session --dest=com.system76.CosmicSession --type=method_call /com/system76/CosmicSession com.system76.CosmicSession.Exit")
elif self.win.environment == 'Hyprland':
os.system("hyprctl dispatch exit") #does logout without prompting the user because i couldn't find a good way to do it
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
os.system("hyprctl dispatch exit") #does logout without prompting the user because i couldn't find a good way to do it
os.system("hyprctl dispatch exit") if not flatpak else os.system(f'notify-send "{_["err_occured"]}" "It is not possible to log out of the system in the Flatpak environment. Please log out manually."')

Unfortunately I also don't know what D-Bus interface to use for Hyprland, so I've set it up so that if the application is running outside of Flatpak, the command is executed, if it's not, a notification is sent that you need to log out manually.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i found a way to bypass the dbus and just kill the process but will need to test if it works it is with psutil os and signal

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what i get is this {1: 'bwrap', 2: 'savedesktop'} is there a way to not sandbox the processes and find them via the program?

Copy link
Author

@pyclicker pyclicker Oct 28, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately i can not kill or send a dbus req to hyprland since it doesn't expose a port/bus so i think ill just look into how to remove that button when the wm is not found

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of importing the psutil and signal modules, you can use this command, which should hopefully work on all DEs (I tried it on GNOME, and it works):

dbus-send --system --print-reply --dest=org.freedesktop.login1 /org/freedesktop/login1 'org.freedesktop.login1.Manager.TerminateSession' string:$(dbus-send --system --print-reply --dest=org.freedesktop.login1 /org/freedesktop/login1 'org.freedesktop.login1.Manager.ListSessions' | awk -F 'string "' '/string "/ {print $2; exit}' | awk -F '"' '{print $1}')

however, it is necessary to add permissions to the Flatpak manifest in the finish-args section: --system-talk-name=org.freedesktop.login1.

@pyclicker
Copy link
Author

everything you mentioned is done

@vikdevelop
Copy link
Owner

everything you mentioned is done

After all, I suggested using the command given here: #336 (comment), which is a universal command that should work everywhere. So there's no reason to make it difficult to disable the Log Out button, just enable the --system-talk-name=org.freedesktop.login1 permission in the manifest. It's also possible to enable it in the Flatseal app.

@pyclicker
Copy link
Author

everything you mentioned is done

After all, I suggested using the command given here: #336 (comment), which is a universal command that should work everywhere. So there's no reason to make it difficult to disable the Log Out button, just enable the --system-talk-name=org.freedesktop.login1 permission in the manifest. It's also possible to enable it in the Flatseal app.

unfortunately that does not work for me in a plain shell and it returns
method return time=1730113271.934049 sender=:1.15 -> destination=:1.1505 serial=3341 reply_serial=2

@pyclicker pyclicker requested a review from vikdevelop October 28, 2024 11:06
Tygo added 2 commits October 28, 2024 12:08
…g so i figured that it works but doesnt close hyprland and only closes the portal or smh
src/main_window.py Outdated Show resolved Hide resolved
@@ -31,7 +30,23 @@ def __init__(self, *args, **kwargs):
# set the window size and maximization from the GSettings database
(width, height) = settings["window-size"]
self.set_default_size(width, height)


# Check the user's current desktop
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Check the user's current desktop
# Check the user's current desktop
desktopenv = os.getenv('XDG_CURRENT_DESKTOP')

gi.require_version('Gtk', '4.0')
gi.require_version('Adw', '1')
from datetime import date
from pathlib import Path
from threading import Thread
from gi.repository import Gtk, Adw, Gio, GLib
from localization import _, home, download_dir
from localization import _, home, download_dir, desktopenv
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
from localization import _, home, download_dir, desktopenv
from localization import _, home, download_dir

src/main_window.py Outdated Show resolved Hide resolved
pyclicker and others added 3 commits October 28, 2024 11:16
sure thanks for the suggestion

Co-authored-by: vikdevelop <vikdevelop@proton.me>
Co-authored-by: vikdevelop <vikdevelop@proton.me>
@pyclicker pyclicker requested a review from vikdevelop October 28, 2024 14:01
@vikdevelop
Copy link
Owner

Thank you for your contribution! However, I have another branch called release_3.5 that contains significant changes, so I can't merge this PR into the main branch because then I wouldn't be able to merge the release_3.5 branch. Instead, I'll copy your changes manually into the release_3.5 branch and close this PR.

The changes you have contributed will be included in the stable version 3.5, but that will be released a bit later. In the meantime, you can participate in the beta testing, where the next beta version will be released in a few days, which will already include your changes.

@vikdevelop vikdevelop closed this Oct 28, 2024
vikdevelop added a commit that referenced this pull request Oct 28, 2024
... and improve the sentence about unsupported option to log out of the Hyprland WM

This and two previous commits are related to the #336 PR
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants