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

Enhancement: Adding a option to list matched profiles #8

Merged
merged 6 commits into from
Jan 24, 2018

Conversation

op8867555
Copy link
Contributor

@op8867555 op8867555 commented Jan 17, 2018

This PR adds a option randrctl list -s to list matched profiles.

> randrctl list -s
office_dp1_normal 10
office_dp1 10
default 3
laptop-rot-right 1

I'm using this for several months with rofi and found it is quite convenient. I have binded XF86Display to randrctl auto and Shift+XF86Display to:

randrctl switch-to $(randrctl list -s | rofi -dmenu -p "randrctl switch-to " | cut -d" " -f1)

A quick screenshot to describe that:
2018-01-17-182658_1920x1080_scrot

There are some design choices can be discussed(for example, using space or tab as separator). Please take a look and feel free to review it.

@koiuo
Copy link
Owner

koiuo commented Jan 17, 2018

Thanks for a great feature. I'll review this PR later today or tomorrow.

@@ -167,6 +167,15 @@ def find_best(self, available_profiles: list, xrandr_outputs: list):
score = self._calculate_profile_score(p, xrandr_outputs)
if score >= 0:
Copy link
Owner

Choose a reason for hiding this comment

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

what do you think about changing this to > 0 ?

Honestly, I do not remember, why it is >= 0 and I haven't touched code for quite a while, so you might know better now.

But at least in the context of the feature you implemented listing profiles with score 0 doesn't seem to have any sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I'm right, a profile scored 0 means its doesn't have any match rules and it will always match. I'm not sure about what is the scenario people use a profile without any match rules. So s > 0 sounds more sense than s >= 0 in this case. I will change it to s > 0 later.

@koiuo
Copy link
Owner

koiuo commented Jan 24, 2018

@op8867555
So I checked, why we need score 0.

We want to match profiles that just declare the list of expected outputs

match:
  LVDS1: {}
  HDMI1: {}
outputs:
  LVDS1:
    ...
  HDMI1:
    ...

Behavior is: apply profile, if LVDS1 and HDMI1 are connected; and it doesn't matter, what is connected.

Previously there was a bug in the code, where profiles without match section were considered for auto-matching and score 0 was always calculated for such profiles.

I fixed the bug and updated tests (so, hopefully, it is now clear from tests, what the behavior should be).

Please, revert your last commit.

P.S. sorry for taking so long to reply and thanks again for contribution.

`match` now always return a sorted list, so it's no need to sort it again in
`find_best`.
@op8867555
Copy link
Contributor Author

Thanks for making it clear! I reverted the commit and removed some unnecessary parts.

@koiuo koiuo merged commit 0809488 into koiuo:master Jan 24, 2018
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