It's more intuitive.
Details
- Reviewers
- None
- Group Reviewers
KWin - Commits
- R108:85c099936ec8: Use QRect::moveRight() and QRect::moveBottom() in XdgShellClient…
Relevant tests pass.
Diff Detail
- Repository
- R108 KWin
- Branch
- split-out-xdg-shell-popup
- Lint
Lint Errors Excuse: asdf Severity Location Code Message Error input.h:310 Cppcheck unknownMacro - Unit
No Unit Test Coverage - Build Status
Buildable 21881 Build 21899: arc lint + arc unit
xdgshellclient.cpp | ||
---|---|---|
1710 | So what is this comment about then? |
xdgshellclient.cpp | ||
---|---|---|
1710 | It says that you can't do popupPosition.moveRight(bounds.x() + bounds.width()); |
or to rephrase zzag:
.right() is off by one
.setRight is off by one
so it cancels out.
Personally I find it so difficult to parse that it's easier to just boycott the method.
Everyone who comes across it and is aware of QRect's ridiculousness ends up having to do a load of additional checking in their heads anyway.
Though we don't have a kwin policy on that and we use QRect::right/bottom in other places, so I can't have a formal objection here.
I just approved another patch that used setRight setBottom in the same method, so we may as well be consistent.
Hmm, then these two https://github.com/KDE/kwin/blob/master/xdgshellclient.cpp#L1653-L1659 also have to be fixed.
I personally would prefer to continue using QRect::move{Right,Bottom}() and QRect::set{Right,Bottom}() as long as borders match, e.g.
rect.moveRight(anotherRect.right());
I just approved another patch that used setRight setBottom in the same method, so we may as well be consistent.
What I meant, was: you can ship this given we also use setRight(other.right()) elsewhere
I just can't press the button now that it's abandoned.