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

Implement a client for communication to the daemon via sockets #18

Merged
merged 3 commits into from
Mar 26, 2019

Conversation

yashsriv
Copy link
Contributor

This commit implements --lock-now by enforcing a default socket file and using that for passing the \x02 byte when called with --lock-now. Its also possible to override the socket in normal launch and --lock-now via --socket.

@jD91mZM2
Copy link
Owner

What's wrong with

echo -ne "\x2" | socat - UNIX-CONNECT:/tmp/xidlehook.sock

?

While the code looks good, I'm a little hesitant to adding a default socket. One of the biggest points with xidlehook is being a general purpose timer: Not being exclusive to one instance, and not being exclusive to screen locking.

@yashsriv
Copy link
Contributor Author

This is because it would then resemble xautolock more, which xidlehook is replacing for me.

Also, its more concise for a user if you tell them:

To run primary timer command instantly, run:

$ xidlehook --lock-now

Instead of:

The socket API is very simple. Each command is a single byte, sent over a unix
socket. Bind a file using --socket /path/to/xidlehook.sock (where the path is
whatever you want), and then you can send one of the following bytes:

Byte Command
0 Deactivate
1 Activate
2 Run the timer command immediately

A common use case of xidlehook is using it to run a lockscreen. To then
manually lock the screen, you could bind this bash command to a shortcut:

echo -ne "\x2" | socat - /path/to/xidlehook.sock

@yashsriv
Copy link
Contributor Author

I think, a common ground between our views could be no default sockets, but the flag existing and being used with the socket flag.

xidlehook --lock-now --socket /tmp/xidlehook.sock

@jD91mZM2
Copy link
Owner

jD91mZM2 commented Mar 26, 2019

If we have --lock-now (which probably should have the generic name --trigger), we'll need --enable and --disable too. And adding all that to the code could be messy so it would maybe be better to make a separate client instead of bloating the xidlehook server.

Don't get me wrong, I don't dislike this at all. I'd use something like this myself if it existed -- buuut I want to modularize as much as possible to properly follow the UNIX philosophy of having each tool do one thing, but well.

Maybe this could be moved to either

  1. A separate project.
  2. A separate bin, but inside the main repo so it's a standard and gets installed alongside.

@yashsriv
Copy link
Contributor Author

I guess I got convinced by your argument 😛 . I have created a separate binary file for both.

@yashsriv yashsriv changed the title Implement --lock-now for instantly calling primary timer Implement a client for communication to the daemon via sockets Mar 26, 2019
@jD91mZM2
Copy link
Owner

Pushed a teeny tiny change, hope that was OK! I would prefer to have xidlehook still be main.rs, as it is still the "main" program, so to speak. Let me know if you think this is ready to be merged now, and I'll merge it!

@yashsriv
Copy link
Contributor Author

lgtm 👍

@jD91mZM2 jD91mZM2 merged commit 4fdda52 into jD91mZM2:master Mar 26, 2019
@jD91mZM2
Copy link
Owner

Thanks for this 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