diff options
Diffstat (limited to 'doc/CODING_STYLE.txt')
-rw-r--r-- | doc/CODING_STYLE.txt | 613 |
1 files changed, 613 insertions, 0 deletions
diff --git a/doc/CODING_STYLE.txt b/doc/CODING_STYLE.txt new file mode 100644 index 000000000..b8682c82b --- /dev/null +++ b/doc/CODING_STYLE.txt @@ -0,0 +1,613 @@ +Clutter Coding Style +------------------------------------------------------------------------------- + +This document is intended to be a short description of the preferred +coding style to be used for the Clutter source code. + +Coding style is a matter of consistency, readability and maintainance; +coding style is also completely arbitrary and a matter of taste. This +document will use examples at the very least to provide authoritative +and consistent answers to common questions regarding the coding style, +and will also try to identify the allowed exceptions. + +The examples will show the preferred coding style; the negative examples +will be clearly identified. Please, don't submit code to Clutter that +looks like any of these. + +Part of the rationales for these coding style rules are available either +in the kernel CodingStyle document or in Cairo's CODING_STYLE one. + +When in doubt, check the surrounding code and try to imitate it. + +Clutter provides an Uncrustify configuration file that tries to match +this document. Since automated tools are not a substitute for human eye, +they should not be entirely relied upon - but they can give an initial +layout for contributors. + ++ Line width + +The maximum line width for source files is 80 characters, whenever possible. +Longer lines are usually an indication that you either need a function +or a pre-processor macro. + ++ Indentation + +Each new level is indented 2 or more spaces than the previous level: + + if (condition) + single_statement (); + +This can only be achieved using space characters. It may not be achieved +using tab characters alone, or using a combination of spaces and tabs. + +Do not change the editor's configuration to change the meaning of a +tab character (see below); code using tabs to indent will not be accepted +into Clutter. + +Even if two spaces for each indentation level allows deeper nesting than +8 spaces, Clutter favours self-documenting function names that can take +quite some space. For this reason you should avoid deeply nested code. + ++ Tab characters + +The tab character must always be expanded to spaces. If a literal +tab must be used inside the source, the tab must always be interpreted +according to its traditional meaning: + + Advance to the next column which is a multiple of 8. + [ these two lines should be aligned ] + ++ Braces + +Curly braces should not be used for single statement blocks: + + if (condition) + single_statement (); + else + another_single_statement (arg1); + +In case of multiple statements, curly braces should be put on another +indentation level: + + if (condition) + { + statement_1 (); + statement_2 (); + statement_3 (); + } + +The "no block for single statements" rule has only three exceptions: + + ① if the single statement covers multiple lines, e.g. for functions with + many arguments, and it is followed by else or else if: + + /* valid */ + if (condition) + { + a_single_statement_with_many_arguments (some_lengthy_argument, + another_lengthy_argument, + and_another_one, + plus_one); + } + else + another_single_statement (arg1, arg2); + + ② if the condition is composed of many lines: + + /* valid */ + if (condition1 || + (condition2 && condition3) || + condition4 || + (condition5 && (condition6 || condition7))) + { + a_single_statement (); + } + + ③ Nested if's, in which case the block should be placed on the + outermost if: + + /* valid */ + if (condition) + { + if (another_condition) + single_statement (); + else + another_single_statement (); + } + + /* invalid */ + if (condition) + if (another_condition) + single_statement (); + else if (yet_another_condition) + another_single_statement (); + +In general, new blocks should be placed on a new indentation level, +like: + + int retval = 0; + + statement_1 (); + statement_2 (); + + { + int var1 = 42; + gboolean res = FALSE; + + res = statement_3 (var1); + + retval = res ? -1 : 1; + } + +While curly braces for function definitions should rest on a new line +they should not add an indentation level: + + /* valid */ + static void + my_function (int argument) + { + do_my_things (); + } + + /* invalid */ + static void + my_function (int argument) { + do_my_things (); + } + + /* invalid */ + static void + my_function (int argument) + { + do_my_things (); + } + +Curly braces must not be placed on the same line as a condition: + + /* invalid */ + if (condition) { + statement_1 (); + statement_2 (); + } + ++ Conditions + +Do not check boolean values for equality: + + /* invalid */ + if (condition == TRUE) + do_foo (); + + /* valid */ + if (another_condition) + do_bar (); + +Even if C handles NULL equality like a boolean, be explicit: + + /* valid */ + if (some_pointer == NULL) + do_blah (); + + /* invalid */ + if (some_other_pointer) + do_blurp (); + +In case of conditions split over multiple lines, the logical operators should +always go at the end of the line: + + /* invalid */ + if (condition1 + || condition2 + || condition3) + { + do_foo (); + } + + /* valid */ + if (condition1 && + condition2 && + (condition3 || (condition4 && condition5))) + { + do_blah (); + } + ++ Functions + +Functions should be declared by placing the returned value on a separate +line from the function name: + + void + my_function (void) + { + } + +The arguments list must be broken into a new line for each argument, +with the argument names right aligned, taking into account pointers: + + void + my_function (some_type_t type, + another_type_t *a_pointer, + final_type_t another_type) + { + } + +The alignment also holds when invoking a function without breaking the +80 characters limit: + + align_function_arguments (first_argument, + second_argument, + third_argument); + +To respect the 80 characters limit do not break the function name from +the arguments: + + /* invalid */ + a_very_long_function_name_with_long_parameters + (argument_the_first, argument_the_second); + + /* valid */ + first_a = argument_the_first; + second_a = argument_the_second; + a_very_long_function_name_with_long_parameters (first_a, second_a); + ++ Whitespace + +Always put a space before a parenthesis but never after: + + /* valid */ + if (condition) + do_my_things (); + + /* valid */ + switch (condition) + { + } + + /* invalid */ + if(condition) + do_my_things(); + + /* invalid */ + if ( condition ) + do_my_things ( ); + +A switch() should open a block on a new indentation level, and each case +should start on the same indentation level as the curly braces, with the +case block on a new indentation level: + + /* valid */ + switch (condition) + { + case FOO: + do_foo (); + break; + + case BAR: + do_bar (); + break; + } + + /* invalid */ + switch (condition) { + case FOO: do_foo (); break; + case BAR: do_bar (); break; + } + + /* invalid */ + switch (condition) + { + case FOO: do_foo (); + break; + case BAR: do_bar (); + break; + } + + /* invalid */ + switch (condition) + { + case FOO: + do_foo (); + break; + case BAR: + do_bar (); + break; + } + +It is preferable, though not mandatory, to separate the various cases with +a newline: + + switch (condition) + { + case FOO: + do_foo (); + break; + + case BAR: + do_bar (); + break; + + default: + do_default (); + } + +The 'break' statement for the default: case is not mandatory. + +If a case block needs to declare new variables, the same rules as the +inner blocks (see above) apply; the break statement should be placed +outside of the inner block: + + switch (condition) + { + case FOO: + { + int foo; + + foo = do_foo (); + } + break; + + ... + } + +When declaring a structure type use newlines to separate logical sections +of the structure: + + struct _ClutterActorPrivate + { + /* fixed position */ + ClutterUnit fixed_x; + ClutterUnit fixed_y; + + ClutterRequestMode request_mode; + + /* requisition sizes */ + ClutterUnit request_width_for_height; + ClutterUnit request_min_width; + ClutterUnit request_natural_width; + ClutterUnit request_height_for_width; + ClutterUnit request_min_height; + ClutterUnit request_natural_height; + + ClutterActorBox allocation; + + ... + }; + +Do not eliminate whitespace and newlines just because something would +fit on 80 characters: + + /* invalid */ + if (condition) foo (); else bar (); + +Do eliminate trailing whitespace on any line, preferably as a separate +patch or commit. Never use empty lines at the beginning or at the end of +a file. + +Do enable the default git pre-commit hook that detect trailing +whitespace for you and help you to avoid corrupting Clutter's tree with +it. Do that as follows: + + chmod a+x .git/hooks/pre-commit + +You might also find the git-stripspace utility helpful which acts as a +filter to remove trailing whitespace as well as initial, final, and +duplicate blank lines. + ++ Headers + +Headers are special, for Clutter, in that they don't have to obey the +80 characters limit. The only major rule for headers is that the functions +definition should be vertically aligned in three columns: + + return value function_name (type argument, + type argument, + type argument); + +The maximum width of each column is given by the longest element in the +column: + + void clutter_type_set_property (ClutterType *type, + const gchar *value, + GError **error); + const gchar *clutter_type_get_property (ClutterType *type); + +It is also possible to align the columns to the next tab: + + void clutter_type_set_prop (ClutterType *type, + gfloat value); + gfloat clutter_type_get_prop (ClutterType *type); + gint clutter_type_update_foobar (ClutterType *type); + +Public headers should never be included directly: + + #if !defined(__CLUTTER_H_INSIDE__) && !defined(CLUTTER_COMPILATION) + #error "Only <clutter/clutter.h> can be included directly." + #endif + +Public headers should also have inclusion guards (for internal usage) +and C++ guards: + + #ifndef __CLUTTER_HEADER_H__ + #define __CLUTTER_HEADER_H__ + + #include <clutter/clutter-actor.h> + + G_BEGIN_DECLS + + ... + + G_END_DECLS + + #endif /* __CLUTTER_HEADER_H__ */ + ++ Includes + +Clutter source files should never include the global clutter.h header, but +instead include the individual headers that are needed. Every file must +include config.h first, then its own header, then other Clutter headers +that it needs, then system and third-party headers that it needs. + + /* valid */ + #include "config.h" + + #include "clutter-foo.h" + + #include "clutter-actor.h" + #include "clutter-container.h" + + ... + + #include <string.h> + ++ GObject + +GObject classes definition and implementation require some additional +coding style notices. + +Typedef declarations should be placed at the beginning of the file: + + typedef struct _ClutterActor ClutterActor; + typedef struct _ClutterActorPrivate ClutterActorPrivate; + typedef struct _ClutterActorClass ClutterActorClass; + +This includes enumeration types: + + typedef enum { + CLUTTER_REQUEST_WIDTH_FOR_HEIGHT, + CLUTTER_REQUEST_HEIGHT_FOR_WIDTH + } ClutterRequestMode; + +And callback types: + + typedef void (* ClutterCallback) (ClutterActor *actor, + gpointer user_data); + +Instance structures should only contain the parent type and a pointer to a +private data structure, and they should be annotated as "private": + + struct _ClutterRectangle + { + /*< private >*/ + ClutterActor parent_instance; + + ClutterRectanglePrivate *priv; + }; + +All the properties should be stored inside the private data structure, which +is defined inside the source file - or, if needed, inside a private header +file; the private header filename must end with "-private.h" and must not be +installed. + +The private data structure should only be accessed internally using the +pointer inside the instance structure, and never using the +G_TYPE_INSTANCE_GET_PRIVATE() macro or the g_type_instance_get_private() +function. + +Always use the G_DEFINE_TYPE(), G_DEFINE_TYPE_WITH_CODE() macros, or +their abstract variants G_DEFINE_ABSTRACT_TYPE() and +G_DEFINE_ABSTRACT_TYPE_WITH_CODE(). + +Avoid forward declaration for functions: use the G_DEFINE_* macros right +after the private types, variables and macros declarations. + +Interface types should always have the dummy typedef for cast purposes: + + typedef struct _ClutterFoo ClutterFoo; + +The interface structure should have "Iface" postfixed to the dummy typedef: + + typedef struct _ClutterFooIface ClutterFooIface; + +Interfaces must have the following macros: + + - Macro: - Expands to: + • CLUTTER_TYPE_<iface_name> <iface_name>_get_type + • CLUTTER_<iface_name> G_TYPE_CHECK_INSTANCE_CAST + • CLUTTER_IS_<iface_name> G_TYPE_CHECK_INSTANCE_TYPE + • CLUTTER_<iface_name>_GET_IFACE G_TYPE_INSTANCE_GET_INTERFACE + ++ Memory allocation + +When dynamically allocating data on the heap either use g_new() or, +if allocating multiple small data structures, g_slice_new(). + +Public structure types should always be returned after being zero-ed, +either explicitly for each member, or by using g_new0() or g_slice_new0(). + ++ Macros + +Try to avoid private macros unless strictly necessary. Remember to #undef +them at the end of a block or a series of functions needing them. + +Inline functions are usually preferable to private macros. + +Public macros should not be used unless they evaluate to a constant. + ++ Public API + +Avoid exporting variables as public API, since this is cumbersome on some +platforms. It is always preferable to add getters and setters instead. + ++ Private API + +Non-exported functions that are needed in more than one source file +should be named "_clutter_...", and declared in a private header file. + +Underscore-prefixed functions are never exported. + +Non-exported functions that are only needed in one source file +should be declared static. + ++ Documentation + +All public APIs must have gtk-doc comments. For functions, these should +be placed in the source file, directly above the function. + + /* valid */ + /** + * clutter_get_flow: + * @actor: a #ClutterActor + * + * Gets the flow of an actor. + * + * Note that flows may be laminar or turbulent... + * + * Return value: (transfer none): the flow of @actor + */ + ClutterFlow * + clutter_get_flow (ClutterActor *actor) + { + ... + } + +Doc comments for macros, function types, class structs, etc should be +placed next to the definitions, typically in headers. + +Section introductions should be placed in the source file they describe, +after the license header: + + /* valid */ + /** + * SECTION:clutter-align-constraint + * @Title: ClutterAlignConstraint + * @Short_Description: A constraint aligning the position of an actor + * + * #ClutterAlignConstraint is a #ClutterConstraint that aligns the position + * of the #ClutterActor to which it is applied to the size of another + * #ClutterActor using an alignment factor + * + * [...] + */ + +To properly document a new function, macro, function type or struct, +it needs to be listed in the clutter-sections.txt file. + +To properly document a new class, it needs to be given its own section +in clutter-sections.txt, needs to be included in clutter-docs.xml, and the +get_type function needs to listed in clutter.types. + ++ Old code + +It is ok to update the style of a code block or function when you +are touching it anyway, but sweeping whitespace changes obscure the +git history and should be avoided. |