Conversation
|
@flacjacket if you have the time can you approve CI on this |
|
This is great, thank you @jwijenbergh and @m-col! I'd love to see pywlroots 0.17.0. |
jwijenbergh
force-pushed
the
wlr-0.17
branch
3 times, most recently
from
0b54ba7 to
01201f8
Compare
|
Hi @flacjacket A polite nudge on this. Would be great if we can move onto 0.17 support soon. Thanks! |
|
Thank you for this! I missed that this was a PR and not an issue noting what is needed to get to 0.17. I'll take a look at this this weekend, if it is possible to update the conflicts we can work on getting this merged in! |
Thanks! Will work on the conflicts and CI later today |
OutputState used property setters, whereas Output uses set_. In this codebase, it seems like there is a preference for explicit set_ functions as there is little use of these setters
| layout changes. If the output is already in the layout, it will become | ||
| auto configured. If the position of the output is set such as with | ||
| `wlr_output_layout_move()`, the output will become manually configured. | ||
| def add_auto(self, output: Output) -> OutputLayoutOutput | None: |
There was a problem hiding this comment.
I am not sure if I like the return value here. Would be better if pywlroots raises an exception if it runs out of memory. Other functions do this as well in this rare case.
There was a problem hiding this comment.
hmmm I am not sure I like the other way around, in my mind I prefer to have everything closer to wlroots API so it's easier to upgrade to a new wlroots. There are still a lot of APIs that do Class | None
There was a problem hiding this comment.
Out of memory should be a very rare error. However, if the return value can also be None, we impose the handling of this rare error on everyone using pywlroots.
It is true that we have functions whose return value can also be None, but this is usually not an indicator of an error, but rather that a value was not set.
I'm not a fan of exceptions, in fact I've already made a few suggestions about replacing them with appropriate return values, but I think an exception makes sense here.
| return Output(output_ptr) | ||
|
|
||
| def add(self, output: Output, lx: int, ly: int) -> None: | ||
| def add(self, output: Output, lx: int, ly: int) -> OutputLayoutOutput | None: |
There was a problem hiding this comment.
See comment regarding add_auto
|
Thanks for pushing to make this happen! |
5 participants
supersedes #129, testing CI here. totally not done yet, currently I just added the fractional scale API. Need to see what is missing here