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

Add toggle logic inside DPMS handler #6099

Merged
merged 1 commit into from
Mar 25, 2021
Merged

Add toggle logic inside DPMS handler #6099

merged 1 commit into from
Mar 25, 2021

Conversation

ivanfed0t0v
Copy link
Contributor

@ivanfed0t0v ivanfed0t0v commented Mar 15, 2021

Logic that obtains current DPMS state is put inside the handler.

sway_output from which the current DPMS state will be obtained is selected by the following logic:

  • For - and -- the focused output is used;
  • For * error "Cannot apply toggle to all outputs" is reported;
  • For everything else all_output_by_name_or_id() is used.

Fixes #5929.

@ivanfed0t0v
Copy link
Contributor Author

ivanfed0t0v commented Mar 15, 2021

Test cases:

  • $ swaymsg output <output> dpms toggle - should successfully toggle specified output.

On multi monitor setup:

  • $ swaymsg output - dpms toggle - should toggle just focused output.
  • $ swaymsg output * dpms toggle - should print "Cannot apply toggle to all outputs." and exit.
  • $ swaymsg output * dpms on - should work and not print any errors.
  • $ swaymsg output <unknown-output> dpms toggle - should print "Cannot apply toggle to unknown output".

sway/commands/output.c Outdated Show resolved Hide resolved
@rpigott
Copy link
Member

rpigott commented Mar 17, 2021

Hi, thanks for the PR.

The new intended behavior makes sense to me, at least. I doubt it would make much sense to toggle each output state individually.

@ivanfed0t0v
Copy link
Contributor Author

I doubt it would make much sense to toggle each output state individually.

I agree and I don't think it's currently possible to toggle DPMS on each output individually from inside the handler anyway.

Actually, handler at output/toggle.c simply reports an error when called on wildcard. I think I should do the same with DPMS for consistency.

Copy link
Member

@rpigott rpigott left a comment

Choose a reason for hiding this comment

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

Just one small thing – now that dpms has a "toggle" argument could you please add it to the sway-output man page? I think the meaning is clear, so just listing on|off|toggle for the argument of dpms is probably fine.

Other than that, this LGTM. Should probably be squashed for merging.

Logic that obtains current DPMS state is put inside the handler.

sway_output from which the current DPMS state will be obtained is selected by the following logic:
* For '-' and '--' the focused output is used;
* For '*' error "Cannot apply toggle to all outputs" is reported;
* For everything else all_output_by_name_or_id() is used.

Fixes #5929.
@ivanfed0t0v ivanfed0t0v changed the title Fix DPMS toggle handler Add toggle logic inside DPMS handler Mar 20, 2021
Copy link
Member

@emersion emersion 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, and it seems like @rpigott is fine with this version too.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

DPMS toggle
3 participants