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

fix: remove cause of hyprlock crashing (caused by multiple instances of hyprlock running) + misc. Hypridle.conf changes #309

Merged
merged 2 commits into from
May 29, 2024

Conversation

ThaSiouL
Copy link
Contributor

Pull Request

Description

changed hypridle.conf and LockScreen.sh so hyprlock won't be called if it is already running, as that renders all hyprlock instances useless. (checkable by pressing Ctrl + Alt + L twice, only killall hyprlock fixes this.)

"loginctl lock-session" now calls hyprlock and "loginctl unlock-session" kills hyprlock. (in case someone has a program that calls the generic commands or follows am online guide in tty or something if they are having problems with hyprlock)

Add a automatic hyprlock call when going to sleep as well as turning the screen on after waking up.

Copied 2 listeners to turn off the screen from my personal config. One after locking the display and the other doing this faster in case hyprlock is already running.

I commented out the 2 listeners as they would change the default behavior.

Type of change

Please put an x in the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update (non-breaking change; modified files are limited to the documentations)
  • Technical debt (a code change that does not fix a bug or add a feature but makes something clearer for devs)
  • Other (provide details below)

Checklist

Please put an x in the boxes that apply:

  • I have read the CONTRIBUTING document.
  • My code follows the code style of this project.
  • My commit message follows the commit guidelines.
  • My change requires a change to the documentation.
  • I want to add something in Hyprland-Dots wiki.
  • I have added tests to cover my changes.
  • I have tested my code locally and it works as expected.
  • All new and existing tests passed.

Additional context

Add any other context about the problem here.

…es running)

Also added optional Display Timeouts to hypridle.conf (commented out by default)
# unlock_cmd = notify-send "unlock!" # same as above, but unlock
before_sleep_cmd = hyprlock # command ran before sleep
# after_sleep_cmd = notify-send "Awake!" # command ran after sleep
lock_cmd = pidof hyprlock || hyprlock # runs hyprlock if it is not already running (this is always run when "loginctl lock-session" is called)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
lock_cmd = pidof hyprlock || hyprlock # runs hyprlock if it is not already running (this is always run when "loginctl lock-session" is called)
lock_cmd = pidof hyprlock || hyprlock --immediate # runs hyprlock if it is not already running (this is always run when "loginctl lock-session" is called)

Else there is a bit delay when locking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just kept it like it was in the listener beforehand.

I thought the flag was just to skip the grace period defined in the hyprlock.conf.

I might be slightly biased as it is a needed feature for me since I personally disable the notifications before locking and have my grace period set to 10 seconds so I can unlock it again fast if I am just sitting there doing nothing.

Copy link
Contributor

@JohnRTitor JohnRTitor May 29, 2024

Choose a reason for hiding this comment

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

No issues, for me the flag helps mitigate the issue where if I quickly move my mouse right after executing hyprlock, the session does not lock properly and quickly unlocks itself.

I am using hyprlock or hypridle configs provided by these dots without any modification.

This very well could be a upstream bug though.

EDIT: this is intended as can be seen from the logs

[LOG] In grace and cursor moved more than 5px, unlocking!

Copy link
Owner

Choose a reason for hiding this comment

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

No issues, for me the flag helps mitigate the issue where if I quickly move my mouse right after executing hyprlock, the session does not lock properly and quickly unlocks itself.

I am using hyprlock or hypridle configs provided by these dots without any modification.

This very well could be a upstream bug though.

EDIT: this is intended as can be seen from the logs

[LOG] In grace and cursor moved more than 5px, unlocking!

but the unlock immediately was because of grace period aint it?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, both for grace period and cursor moving.

@JaKooLit
Copy link
Owner

JaKooLit commented May 25, 2024

hey guys.. I will be busy this week so I can only review / merge once I verify / test

looking at this closed issues on hyprlock

https://github.com/hyprwm/hyprlock/issues?q=is%3Aissue+is%3Aclosed

most of Vaxry's reply was a bug in hyprland, which is fixed on upstream....

I suggest to install hyprland-git or from source and test the normal hyprlock command without going around...

Id rather wait for the fix than trying to get around with it because then we are just inviting all the unnecessary bugs again that might come along due to unnecessary codes / work around

@ThaSiouL
Copy link
Contributor Author

I have those parts of the script from the example in the official hyprland wiki and used this with the Arch-stable packages for some time already.

It was best practice at some point. Maybe it still is or it might be redundant now and the wiki is out of date...

I changed over to the -git packages for now and will test the change.

@JaKooLit
Copy link
Owner

I have those parts of the script from the example in the official hyprland wiki and used this with the Arch-stable packages for some time already.

It was best practice at some point. Maybe it still is or it might be redundant now and the wiki is out of date...

I changed over to the -git packages for now and will test the change.

Cool.. let me know outcome of you your test...

actually for the past few days, even on my gentoo, I havent seen any issues on hyprlock / hypridle...

same on my Arch with hyprland-git version....

@JaKooLit
Copy link
Owner

any comment / change in here?

@ThaSiouL
Copy link
Contributor Author

i tried both the old and new config with the git packages and both work as they should. so you could just close this if you like.

But i still think calling hyprlock via lock_cmd feels better, as long as you don't want to send extra args like you do in LockScreen.sh.

@JaKooLit
Copy link
Owner

i tried both the old and new config with the git packages and both work as they should. so you could just close this if you like.

But i still think calling hyprlock via lock_cmd feels better, as long as you don't want to send extra args like you do in LockScreen.sh.

I have now removed the --immediate command and just leave the -q

can you explain about what you said calling hyprlock via lock_cmd?

@JohnRTitor
Copy link
Contributor

JohnRTitor commented May 29, 2024

I agree. loginctl lock-session is a standard among screenlockers and should be used. This PR makes that possible. I have tested this PR and it works as it should (except for the above problem I mentioned, but I can patch that easily in my fork by adding --immediate)

Please rebase this PR on top of upstream/development and remove merge conflicts. @JaKooLit I think this is good to go. lock_cmd makes it possible to lock the screen using systemd's own command, loginctl lock-session

@JaKooLit
Copy link
Owner

weird.....

my screen is not locking if I execute command loginctl lock-session

... am I missing something?

lmao

@ThaSiouL
Copy link
Contributor Author

ThaSiouL commented May 29, 2024

@JohnRTitor you don't need to set that in the hypridle.conf. Just set grace = 0 in your hyprlock.conf so it is always immediate for you.

I feel like the only time a normal user wants to lock immediately is when locking manually (eg. pressing ctrl + alt + L to execute lockscreen.sh)

See the description of the --immediate flag:

~ ❱ hyprlock --help
Usage: hyprlock [options]

Options:
  -v, --verbose            - Enable verbose logging
  -q, --quiet              - Disable logging
  -c FILE, --config FILE   - Specify config file to use
  --display (display)      - Specify the Wayland display to connect to
  --immediate              - Lock immediately, ignoring any configured grace period
  -h, --help               - Show this help message

@JohnRTitor
Copy link
Contributor

JohnRTitor commented May 29, 2024

my screen is not locking if I execute command loginctl lock-session

... am I missing something?

You need to apply the changes in this PR to hypridle.conf. Then relogin.

@ThaSiouL
Copy link
Contributor Author

yeah hypridle has no way to re-parse the config after it is started.

@JaKooLit JaKooLit changed the base branch from development to dev-hypridle May 29, 2024 18:35
@JaKooLit JaKooLit merged commit 703287a into JaKooLit:dev-hypridle May 29, 2024
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.

None yet

3 participants