jD91mZM2 / xidlehook

GitLab: https://gitlab.com/jD91mZM2/xidlehook
MIT License
387 stars 33 forks source link

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

Closed yashsriv closed 5 years ago

yashsriv commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

jD91mZM2 commented 5 years ago

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 commented 5 years ago

lgtm :+1:

jD91mZM2 commented 5 years ago

Thanks for this PR! :smile: