mirror of
https://github.com/brl/mutter.git
synced 2024-11-29 19:40:43 -05:00
2b81d90dd7
The G_CONST_RETURN define in GLib is, and has always been, a bit fuzzy. We always used it to conform to the platform, at least for public-facing API. At first I assumed it has something to do with brain-damaged compilers or with weird platforms where const was not really supported; sadly, it's something much, much worse: it's a define that can be toggled at compile-time to remove const from the signature of public API. This is a truly terrifying feature that I assume was added in the past century, and whose inception clearly had something to do with massive doses of absynthe and opium — because any other explanation would make the existence of such a feature even worse than assuming drugs had anything to do with it. Anyway, and pleasing the gods, this dubious feature is being removed/deprecated in GLib; see bug: https://bugzilla.gnome.org/show_bug.cgi?id=644611 Before deprecation, though, we should just remove its usage from the whole API. We should especially remove its usage from Cally's internals, since there it never made sense in the first place.
614 lines
16 KiB
Plaintext
614 lines
16 KiB
Plaintext
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.
|