From 7ee85e8142174a806980cbc9c4b939a4255055f0 Mon Sep 17 00:00:00 2001 From: ozone10 Date: Fri, 7 Apr 2023 12:15:50 +0200 Subject: [PATCH] [xml] Add new guidelines, fix markdown warnings Ref: https://github.com/notepad-plus-plus/notepad-plus-plus/pull/13444#discussion_r1160284972 Close #13487 --- CONTRIBUTING.md | 336 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 232 insertions(+), 104 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ec5cce2af..a0b35094a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -4,7 +4,7 @@ ## Reporting Issues -Bug reports are appreciated. Following a few guidelines listed below will help speed up the process of getting them fixed. +Bug reports are appreciated. Following a few guidelines listed below will help speed up the process of getting them fixed. 1. Search the issue tracker to see if it has already been reported. 2. Disable your plugins to see if one of them is the problem. You can do this by renaming your `plugins` folder to something else. @@ -27,8 +27,6 @@ Your pull requests are welcome; however, they may not be accepted for various re Opening an issue beforehand allows the administrators and the community to discuss bugs and enhancements before work begins, preventing wasted effort. - - ### Guidelines for pull requests 1. Respect existing Notepad++ coding style. Observe the code near your intended change, and attempt to preserve the style of that code with the changes you make. @@ -39,22 +37,22 @@ Opening an issue beforehand allows the administrators and the community to discu 6. PR of reformatting (changing of ws/TAB, line endings or coding style) of source code won't be accepted. Use issue trackers for your request instead. 7. Typo fixing and code refactoring won't be accepted - please create issues with title started with `TYPO` to request the changing. 8. Address the review change requests by pushing new commits to the same PR. Avoid amending a commit and then force pushing it. All the PR commits are squashed before merging to the main branch. -9. Finally, please test your pull requests, at least once. +9. When creating new PR, try to base it on latest master. +10. Don't merge `upstream/master` (using git or via github sync), if your PR is based on older `upstream/master`. If you need to base it on latest `master` (e.g. to check and fix merge conflict), use commands `git fetch upstream` to get latest `master` and then `git rebase upstream/master` to rebase it onto this latest `upstream/master`. +11. Finally, please test your pull requests, at least once. In short: The easier the code review is, the better the chance your pull request will get accepted. - - ### Coding style -![stay clean](https://notepad-plus-plus.org/assets/images/good-bad-practice.jpg) - +![stay clean](https://notepad-plus-plus.org/assets/images/good-bad-practice.jpg) #### GENERAL -1. ##### Do not use Java-like braces. +1. Do not use Java-like braces + + * Good: - * ###### Good: ```cpp void MyClass::method1() { @@ -65,7 +63,8 @@ In short: The easier the code review is, the better the chance your pull request } ``` - * ###### Bad: + * Bad: + ```cpp void MyClass::method1() { if (aCondition) { @@ -73,8 +72,11 @@ In short: The easier the code review is, the better the chance your pull request } } ``` - However, the method definition could be defined in a header file (.h), if there's one line code only. In this case, Java-like braces should be used. - * ###### Good: + + However, the method definition could be defined in a header file (.h), if there's one line code only. In this case, Java-like braces should be used. + + * Good: + ```cpp class MyClass { @@ -88,152 +90,237 @@ In short: The easier the code review is, the better the chance your pull request int _x; } ``` -2. ##### Use tabs instead of white-spaces (we usually set our editors to 4 white-spaces for 1 tab, but the choice is up to you). +2. Use tabs instead of white-spaces (we usually set our editors to 4 white-spaces for 1 tab, but the choice is up to you) + +3. Always leave one space before and after binary and ternary operators -3. ##### Always leave one space before and after binary and ternary operators. + * Good: - * ###### Good: ```cpp if (a == 10 && b == 42) ``` - * ###### Bad: + * Bad: + ```cpp if (a==10&&b==42) ``` -4. ##### Only leave one space after semi-colons in "for" statements. +4. Only leave one space after semi-colons in "for" statements + + * Good: - * ###### Good: ```cpp for (int i = 0; i != 10; ++i) ``` - * ###### Bad: + * Bad: + ```cpp for(int i=0;i<10;++i) ``` -5. ##### Function names are not separated from the first parenthesis. +5. Function names are not separated from the first parenthesis + + * Good: - * ###### Good: ```cpp foo(); myObject.foo(24); ``` - * ###### Bad: + * Bad: + ```cpp foo (); ``` -6. ##### Keywords are separated from the first parenthesis by one space. +6. Keywords are separated from the first parenthesis by one space + + * Good: - * ###### Good: ```cpp if (true) while (true) ``` - * ###### Bad: + * Bad: + ```cpp if(myCondition) ``` -7. ##### Use the following indenting for "switch" statements: +7. Switch + + * Use the following indenting for "switch" statements: + + ```cpp + switch (test) + { + case 1: + { + // Do something + break; + } + default: + // Do something else + } // No semi-colon here + ``` + + * If possible use `default` statement, and prefer using it as last case. + * When using switch with enum or known range, try to cover all values if not using `default`. + + ```cpp + enum class Test {val1, val2, val3} + + switch (Test) + { + case Test::val1: + { + // Do something + break; + } + + //case Test::val2: + //case Test::val3: + default: + // Do something else + } // No semi-colon here + ``` + + When using `default` adding uncovered values as comments can help to convey intention. - ```cpp - switch (test) - { - case 1: - { - // Do something - break; - } - default: - // Do something else - } // No semi-colon here - ``` + * Use `[[fallthrough]]` if fall through is intended. -8. ##### Avoid magic numbers. + ```cpp + switch (test) + { + case 1: + { + // Do something + } + // I want fall through // adding comment can help to convey intention + [[fallthrough]]; + + case 2: + { + // Do something + break; + } + + default: + // Do something else + } // No semi-colon here + ``` + +8. Avoid magic numbers + + * Good: - * ###### Good: ```cpp if (foo == I_CAN_PUSH_ON_THE_RED_BUTTON) startTheNuclearWar(); ``` - * ###### Bad: + * Bad: + ```cpp while (lifeTheUniverseAndEverything != 42) lifeTheUniverseAndEverything = buildMorePowerfulComputerForTheAnswer(); ``` -9. ##### Prefer enums for integer constants. +9. Prefer enums for integer constants + +10. Use initialization with curly braces -10. ##### Use initialization with curly braces. + * Good: - * ###### Good: ```cpp MyClass instance{10.4}; ``` - * ###### Bad: + * Bad: + ```cpp MyClass instance(10.4); ``` -11. ##### Always use `empty()` for testing if a string is empty or not. +11. Always use `empty()` for testing if a string is empty or not + + * Good: - * ###### Good: ```cpp if (!string.empty()) ... ``` - * ###### Bad: + * Bad: + ```cpp if (string != "") ... ``` +12. Always use `C++ conversion` instead of `C-Style cast` + + * Generally, all the conversion among types should be avoided. If you have no choice, use C++ conversion. -12. ##### Always use `C++ conversion` instead of `C-Style cast`. Generally, all the conversion among types should be avoided. If you have no choice, use C++ conversion. + * Good: - * ###### Good: ```cpp char aChar = static_cast(_pEditView->execute(SCI_GETCHARAT, j)); ``` - * ###### Bad: + * Bad: + ```cpp char aChar = (char)_pEditView->execute(SCI_GETCHARAT, j); ``` -13. ##### Use `!` instead of `not`, `&&` instead of `and`, `||` instead of `or`. +13. Use `!` instead of `not`, `&&` instead of `and`, `||` instead of `or` + + * Good: - * ###### Good: ```cpp if (!::PathFileExists(dir2Search)) ``` - * ###### Bad: + * Bad: + ```cpp if (not ::PathFileExists(dir2Search)) ``` +14. Always initializatize local and global variables + + * For primitive types and enum prefer initialization with `=`. + * For other prefer `{}`-initializer syntax. + * For "numerical" variables using literal suffix can help to convey intention. + + ```cpp + constexpr float g_globalVariable = 0.0F; + + void test() + { + constexpr UINT strLen = 1024U; + wchar_t myString[strLen]{}; + } + ``` + #### NAMING CONVENTIONS -1. ##### Classes uses Pascal Case +1. Classes uses Pascal Case + + * Good: - * ###### Good: ```cpp class IAmAClass {}; ``` - * ###### Bad: + * Bad: + ```cpp class iAmAClass {}; @@ -241,87 +328,128 @@ In short: The easier the code review is, the better the chance your pull request {}; ``` -2. ##### Methods & method parameters use camel Case +2. Methods & method parameters - ```cpp - void myMethod(uint myVeryLongParameter); - ``` + * Use camel Case -3. ##### Member variables + ```cpp + void myMethod(uint myVeryLongParameter); + ``` -Any member variable name of class/struct should be preceded by an underscore. +3. Member variables - ```cpp - public: - int _publicAttribute; - private: - int _pPrivateAttribute; - float _pAccount; - ``` + * Any member variable name of class/struct should be preceded by an underscore. -4. ##### Always prefer a variable name that describes what the variable is used for. + ```cpp + public: + int _publicAttribute; + private: + int _pPrivateAttribute; + float _pAccount; + ``` + +4. Always prefer a variable name that describes what the variable is used for + + * Good: - * ###### Good: ```cpp if (hours < 24 && minutes < 60 && seconds < 60) ``` - * ###### Bad: + * Bad: + ```cpp if (a < 24 && b < 60 && c < 60) ``` - - #### COMMENTS -1. ##### Use C++ comment line style rather than C comment style. +1. Use C++ comment line style rather than C comment style - * ###### Good: - ``` + * Good: + + ```cpp // Two lines comment // Use still C++ comment line style ``` - * ###### Bad: - ``` + * Bad: + + ```cpp /* Please don't piss me off with that */ ``` +#### BEST PRACTICES + +1. Use C++11/14/17/20 whenever it is possible. + +2. Use C++11 member initialization feature whenever it is possible. + + ```cpp + class Foo + { + int value = 0; + }; + ``` + +3. Incrementing + * Prefer Pre-increment + ```cpp + ++i + ``` -#### BEST PRACTICES + * Over Post-increment -1. ##### Use C++11/14/17 whenever it is possible + ```cpp + i++ + ``` -2. ##### Use C++11 member initialization feature whenever it is possible + (It does not change anything for built-in types but it would bring consistency) - ```cpp - class Foo - { - int value = 0; - }; - ``` +4. Avoid using pointers. References are preferred instead. You might need the variable to be assigned a `NULL` value: in this case the `NULL` value has semantics and must be checked. Wherever possible, use a SmartPtr instead of old-school pointers. -3. ##### Prefer Pre-increment: - ```cpp - ++i - ``` +5. Avoid using new if you can use automatic variable. However, avoid `shared_ptr` as much as possible. Prefer `unique_ptr` instead. - ##### **Over Post-increment:** - ```cpp - i++ - ``` - (It does not change anything for built-in types but it would bring consistency) +6. Don't place any "using namespace" directives in headers. -4. ##### Avoid using pointers. References are preferred instead. You might need the variable to be assigned a NULL value: in this case the NULL value has semantics and must be checked. Wherever possible, use a SmartPtr instead of old-school pointers. +7. Compile time is without incidence. Increasing compile time to reduce execution time is encouraged. -5. ##### Avoid using new if you can use automatic variable. However, avoid `shared_ptr` as much as possible. Prefer `unique_ptr` instead. +8. Code legibility and length is less important than easy and fast end-user experience. -6. ##### Don't place any "using namespace" directives in headers. +9. Prefer `constexpr` over `const` if value can be evaluated at compile time. -7. ##### Compile time is without incidence. Increasing compile time to reduce execution time is encouraged. +10. Check if there are helper functions in headers or lambda fuctions to reuse them instead of writing new code. + + * Example + + ```cpp + // in StaticDialog.h + isCheckedOrNot(); + setChecked(); + + // in Parameters.cpp + parseYesNoBoolAttribute(); + ``` + +11. Check if there is already defined global variable, and reuse it instead of defining new one. + +12. Avoid "Yoda conditions". + + * Good: + + ```cpp + if (iAmYourFather == true) + ... + ``` + + * Bad: + + ```cpp + if (true == iAmYourFather) + ... + ``` -8. ##### Code legibility and length is less important than easy and fast end-user experience. +13. Check [C++ Core Guidlines](https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md) for additional guidelines.