Merge pull request #116 from ascent12/contributing
Updated CONTRIBUTING.md
This commit is contained in:
commit
0c48ef5ad8
294
CONTRIBUTING.md
294
CONTRIBUTING.md
|
@ -1,9 +1,9 @@
|
|||
# Contributing to wlroots
|
||||
|
||||
Contributing just involves sending a pull request. You will probably be more
|
||||
successful with your contribution if you visit the [IRC
|
||||
channel](http://webchat.freenode.net/?channels=sway-devel&uio=d4) upfront and discuss
|
||||
your plans.
|
||||
successful with your contribution if you visit
|
||||
[#sway-devel](https://webchat.freenode.net/?channels=sway-devel) on
|
||||
irc.freenode.net upfront and discuss your plans.
|
||||
|
||||
## Pull Requests
|
||||
|
||||
|
@ -11,35 +11,36 @@ If you already have your own pull request habits, feel free to use them. If you
|
|||
don't, however, allow me to make a suggestion: feature branches pulled from
|
||||
upstream. Try this:
|
||||
|
||||
1. Fork sway
|
||||
2. Clone your fork
|
||||
3. git remote add upstream https://github.com/SirCmpwn/wlroots
|
||||
1. Fork wlroots
|
||||
2. `git clone https://github.com/username/wlroots && cd wlroots`
|
||||
3. `git remote add upstream https://github.com/SirCmpwn/wlroots`
|
||||
|
||||
You only need to do this once. You're never going to use your fork's master
|
||||
branch. Instead, when you start working on a feature, do this:
|
||||
|
||||
1. git fetch upstream
|
||||
2. git checkout -b add-so-and-so-feature upstream/master
|
||||
3. work
|
||||
4. git push -u origin add-so-and-so-feature
|
||||
5. Make pull request from your feature branch
|
||||
1. `git fetch upstream`
|
||||
2. `git checkout -b add-so-and-so-feature upstream/master`
|
||||
3. Add and commit your changes
|
||||
4. `git push -u origin add-so-and-so-feature`
|
||||
5. Make a pull request from your feature branch
|
||||
|
||||
## Commit Messages
|
||||
|
||||
Please strive to write good commit messages. Here's some guidelines to follow:
|
||||
|
||||
The first line should be limited to 50 characters and should be a sentence that
|
||||
completes the thought [When applied, this commit will...] "Implement cmd_move"
|
||||
or "Fix #742" or "Improve performance of arrange_windows on ARM" or similar.
|
||||
completes the thought [When applied, this commit will...] *"Implement
|
||||
cmd_move"* or *"Fix #742"* or *"Improve performance of arrange_windows on ARM"*
|
||||
or similar.
|
||||
|
||||
The subsequent lines should be seperated from the subject line by a single
|
||||
The subsequent lines should be separated from the subject line by a single
|
||||
blank line, and include optional details. In this you can give justification
|
||||
for the change, [reference Github
|
||||
issues](https://help.github.com/articles/closing-issues-via-commit-messages/),
|
||||
or explain some of the subtler details of your patch. This is important because
|
||||
when someone finds a line of code they don't understand later, they can use the
|
||||
`git blame` command to find out what the author was thinking when they wrote
|
||||
it. It's also easier to review your pull requests if they're seperated into
|
||||
it. It's also easier to review your pull requests if they're separated into
|
||||
logical commits that have good commit messages and justify themselves in the
|
||||
extended commit description.
|
||||
|
||||
|
@ -47,158 +48,153 @@ As a good rule of thumb, anything you might put into the pull request
|
|||
description on Github is probably fair game for going into the extended commit
|
||||
message as well.
|
||||
|
||||
See [here](https://chris.beams.io/posts/git-commit/) for more details.
|
||||
|
||||
## Coding Style
|
||||
|
||||
wlroots is written in C with the same style as Sway. The style guidelines is
|
||||
[kernel
|
||||
wlroots is written in C with a style similar to the [kernel
|
||||
style](https://www.kernel.org/doc/Documentation/process/coding-style.rst), but
|
||||
all braces go on the same line (*"but K&R says so!" is a silly way of justifying
|
||||
something*). Some points to note:
|
||||
with a few notable differences.
|
||||
|
||||
* Do not use typedefs unless you have a good reason
|
||||
* Do not use macros unless you have a *really* good reason
|
||||
* Align `case` with `switch`
|
||||
* Tabs, not spaces
|
||||
* `char *pointer` - note position of `*`
|
||||
* Use logging with reckless abandon
|
||||
* Always include braces for if/for/while/etc, even for one-liners
|
||||
Try to keep your code conforming to C11 and POSIX as much as possible, and do
|
||||
not use GNU extensions.
|
||||
|
||||
An example of well formatted code:
|
||||
### Brackets
|
||||
|
||||
```C
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include "log.h"
|
||||
#include "example.h"
|
||||
|
||||
struct foobar {
|
||||
char *foo;
|
||||
int bar;
|
||||
long baz;
|
||||
}; // Do not typedef without a good reason
|
||||
|
||||
int main(int argc, const char **argv) {
|
||||
if (argc != 4) {
|
||||
sway_abort("Do not run this program manually. See man 5 sway and look for output options.");
|
||||
Brackets always go on the same line, including in functions.
|
||||
Always include brackets for if/while/for, even if it's a single statement.
|
||||
```c
|
||||
void function() {
|
||||
if (condition1) {
|
||||
do_thing1();
|
||||
}
|
||||
|
||||
if (!registry->desktop_shell) {
|
||||
sway_abort("swaybg requires the compositor to support the desktop-shell extension.");
|
||||
}
|
||||
|
||||
int desired_output = atoi(argv[1]);
|
||||
sway_log(L_INFO, "Using output %d of %d", desired_output, registry->outputs->length);
|
||||
int i;
|
||||
struct output_state *output = registry->outputs->items[desired_output];
|
||||
struct window *window = window_setup(registry, 100, 100, false);
|
||||
if (!window) {
|
||||
sway_abort("Failed to create surfaces.");
|
||||
}
|
||||
window->width = output->width;
|
||||
window->height = output->height;
|
||||
desktop_shell_set_background(registry->desktop_shell, output->output, window->surface);
|
||||
list_add(surfaces, window);
|
||||
|
||||
cairo_surface_t *image = cairo_image_surface_create_from_png(argv[2]);
|
||||
double width = cairo_image_surface_get_width(image);
|
||||
double height = cairo_image_surface_get_height(image);
|
||||
|
||||
const char *scaling_mode_str = argv[3];
|
||||
enum scaling_mode scaling_mode;
|
||||
if (strcmp(scaling_mode_str, "stretch") == 0) {
|
||||
scaling_mode = SCALING_MODE_STRETCH;
|
||||
} else if (strcmp(scaling_mode_str, "fill") == 0) {
|
||||
scaling_mode = SCALING_MODE_FILL;
|
||||
} else if (strcmp(scaling_mode_str, "fit") == 0) {
|
||||
scaling_mode = SCALING_MODE_FIT;
|
||||
} else if (strcmp(scaling_mode_str, "center") == 0) {
|
||||
scaling_mode = SCALING_MODE_CENTER;
|
||||
} else if (strcmp(scaling_mode_str, "tile") == 0) {
|
||||
scaling_mode = SCALING_MODE_TILE;
|
||||
if (condition2) {
|
||||
do_thing2();
|
||||
} else {
|
||||
sway_abort("Unsupported scaling mode: %s", scaling_mode_str);
|
||||
do_thing3();
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
for (i = 0; i < surfaces->length; ++i) {
|
||||
struct window *window = surfaces->items[i];
|
||||
if (window_prerender(window) && window->cairo) {
|
||||
switch (scaling_mode) {
|
||||
case SCALING_MODE_STRETCH:
|
||||
cairo_scale(window->cairo,
|
||||
(double) window->width / width,
|
||||
(double) window->height / height);
|
||||
cairo_set_source_surface(window->cairo, image, 0, 0);
|
||||
break;
|
||||
case SCALING_MODE_FILL:
|
||||
{
|
||||
double window_ratio = (double) window->width / window->height;
|
||||
double bg_ratio = width / height;
|
||||
### Indentation
|
||||
|
||||
if (window_ratio > bg_ratio) {
|
||||
double scale = (double) window->width / width;
|
||||
cairo_scale(window->cairo, scale, scale);
|
||||
cairo_set_source_surface(window->cairo, image,
|
||||
0,
|
||||
(double) window->height/2 / scale - height/2);
|
||||
} else {
|
||||
double scale = (double) window->height / height;
|
||||
cairo_scale(window->cairo, scale, scale);
|
||||
cairo_set_source_surface(window->cairo, image,
|
||||
(double) window->width/2 / scale - width/2,
|
||||
0);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case SCALING_MODE_FIT:
|
||||
{
|
||||
double window_ratio = (double) window->width / window->height;
|
||||
double bg_ratio = width / height;
|
||||
Indentations are a single tab.
|
||||
|
||||
if (window_ratio > bg_ratio) {
|
||||
double scale = (double) window->height / height;
|
||||
cairo_scale(window->cairo, scale, scale);
|
||||
cairo_set_source_surface(window->cairo, image,
|
||||
(double) window->width/2 / scale - width/2,
|
||||
0);
|
||||
} else {
|
||||
double scale = (double) window->width / width;
|
||||
cairo_scale(window->cairo, scale, scale);
|
||||
cairo_set_source_surface(window->cairo, image,
|
||||
0,
|
||||
(double) window->height/2 / scale - height/2);
|
||||
}
|
||||
break;
|
||||
}
|
||||
case SCALING_MODE_CENTER:
|
||||
cairo_set_source_surface(window->cairo, image,
|
||||
(double) window->width/2 - width/2,
|
||||
(double) window->height/2 - height/2);
|
||||
break;
|
||||
case SCALING_MODE_TILE:
|
||||
{
|
||||
cairo_pattern_t *pattern = cairo_pattern_create_for_surface(image);
|
||||
cairo_pattern_set_extend(pattern, CAIRO_EXTEND_REPEAT);
|
||||
cairo_set_source(window->cairo, pattern);
|
||||
break;
|
||||
}
|
||||
default:
|
||||
sway_abort("Scaling mode '%s' not implemented yet!", scaling_mode_str);
|
||||
}
|
||||
For long lines that need to be broken, the continuation line should be indented
|
||||
with an additional tab.
|
||||
If the line being broken is opening a new block (functions, if, while, etc.),
|
||||
the continuation line should be indented with two tabs, so they can't be
|
||||
misread as being part of the block.
|
||||
```c
|
||||
really_long_function(argument1, argument2, ...,
|
||||
argument3, argument4);
|
||||
|
||||
cairo_paint(window->cairo);
|
||||
if (condition1 && condition2 && ...
|
||||
condition3 && condition4) {
|
||||
do_thing();
|
||||
}
|
||||
```
|
||||
|
||||
window_render(window);
|
||||
Try to break the line in the place which you think is the most appropriate.
|
||||
|
||||
|
||||
### Line Length
|
||||
|
||||
Try to keep your lines under 80 columns, but you can go up to 100 if it
|
||||
improves readability.
|
||||
|
||||
### Names
|
||||
|
||||
Function and type names should be prefixed with `wlr_submodule_` (e.g. `struct
|
||||
wlr_drm_plane`, `wlr_output_set_cursor`). For static functions and types local
|
||||
to a file, the names chosen aren't as important.
|
||||
|
||||
For include guards, use the header's filename relative to include. Uppercase
|
||||
all of the characters, and replace any invalid characters with an underscore.
|
||||
|
||||
### Construction/Destruction Functions
|
||||
|
||||
For functions that are responsible for constructing and destructing an object,
|
||||
they should be written as a pair of one of two forms:
|
||||
* `init`/`finish`: These initialize/deinitialize a type, but are **NOT**
|
||||
responsible for allocating it. They should accept a pointer to some
|
||||
pre-allocated memory (e.g. a member of a struct).
|
||||
* `create`/`destroy`: These also initialize/deinitialize, but will return a
|
||||
pointer to a `malloc`ed chunk of memory, and will `free` it in `destroy`.
|
||||
|
||||
A destruction function should always be able to accept a NULL pointer or a
|
||||
zeroed value and exit cleanly; this simplifies error handling a lot.
|
||||
|
||||
### Error Codes
|
||||
|
||||
For functions not returning a value, they should return a (stdbool.h) bool to
|
||||
indicated if they succeeded or not.
|
||||
|
||||
### Macros
|
||||
|
||||
Try to keep the use of macros to a minimum, especially if a function can do the
|
||||
job. If you do need to use them, try to keep them close to where they're being
|
||||
used and `#undef` them after.
|
||||
|
||||
## Example
|
||||
|
||||
```c
|
||||
struct wlr_backend *wlr_backend_autocreate(struct wl_display *display) {
|
||||
struct wlr_backend *backend;
|
||||
if (getenv("WAYLAND_DISPLAY") || getenv("_WAYLAND_DISPLAY")) {
|
||||
backend = attempt_wl_backend(display);
|
||||
if (backend) {
|
||||
return backend;
|
||||
}
|
||||
}
|
||||
|
||||
while (wl_display_dispatch(registry->display) != -1);
|
||||
|
||||
for (i = 0; i < surfaces->length; ++i) {
|
||||
struct window *window = surfaces->items[i];
|
||||
window_teardown(window);
|
||||
const char *x11_display = getenv("DISPLAY");
|
||||
if (x11_display) {
|
||||
return wlr_x11_backend_create(display, x11_display);
|
||||
}
|
||||
list_free(surfaces);
|
||||
registry_teardown(registry);
|
||||
return 0;
|
||||
|
||||
// Attempt DRM+libinput
|
||||
|
||||
struct wlr_session *session = wlr_session_create(display);
|
||||
if (!session) {
|
||||
wlr_log(L_ERROR, "Failed to start a DRM session");
|
||||
return NULL;
|
||||
}
|
||||
|
||||
int gpu = wlr_session_find_gpu(session);
|
||||
if (gpu == -1) {
|
||||
wlr_log(L_ERROR, "Failed to open DRM device");
|
||||
goto error_session;
|
||||
}
|
||||
|
||||
backend = wlr_multi_backend_create(session);
|
||||
if (!backend) {
|
||||
goto error_gpu;
|
||||
}
|
||||
|
||||
struct wlr_backend *libinput = wlr_libinput_backend_create(display, session);
|
||||
if (!libinput) {
|
||||
goto error_multi;
|
||||
}
|
||||
|
||||
struct wlr_backend *drm = wlr_drm_backend_create(display, session, gpu);
|
||||
if (!drm) {
|
||||
goto error_libinput;
|
||||
}
|
||||
|
||||
wlr_multi_backend_add(backend, libinput);
|
||||
wlr_multi_backend_add(backend, drm);
|
||||
return backend;
|
||||
|
||||
error_libinput:
|
||||
wlr_backend_destroy(libinput);
|
||||
error_multi:
|
||||
wlr_backend_destroy(backend);
|
||||
error_gpu:
|
||||
wlr_session_close_file(session, gpu);
|
||||
error_session:
|
||||
wlr_session_destroy(session);
|
||||
return NULL;
|
||||
}
|
||||
```
|
||||
|
|
|
@ -7,9 +7,7 @@ This is a WIP: [status](https://github.com/SirCmpwn/wlroots/issues/9)
|
|||
|
||||
## Contributing
|
||||
|
||||
Development is organized in our [IRC
|
||||
channel](http://webchat.freenode.net/?channels=sway-devel&uio=d4), #sway-devel on
|
||||
irc.freenode.net. Join us and ask how you can help!
|
||||
See [CONTRIBUTING.md](https://github.com/SirCmpwn/wlroots/blob/master/CONTRIBUTING.md)
|
||||
|
||||
## Building
|
||||
|
||||
|
|
Loading…
Reference in New Issue