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

xidlehook: add module #1761

Merged
merged 3 commits into from
Jun 7, 2021
Merged

xidlehook: add module #1761

merged 3 commits into from
Jun 7, 2021

Conversation

dschrempf
Copy link
Contributor

Hi!

I am really enjoying the experience with Home Manager so far, thank you!

We could think about replacing xautlock together with caffeine-ng with xidlehook, see https://gitlab.com/jD91mZM2/xidlehook.(Or adding the xidlehook service instead of replacing). What do you think?

@thiagokokada
Copy link
Contributor

Well, I actually had a similar idea, so I would say go on.

I just don't know how to proceed here. Would it better substitute the services.screen-locker to use xidlehook + caffeine-ng, or would be better to have something like some way to handle both in configuration (I think this can possible make the module rather convoluted and confusing).

@rycee, have some opinion here 🤔 ?

@berbiche
Copy link
Member

I use xidlehook and caffeine-ng in my configuration and have an xidlehook module that is mostly ready to be committed (descriptions lack a final period and there might be more options to add).

https://github.com/berbiche/dotfiles/blob/7d3a007d63443a08696f4f8215ef7c7beeb60907/modules/home-manager/xidlehook.nix

@dschrempf
Copy link
Contributor Author

dschrempf commented Feb 7, 2021

I am using the mentioned xidlehook module for some time now and it works very well, thank you. I suggest merging it into the screen-lock module so that either xautolock or xidlehook can be used. The reason for the suggested merge is that "lock on suspend" is only possible using the combination with xss-lock which is already used therein. Of course, xidlehook could also be used for other stuff, so maybe this suggestion is not so well thought through.

@stale
Copy link

stale bot commented May 8, 2021

Thank you for your contribution! I marked this issue as stale due to inactivity. If this remains inactive for another 7 days, I will close this issue. Please read the relevant sections below before commenting.

If you are the original author of the issue

  • If this is resolved, please consider closing it so that the maintainers know not to focus on this.
  • If this might still be an issue, but you are not interested in promoting its resolution, please consider closing it while encouraging others to take over and reopen an issue if they care enough.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

If you are not the original author of the issue

  • If you are also experiencing this issue, please add details of your situation to help with the debugging process.
  • If you know how to solve the issue, please consider submitting a Pull Request that addresses this issue.

Memorandum on closing issues

If you have nothing of substance to add, please refrain from commenting and allow the bot close the issue. Also, don't be afraid to manually close an issue, even if it holds valuable information.

Closed issues stay in the system for people to search, read, cross-reference, or even reopen--nothing is lost! Closing obsolete issues is an important way to help maintainers focus their time and effort.

@stale stale bot added the status: stale label May 8, 2021
@dschrempf
Copy link
Contributor Author

I think this is still valid. I am still using the mentioned module. I will see if i find time to prepare a pull request (would that be adequate?).

@stale stale bot removed the status: stale label May 8, 2021
@rycee
Copy link
Member

rycee commented May 8, 2021

Unfortunately I don't know anything about any of these applications so I have no particular opinion. Ideally I guess it would be nice to have as simple setup as possible, i.e., to use one program or the other and not attempt to support both.

@berbiche
Copy link
Member

berbiche commented May 9, 2021

@dschrempf please do

dschrempf added a commit to dschrempf/home-manager that referenced this pull request May 31, 2021
See issue nix-community#1761.

This module has been created by 'berbiche' and (slightly) modified by
'dschrempf'. This is stated in the preamble, but let me know if you want a more
detailed statement of who authored what.

Also, @berbiche, please let me know if you want to be added as a maintainer.
@dschrempf dschrempf requested a review from rycee as a code owner May 31, 2021 08:19
@dschrempf
Copy link
Contributor Author

This module has been created by @berbiche and (slightly) modified by @dschrempf. This is stated in the preamble, but let me know if you want a more detailed statement of who authored what.

Also, @berbiche, please let me know if you want to be added as a maintainer.

I am using this module for half a year without problems. However, I do not use the advanced features such as cancellers or delays.

dschrempf added a commit to dschrempf/home-manager that referenced this pull request May 31, 2021
See issue nix-community#1761.

This module has been created by 'berbiche' and (slightly) modified by
'dschrempf'. This is stated in the preamble, but let me know if you want a more
detailed statement of who authored what.

Also, @berbiche, please let me know if you want to be added as a maintainer.
Copy link
Member

@berbiche berbiche left a comment

Choose a reason for hiding this comment

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

Thanks for the PR.

I don't use X11 anymore since I now have a decent screensharing experience on Wayland; therefore, I wouldn't be able to discover bugs in the module.

Could you add a news entry to news.nix?

.github/CODEOWNERS Outdated Show resolved Hide resolved
modules/services/xidlehook.nix Outdated Show resolved Hide resolved
modules/services/xidlehook.nix Outdated Show resolved Hide resolved
modules/services/xidlehook.nix Outdated Show resolved Hide resolved
modules/services/xidlehook.nix Show resolved Hide resolved
modules/services/xidlehook.nix Outdated Show resolved Hide resolved
modules/services/xidlehook.nix Outdated Show resolved Hide resolved
modules/services/xidlehook.nix Show resolved Hide resolved
@berbiche berbiche changed the title Xautolock + caffeine vs xidlehook xidlehook: add module Jun 1, 2021
dschrempf added a commit to dschrempf/home-manager that referenced this pull request Jun 2, 2021
Co-authored-by: Nicolas Berbiche <[email protected]>

See pull request at nix-community#1761.
Copy link
Member

@berbiche berbiche left a comment

Choose a reason for hiding this comment

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

Sorry, I missed one thing.

modules/modules.nix Outdated Show resolved Hide resolved
Co-authored-by: Nicolas Berbiche <[email protected]>

See pull request at nix-community#1761.
@berbiche
Copy link
Member

berbiche commented Jun 7, 2021

@dschrempf looks like you need to run the formatter against the code! ./format

@berbiche berbiche merged commit f74dc9c into nix-community:master Jun 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants