|
|
| (149 intermediate revisions by one other user not shown) |
| Line 1: |
Line 1: |
| == Purpose of this document ==
| | {{Moved To Community}} |
| | |
| This document describes the recommended coding style for kdepim and akonadi. Nobody is
| |
| forced to use this style, but to have consistent formatting of the source code
| |
| files it is strongly recommended to make use of it.
| |
| | |
| ''In short: Kdepim and akonadi coding style follows the''
| |
| [http://techbase.kde.org/Policies/Kdelibs_Coding_Style Kdelibs coding style].
| |
| | |
| We need at least:
| |
| *a specification (a set of rules) for the coding style of the sources
| |
| *some tools to check the sources against the specification
| |
| *some tools to change the sources
| |
| | |
| astyle is a pretty tool to make such changes. But astyle doesn't implement (yet) all the specification rules.
| |
| You can find below some awk-scripts which help us to make most of the changes.
| |
| The last part must be done manually.
| |
| | |
| == The specification rules of coding style for kdepim and akonadi ==
| |
| | |
| *Don't use any <TAB>s
| |
| *Trim the lines
| |
| *Only single empty lines
| |
| *The first line, the last line(s) may not be empty
| |
| *Use one space after each keyword, but not after a cast
| |
| *No "one line" if-statement
| |
| *No ; after some macros
| |
| *Use a space after the name of the class
| |
| *Don't use untyped enum
| |
| *Don't use enum with empty member
| |
| *Place '''*''' and '''&''' near the variable
| |
| *Don't use '''&,''' without a variable
| |
| *No space for '''& >''', '''* >''', '''( )''' or '''( )'''
| |
| *No space arround the index of an array
| |
| *Each initialization of a method in a separate line
| |
| *'''switch''' rules
| |
| *'''if''', '''for''', '''while''' and similar macros rules
| |
| *No space arround an expression surrounded with braces
| |
| *Surround all operators with spaces
| |
| *identation with four spaces
| |
| | |
| == Migration ==
| |
| | |
| As discussed at the KDEPIM meeting, Berlin, 3 March 2013, all the files of KDEPIM will
| |
| be reviewed to follow the coding style. This will be done over a long time,
| |
| directory after directory, for each of the
| |
| rules defined above. For each rule, one can find one or two script(s).
| |
| | |
| The main part of the changes can be done with astyle:
| |
| http://astyle.sourceforge.net/
| |
| | |
| | |
| | |
| == The scripts ==
| |
| | |
| The first script is '''to check''' a single file or a complete directory for all .h and
| |
| .cpp files.
| |
| | |
| If present, the second script '''makes the changes''' for a single file or a complete
| |
| directory for all .h and .cpp files. For some complicated situations, the
| |
| script makes no change.
| |
| | |
| One can use the scripts for own work.It is recommanded to use them in this order.
| |
| | |
| === Don't test a subdirectory ===
| |
| | |
| If a '''.no_coding_file''' file is present on a subdirectory, the test will not be done.
| |
| | |
| === don't use any <TAB>s ===
| |
| *coding-style-check-Tabs.sh
| |
| *The changes are well done with astyle
| |
| | |
| Download the scripts: [[Media:Tabs.tar.gz]]
| |
| | |
| The output of the '''check''' script is:
| |
| {{Output|1=check the file /home/guy-kde/projects/kdepimlibs/ktnef/ktnefparser.cpp
| |
| 1->308: Tab at 16: stream_ >> i; // i <- attribute type & name
| |
| 2->311: Tab at 16: stream_ >> i; // i <- data length
| |
| 3->326: Tab at 22: case attATTACHMENT: // try to get attachment info
| |
| 4->367: Tab at 16: stream_ >> u; // u <- checksum
| |
| a b c d}}
| |
|
| |
| This shows:
| |
| * the name of the file which is under test.
| |
| * the number of occurence('''a'''), the line number('''b'''), the position found('''c''') and the line itself('''d''').
| |
| | |
| === Trim the lines ===
| |
| | |
| *coding-style-check-Trim.sh
| |
| *The changes are well done with astyle
| |
| | |
| Download the scripts: [[Media:Trim.tar.gz]]
| |
| | |
| The output of the '''check''' script is:
| |
| {{Output|1=check the file /home/guy-kde/Software/coding-style-check/trim.cpp
| |
| 1->51: Space(s) at end of line (28): QVariant m_matchData;}}
| |
|
| |
| This shows:
| |
| * the name of the file which is under test.
| |
| * the number of occurence, the line number, the position found and the line itself.
| |
| | |
| === Only single empty lines should be used ===
| |
| Refer to http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Whitespace
| |
| | |
| *coding-style-check-Twice.sh
| |
| *coding-style-change-Twice.sh
| |
| * astyle cannot (yet) do it
| |
| | |
| Download the scripts: [[Media:Twice.tar.gz]]
| |
| | |
| The output of the '''check''' script is:
| |
| {{Output|1=check the file /home/guy-kde/projects/kdepimlibs/syndication/rss2/enclosure.cpp
| |
| 1->25: next empty line found
| |
| 2->26: next empty line found
| |
| 3->30: next empty line found}}
| |
|
| |
| This shows:
| |
| * the name of the file which is under test.
| |
| * the number of occurrences and the line numbers.
| |
| | |
| The '''change''' script:
| |
| *removes all the next empty line(s).
| |
| | |
| === First line, last line(s) may not be empty ===
| |
| | |
| Some of the sources have a first empty lines, some have one or more empty last line(s).
| |
| *coding-style-check-First-Last.sh
| |
| *coding-style-change-First-Last.sh
| |
| *astyle cannot (yet) do it
| |
| | |
| Download the scripts: [[Media:First.tar.gz]]
| |
| | |
| The output of the '''check''' script is:
| |
| {{Output|1=check the file /home/guy-kde/Software/coding-style-check/trim.cpp
| |
| The first line is empty
| |
| The last line is empty}}
| |
|
| |
| The '''change''' script:
| |
| *removes the first line if empty, all the last empty line(s).
| |
| | |
| === put the comments away ===
| |
| | |
| The comments might contain some keyword. It is very difficult to avoid the confusion with the very simple awk-scripts. We prefer to change all the comments with the same number of empty lines.
| |
| | |
| *Comments.awk
| |
| | |
| Download the scripts: [[Media:Comments.tar.gz]]
| |
| | |
| === change the strings ===
| |
| | |
| It is very difficult to parse the strings correctly, so we prefer to change them to an empty string.
| |
| | |
| *Strings.awk
| |
| | |
| Download the scripts: [[Media:Strings.tar.gz]]
| |
| | |
| | |
| After the use of the last two awk-scripts (Comments.awk and Strings.awk), we go on with the next check.
| |
| | |
| ===no "one line" if-statement===
| |
| | |
| Refer to http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Braces
| |
| | |
| The following code:
| |
| {{Output|1=if ( a > b ) c = 123;}}
| |
| is correct, but we prefer the block:
| |
| {{Output|1=if ( a > b ) {
| |
| c = 123;
| |
| } }}
| |
| which is easier to read, to modify.
| |
| | |
| It is also possible to put a breakpoint at the line in the block.
| |
| | |
| As the awk-script is too simple to recognize all the if-statements, we get some false alarm and
| |
| we can't make the changes automatically.
| |
| | |
| *coding-style-check-One-Line-If.sh
| |
| | |
| Download the scripts: [[Media:One-Line-If.tar.gz]]
| |
| | |
| The output of the '''check''' script is:
| |
| {{Output|1=check the file /home/guy-kde/Software/coding-style-check/if-example.cpp
| |
| 1->25: one-line-if found}}
| |
| | |
| ===Pedantic===
| |
| | |
| Looking over the git-history, one can find some "pedantic" changes.
| |
| These are changes to make a better code. The most of them are at the use of macro, where it is not necessary to have a ''';''' at the end ofthe command.
| |
| The script make a check over all these:
| |
| '''AKTEST_MAIN;MAKE_CMD_ROW;Q_DECLARE_FLAGS;Q_PRIVATE_SLOT;Q_DECLARE_METATYPE;Q_DECLARE_OPERATORS_FOR_FLAGS;Q_DE
| |
| CLARE_PRIVATE;Q_DECLARE_PUBLIC;Q_DISABLE_COPY;K_GLOBAL_STATIC;Q_IMPORT_PLUGIN;Q_PROPERTY;Q_UNUSED;QTEST_KDEMAIN;QTEST_MAIN'''
| |
| | |
| *coding-style-check-Pedantic.sh
| |
| *astyle cannot (yet) do it
| |
| | |
| Download the scripts: [[Media:Pedantic.tar.gz]]
| |
| | |
| ===Public===
| |
| | |
| We prefer having a space before the keyword public at the definition of a new class:
| |
| {{Output|1=class DbException : public Akonadi::Exception
| |
| {
| |
| ...
| |
| };}}
| |
| | |
| *coding-style-check-Public.sh
| |
| *coding-style-change-Public.sh
| |
| *astyle cannot (yet) do it
| |
| | |
| Download the scripts: [[Media:Public.tar.gz]]
| |
| | |
| ===#include directive===
| |
| | |
| Refer to http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Qt_Includes
| |
| | |
| We prefer no space at the beginning of the directive. Some (not many) files need to be corrected to unify to all the other files.
| |
| | |
| {{Output|1=// some files use this
| |
| # include <A/b>
| |
| | |
| // we prefer, to unify the coding style
| |
| #include <A/b>}}
| |
| | |
| *coding-style-check-Space-Include.sh
| |
| | |
| Download the scripts: [[Media:Space-Include.tar.gz]]
| |
| | |
| ===enum without any type===
| |
| | |
| Instead of having an untyped enum such as:
| |
| {{Output|1= enum {
| |
| aElement= 123
| |
| }
| |
| }}
| |
| we prefer a #define directive:
| |
| {{Output|1=#define aElement 123
| |
| }}
| |
| | |
| Download the scripts: [[Media:Enum.tar.gz]]
| |
| | |
| ===enum without empty element===
| |
| | |
| The most compilers do not complain such a code:
| |
| | |
| {{Output|1= enum mytype {
| |
| aElement,
| |
| bElement,
| |
| }
| |
| }}
| |
| | |
| The last element is empty.
| |
| We prefer a "pedantic" code such as:
| |
| | |
| {{Output|1= enum mytype {
| |
| aElement,
| |
| bElement
| |
| }
| |
| }}
| |
| | |
| *coding-style-check-Enum-Pedantic.sh
| |
| | |
| The output of the '''check''' script is:
| |
| {{Output|1=check the file /home/guy-kde/Software/coding-style-check/enum-example.cpp
| |
| enum with ,} found at
| |
| 3-> bElement,
| |
| 4-> }
| |
| }}
| |
| | |
| Download the scripts: [[Media:Enum-Pedantic.tar.gz]]
| |
| | |
| === Some more space(s) ===
| |
| | |
| The declaration S *D; declares D as a pointer to the type determined by decl-specifier-seq S.
| |
| | |
| The most compilers do not make any difference for such lines of code:
| |
| | |
| {{Output|1=int *a;
| |
| int* b;
| |
| int * c}}
| |
| | |
| We prefer the first one, without a space beetwen the star and the name of the variable:
| |
| | |
| {{Output|1=int *a;}}
| |
| | |
| The same rule may be use for:
| |
| | |
| {{Output|1=myFunction(int &a, int& b, int & c)
| |
| {
| |
| // some lines
| |
| }
| |
| }}
| |
| | |
| We prefer:
| |
| {{Output|1=myFunction(int &a, int &b, int &c) }}
| |
| | |
| The awk-script checks also the occurences of:
| |
| * '''&,'''
| |
| * '''& >'''
| |
| * '''* >'''
| |
| * '''( )''' and '''( )''' ''empty function call''
| |
| * '''[ ''' and ''' ]''' ''index of an array''
| |
| * '''enum {''' ''untyped enum''
| |
| | |
| Not all the ouputs are real errors. Some codings might be correct.
| |
| | |
| *coding-style-check-NO-Space.sh
| |
| *astyle --reference=name
| |
| Note that astyle makes also changes within the macros SIGNAL and SLOT, which aren't desired.
| |
| This can be corrected with a Qt-utility:
| |
| normalize ''filename''
| |
| | |
| Some lines with "type & name..." must be manually corrected.
| |
| | |
| Download the scripts: [[Media:NO-Space.tar.gz]]
| |
| | |
| The script gives informations about the found line(s).
| |
| | |
| === Member initialization in a class ===
| |
| | |
| This example shows the indentation we prefer:
| |
| | |
| {{Output|1=class myClass {
| |
| // some lines
| |
| public:
| |
| myClass( int i )
| |
| : r(a)
| |
| , b(i)
| |
| , i(i)
| |
| , j(this->i)
| |
| {
| |
| // more lines
| |
| }
| |
| }}
| |
| | |
| *coding-style-check-Default.sh
| |
| *astyle cannot (yet) do it
| |
| | |
| Download the scripts: [[Media:Default.tar.gz]]
| |
| | |
| But the script produces a false alarm with the code:
| |
| {{Output| 1=if (''<expression>'') ? a : b;
| |
| }}
| |
| | |
| The changes must be done manualy
| |
| | |
| === Parenthesis ===
| |
| | |
| We prefer function definition and function call with no space after the opening brace and before the closing brace.
| |
| | |
| *coding-style-check-Parenthesis.sh
| |
| *astyle
| |
| | |
| Download the scripts: [[Media:Parenthesis.tar.gz]]
| |
| | |
| === Switch statement ===
| |
| | |
| This example shows the indentation we prefer:
| |
| | |
| {{Output|1=switch (a) {
| |
| case ''one'':
| |
| // some lines
| |
| break;
| |
| case ''two'':
| |
| // some lines
| |
| break;
| |
| default:
| |
| // some lines
| |
| break;
| |
| }
| |
| }}
| |
| | |
| *coding-style-check-Switch.sh
| |
| *astyle
| |
| | |
| Download the scripts: [[Media:Switch.tar.gz]]
| |
| | |
| === Use block braces ===
| |
| | |
| Even for block with only one statement, we prefer to use braces such as:
| |
| | |
| {{Output|1=if (''condition'') {
| |
| ''statement;''
| |
| }
| |
| }}
| |
| | |
| This should be used with the keywords '''if''', '''for''', '''while''' and '''FOREACH'''.
| |
| | |
| The output of the '''check''' script is:
| |
| | |
| {{Output|1=check the file /home/guy-kde/projects/test-if.cpp
| |
| 62: if without { at end of line: if ( collection.cachePolicyLocalParts() )
| |
| }}
| |
| | |
| *coding-style-check-If.sh
| |
| *astyle
| |
| | |
| Download the scripts: [[Media:If.tar.gz]]
| |
| | |
| But we get some false alarm with statement over more than one line:
| |
| | |
| {{Output|1=if ( ''condition_1''
| |
| && ''condition_2'' ) {
| |
| ''statement;''
| |
| }
| |
| }}
| |
| | |
| === Use one space after each keyword, but not after a cast ===
| |
| | |
| Refer to http://techbase.kde.org/Policies/Kdelibs_Coding_Style#Whitespace
| |
| | |
| For most of the keywords, it is not necessary to make a test. Because the sources have been already compiled. For example this code never appear in a source:
| |
| {{Output|1=inta;
| |
| floatb;}}
| |
| | |
| Some of the keywords are alone in the statement, such as '''break''' and '''continue'''. No test is necessary.
| |
| | |
| The only tests we have to do are the ones where a keyword is (or can be) followed
| |
| by a sign '''( { [ :'''
| |
| | |
| These are:
| |
| '''alignas decltype alignof noexcept typeid asm static_assert switch if catch while for sizeof new Q_FOREACH do try enum union Q_FOREVER bool char char16_t char32_t double float int long wchar_t signed unsigned short'''
| |
| | |
| For only '''one''' keyword:
| |
| *coding-style-check-SpaceAfterKeyword.sh
| |
| *coding-style-change-SpaceAfterKeyword.sh
| |
| | |
| For '''all''' keywords above:
| |
| *coding-style-check-SpaceAfter.sh
| |
| *coding-style-change-SpaceAfter.sh
| |
| | |
| Download the scripts: [[Media:SpaceAfter.tar.gz]]
| |
| | |
| The output of the '''check''' script is:
| |
| {{Output|1=check the file /home/guy-kde/projects/kdepimlibs/akonadi/contact/contactstreemodel.cpp
| |
| 98: if( at 10: if(contact.realName().isEmpty()) {
| |
| 99: if( at 12: if(contact.preferredEmail().isEmpty()) {
| |
| }}
| |
| | |
| The '''change''' script:
| |
| *puts a space after the keyword.
| |
| | |
| == Use all the scripts ==
| |
| | |
| All the scripts can be used with one only script.
| |
| | |
| Download the scripts: [[Media:All.tar.gz]]
| |
| | |
| == Check the objects and the libs ==
| |
| | |
| As a first approach, not any object may have binary change after applying one of the rules.
| |
| To check this, one uses the '''Md5sum-the-Objects.sh'''. Download the script: [[Media:Md5sum-the-Objects.sh.gz]] | |
| Same for the libs. Use the '''Md5sum-the-Libs.sh'''. Download the script: [[Media:Md5sum-the-Libs.sh.gz]]
| |
| | |
| The script can be used with one of the commands:
| |
| * save
| |
| * test
| |
| * clean
| |
| | |
| '''An example:'''
| |
| | |
| {{Input|1=cd <some_kdepim_directory>
| |
| mkdir build
| |
| cd build
| |
| ccmake ../
| |
| make}}
| |
| {{Output|1=<span style="color:Fuchsia">Scanning dependencies of target gpgmepp</span>
| |
| [ 0%] <span style="color:green">Building CXX object gpgme++/CMakeFiles/gpgmepp.dir/gpgmepp_automoc.cpp.o</span>
| |
| [ 0%] <span style="color:green">Building CXX object gpgme++/CMakeFiles/gpgmepp.dir/exception.cpp.o</span>
| |
| [ 0%] <span style="color:green">Building CXX object gpgme++/CMakeFiles/gpgmepp.dir/context.cpp.o</span>
| |
| ...}}
| |
| | |
| {{Input|1=Check-the-Objects.sh save}}
| |
| The script makes a copy of all the objects and a "time stamp":
| |
| {{Output|1=save the object ./kholidays/tests/CMakeFiles/testzodiac.dir/testzodiac.cpp.o
| |
| save the object ./kholidays/tests/CMakeFiles/testzodiac.dir/testzodiac_automoc.cpp.o
| |
| ...
| |
| all objects are saved}}
| |
| | |
| Now, one makes somes change(s) on the source(s) and:
| |
| | |
| {{Input| 1=make}}
| |
| | |
| Depending on the Makefile, some objects will be compiled again:
| |
| | |
| {{Output| 1=<span style="color:Fuchsia">Scanning dependencies of target akonadi-kde</span>
| |
| [ 17%] <span style="color:green">Building CXX object akonadi/CMakeFiles/akonadi-kde.dir/entitytreeview.cpp.o</span>
| |
| [ 17%] <span style="color:green">Building CXX object akonadi/CMakeFiles/akonadi-kde.dir/itemfetchjob.cpp.o</span>
| |
| [ 17%] <span style="color:green">Building CXX object akonadi/CMakeFiles/akonadi-kde.dir/statisticsproxymodel.cpp.o</span>
| |
| ...
| |
| <span style="color:Fuchsia">Scanning dependencies of target akonadi-kmime</span>
| |
| [ 56%] <span style="color:green">Building CXX object akonadi/kmime/CMakeFiles/akonadi-kmime.dir/standardmailactionmanager.cpp.o</span>}}
| |
| | |
| {{Input|1=Check-the-Objects.sh test}}
| |
| | |
| The script finds all the new objects, makes a comparision with the saved version:
| |
| {{Output|1=test the object ./akonadi/CMakeFiles/akonadi-kde.dir/statisticsproxymodel.cpp.o
| |
| test the object ./akonadi/CMakeFiles/akonadi-kde.dir/entitytreeview.cpp.o
| |
| test the object ./akonadi/CMakeFiles/akonadi-kde.dir/itemfetchjob.cpp.o
| |
| test the object ./akonadi/kmime/CMakeFiles/akonadi-kmime.dir/standardmailactionmanager.cpp.o
| |
| all tests are OK
| |
| }}
| |
| | |
| == Check the assembler files ==
| |
| If we add or remove some lines, the debug informations included in the object file will be change also.
| |
| | |
| This is the case with the test/change of "''Only single empty lines should be used''", "''First line, last line(s) may not be empty''" and some more test/change below (''adding some blocks'' with { and }).
| |
| | |
| For this reason it is no more possible to compare the objects.
| |
| We have to compare the assembler files.
| |
| This works pretty well for the version with '''CMAKE_BUILD_TYPE''' set to ''release''.
| |
| For the version with '''CMAKE_BUILD_TYPE''' set to ''debug'', we must remove all the debug informations before the comparision could take place.
| |
| | |
| === Generate the assembler files ===
| |
| | |
| To generate the assembler files, we only need to modify the ''build.make'' in every folder.
| |
| | |
| The script '''Prepare-build_make_files.sh''' works on the all directory, finds the line with the compiler command,
| |
| duplicates the line, add a ''-S option'' and changes the name of the output to ''somename.s''.
| |
| After a new ''make'' command, we can save all the assembler files with the script '''Check-the-assembler_code.sh'''.
| |
| Download the script: [[Media:Prepare-build_make_files.gz]]
| |
| | |
| === Remove the debug informations ===
| |
| | |
| The debug informations change with the changes of line numbers.
| |
| We drop all these debug informations before making the test.
| |
| | |
| The script to check the assembler files can be used in the same way as the one above (Check-the-Objects.sh).
| |
| To check this, one uses the '''Check-the-assembler_code.sh'''. Download the script: [[Media:Check-the-assembler_code.sh.gz]]
| |
| | |
| The script can be used with one of the commands:
| |
| * save
| |
| * test
| |
| * clean
| |
