-
Notifications
You must be signed in to change notification settings - Fork 63
Add named attribute access to gnome-monitor.py
(New)
#1916
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
base: main
Are you sure you want to change the base?
Conversation
b37882d
to
7e759db
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1916 +/- ##
==========================================
+ Coverage 50.46% 50.99% +0.53%
==========================================
Files 382 384 +2
Lines 41039 41589 +550
Branches 6892 7103 +211
==========================================
+ Hits 20709 21209 +500
- Misses 19585 19615 +30
- Partials 745 765 +20
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
gnome-monitor.py
(New)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please let me know, if there is some misunderstanding. Thanks.
checkbox-support/checkbox_support/dbus/tests/test_gnome_monitor.py
Outdated
Show resolved
Hide resolved
) | ||
|
||
|
||
class MutterDisplayMode(_MutterDisplayModeT): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using MutterDisplayMode
instead of Mode
will impact #1843. Please notice me, there should be a following PR right after this PR has merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I can export an alias Mode = MutterDisplayMode
to not break 1843 and remove the alias in a follow up PR
resolution_filter: Callable[[List[Mode]], List[Mode]] = None, | ||
action: Callable[..., Any] = None, | ||
resolution_filter: Optional[ResolutionFilter] = None, | ||
post_cycle_action: Optional[Callable[..., Any]] = None, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will impact #1843. Please notice me, there should be a following PR right after this PR has merged.
0e0c2ce
to
28ffd4a
Compare
Description
This PR adds complete type annotations and named properties to the original gnome-monitors script to hopefully make it more ergonomic to use. It's not something that needs to be urgently merged so don't worry about this if you have a fire to fix xD
Basically turns this:
into this:
with nice IDE autocomplete.
#1843 could take advantage of this to more directly access resolution numbers instead of strings
Resolved issues
In the original
MonitorConfigGnome
, accessing deeply nested properties from theGetCurrentState
's return value was a bit awkward. If we need to get the 1st refresh rate of the 1st monitor, we need to writestate[1][1][0][1][0][3]
which looks like magic numbers without looking at the original dbus xml. The originalMonitorConfigGnome
provided nice ways to get the resolution strings but all the other properties were not included.This PR circumvents this magic index situation by providing named attribute access for the return value of
GetCurrentState
, so now we can writestate.physical_monitors[0].modes[0].refresh_rate
and only use indices in places where there's an actual array.Documentation
The main reference point is this xml and gnome-randr
None of the new classes (MutterDisplayConfig, PhysicalMonitor, LogicalMonitor, MonitorInfo) should be directly constructed because they assume that the input GLib.Variant has the correct type. Use
MonitorConfigGnome.get_current_state()
as the entry point since it asks GLib to do a deep type check first before returning.Tests
Original unit tests