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

Shell command options for media keys #256

Merged
merged 5 commits into from
Aug 26, 2022
Merged

Conversation

jclds139
Copy link

Closes #136

Description

Enables options to provide a shell command to run for each of the keys that could be passed by --pass-*-keys.
A master option, --custom-key-commands enables all of the others, then each of --*-cmd corresponds to a single keysym, taking a shell command to run when that key is pressed.

This could look like a security hole, but it realistically should be no more dangerous than the existing --pass-*-keys options, which let the root window do whatever it wants in responds to the keypress.

Release notes

Notes: Enables custom shell commands to handle media, power, screen, and volume keys as an alternative to passing them to the root window.

Copy link

@JezerM JezerM left a comment

Choose a reason for hiding this comment

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

I suggest to change the naming to cmd-**-** for a better scheme, which might help to find these commands faster, like --cmd-brightness-up and --cmd-power-off.

This feature could be insecure, but yet it's opt to the user to take the risk and make sure it works with their environment. It's simpler and better than my previous solution.

@jclds139
Copy link
Author

jclds139 commented Mar 16, 2022

Good point! Done. Do you want the commits squashed together?

edit: apologies for accidentally closing the PR when commenting

@jclds139 jclds139 closed this Mar 16, 2022
@jclds139 jclds139 reopened this Mar 16, 2022
i3lock.c Outdated Show resolved Hide resolved
Copy link
Author

@jclds139 jclds139 left a comment

Choose a reason for hiding this comment

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

Is this acceptable? I tried to keep different key types to their own block of 10.

i3lock.c Show resolved Hide resolved
JezerM
JezerM previously approved these changes Jul 21, 2022
Copy link

@JezerM JezerM left a comment

Choose a reason for hiding this comment

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

Looks good to me~

@Raymo111
Copy link
Owner

Raymo111 commented Jul 21, 2022

@jclds139 @JezerM I can't get it to work? I'm running

./build.sh -d
LD_PRELOAD= ./build/i3lock --no-verify --cmd-volume-up="touch /tmp/success"

i3lock triggers correctly, but when I press the volume up key, no file is created.

@JezerM
Copy link

JezerM commented Jul 21, 2022

You're forgetting the --custom-key-commands flag to enable those commands.

@Raymo111
Copy link
Owner

You're forgetting the --custom-key-commands flag to enable those commands.

Thanks, I was being dumb :p

Copy link
Owner

@Raymo111 Raymo111 left a comment

Choose a reason for hiding this comment

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

Just a quick thing about strdup, everything else looks great and it works very nicely!

i3lock.c Outdated Show resolved Hide resolved
jclds139 added a commit to jclds139/i3lock-color that referenced this pull request Jul 22, 2022
jclds139 added a commit to jclds139/i3lock-color that referenced this pull request Jul 22, 2022
@jclds139 jclds139 requested a review from Raymo111 July 22, 2022 00:39
Copy link
Owner

@Raymo111 Raymo111 left a comment

Choose a reason for hiding this comment

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

Couple last things

i3lock-bash Outdated Show resolved Hide resolved
i3lock.c Outdated Show resolved Hide resolved
i3lock.c Outdated Show resolved Hide resolved
Copy link
Owner

@Raymo111 Raymo111 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for contributing!

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.

--pass-media-keys, --pass-screen-keys and --pass-power-keys options do not work in DEs
3 participants