From c9c3c3ad653cbfcb1f424d369ff34c644704fd86 Mon Sep 17 00:00:00 2001 From: Vlad Zahorodnii Date: Mon, 3 Feb 2020 12:31:53 +0200 Subject: [PATCH] Introduce coding-conventions.md Summary: Hopefully, the new document makes it clear when one can use QRect::right() and QRect::bottom(). Reviewers: #kwin, davidedmundson Reviewed By: #kwin, davidedmundson Subscribers: davidedmundson, kwin Tags: #kwin Differential Revision: https://phabricator.kde.org/D27060 --- HACKING.md | 3 ++ doc/coding-conventions.md | 86 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+) create mode 100644 doc/coding-conventions.md diff --git a/HACKING.md b/HACKING.md index 749c9b506a..4aace2c7d8 100644 --- a/HACKING.md +++ b/HACKING.md @@ -91,5 +91,8 @@ KWin uses [KDE's phabricator instance](https://phabricator.kde.org) for code rev More complete documentation can be found in [KDE's wiki](https://community.kde.org/Infrastructure/Phabricator). Please add "#KWin" as reviewers. Please run KWin's automated test suite prior to uploading a patch to ensure that the change does not break existing code. +# Coding conventions +KWin's coding conventions are explained in document [coding-conventions.md](doc/coding-conventions.md). + # Coding style KWin code follows the [Frameworks coding style](https://techbase.kde.org/Policies/Frameworks_Coding_Style). diff --git a/doc/coding-conventions.md b/doc/coding-conventions.md new file mode 100644 index 0000000000..42bb4bfbd4 --- /dev/null +++ b/doc/coding-conventions.md @@ -0,0 +1,86 @@ +# Coding Conventions + +This document describes some of the recommended coding conventions that should be followed in KWin. + +For KWin, it is recommended to follow the KDE Frameworks Coding Style. + + +## `auto` Keyword + +Optionally, you can use the `auto` keyword in the following cases. If in doubt, for example if using +`auto` could make the code less readable, do not use `auto`. Keep in mind that code is read much more +often than written. + +* When it avoids repetition of a type in the same statement. + + ``` + auto something = new MyCustomType; + auto keyEvent = static_cast(event); + auto myList = QStringList({ "FooThing", "BarThing" }); + ``` + +* When assigning iterator types. + + ``` + auto it = myList.const_iterator(); + ``` + + +## `QRect::right()` and `QRect::bottom()` + +For historical reasons, the `QRect::right()` and `QRect::bottom()` functions deviate from the true +bottom-right corner of the rectangle. Note that this is not the case for the `QRectF` class. + +As a general rule, avoid using `QRect::right()` and `QRect::bottom()` as well methods that operate +on them. There are exceptions, though. + +Exception 1: you can use `QRect::moveRight()` and `QRect::moveBottom()` to snap a `QRect` to +another `QRect` as long as the corresponding borders match, for example + +``` +// Ok +rect.moveRight(anotherRect.right()); +rect.moveBottom(anotherRect.bottom()); +rect.moveBottomRight(anotherRect.bottomRight()); + +// Bad +rect.moveRight(anotherRect.left() - 1); // must be rect.moveLeft(anotherRect.left() - rect.width()); +rect.moveBottom(anotherRect.top() - 1); // must be rect.moveTop(anotherRect.top() - rect.height()); +rect.moveBottomRight(anotherRect.topLeft() - QPoint(1, 1)); +``` + +Exception 2: you can use `QRect::setRight()` and `QRect::setBottom()` to clip a `QRect` by another +`QRect` as long as the corresponding borders match, for example + +``` +// Ok +rect.setRight(anotherRect.right()); +rect.setBottom(anotherRect.bottom()); +rect.setBottomRight(anotherRect.bottomRight()); + +// Bad +rect.setRight(anotherRect.left()); +rect.setBottom(anotherRect.top()); +rect.setBottomRight(anotherRect.topLeft()); +``` + +Exception 3: you can use `QRect::right()` and `QRect::bottom()` in conditional statements as long +as the compared borders are the same, for example + +``` +// Ok +if (rect.right() > anotherRect.right()) { + return; +} +if (rect.bottom() > anotherRect.bottom()) { + return; +} + +// Bad +if (rect.right() > anotherRect.left()) { + return; +} +if (rect.bottom() > anotherRect.top()) { + return; +} +```