From e49aed8012b4e457d353e2ce1671520a45354e6e Mon Sep 17 00:00:00 2001 From: Scott Anderson Date: Sun, 20 Aug 2017 19:55:18 +1200 Subject: [PATCH 1/5] Updated CONTRIBUTING.md Changed the contribution guidelines to be more consistent with the style that wlroots has evolved into, and removed some duplicate information that already exists in the kernel style. --- CONTRIBUTING.md | 249 ++++++++++++++++++------------------------------ README.md | 4 +- 2 files changed, 93 insertions(+), 160 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0f54749b..c04694a9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,8 +2,7 @@ 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. +channel](irc://chat.freenode.net/sway-devel) upfront and discuss your plans. ## Pull Requests @@ -11,35 +10,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 +47,93 @@ 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 -#include -#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; - - 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; - - 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); - } - - cairo_paint(window->cairo); - - window_render(window); - } - } - - while (wl_display_dispatch(registry->display) != -1); - - for (i = 0; i < surfaces->length; ++i) { - struct window *window = surfaces->items[i]; - window_teardown(window); - } - list_free(surfaces); - registry_teardown(registry); - return 0; } ``` + +### Indentation + +Indentations are a single tab. + +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); + +if (condition1 && condition2 && ... + condition3 && condition4) { + do_thing(); +} +``` + +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. + +### 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 an +otherwise uninitialised 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. + +## Meson Coding Style + +The Meson style is similar to the C style, but indentations are 2 spaces. diff --git a/README.md b/README.md index 23aceab0..bc4f6471 100644 --- a/README.md +++ b/README.md @@ -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 From deb90ae59debd9774a06f599776177fe9e425172 Mon Sep 17 00:00:00 2001 From: Scott Anderson Date: Sun, 20 Aug 2017 20:35:43 +1200 Subject: [PATCH 2/5] Changed "uninitialised" to "zeroed" --- CONTRIBUTING.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c04694a9..f7899b73 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -119,9 +119,8 @@ 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 an -otherwise uninitialised value and exit cleanly; this simplifies error handling -a lot. +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 From b3dd1b5f957e33cc41bdd548e9dbc5f271a37f8b Mon Sep 17 00:00:00 2001 From: Scott Anderson Date: Mon, 21 Aug 2017 00:08:43 +1200 Subject: [PATCH 3/5] Changed IRC back to webchat link Github doesn't support irc:// links, for some stupid reason. --- CONTRIBUTING.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f7899b73..6187bd02 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,8 +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](irc://chat.freenode.net/sway-devel) upfront and discuss your plans. +successful with your contribution if you visit +[#sway-devel](https://webchat.freenode.net/?channels=sway-devel) on freenode +upfront and discuss your plans. ## Pull Requests From bee3560f137f814f2fc8a1e8035bd867a07c6e55 Mon Sep 17 00:00:00 2001 From: Scott Anderson Date: Sat, 30 Sep 2017 15:15:43 +1300 Subject: [PATCH 4/5] Added example --- CONTRIBUTING.md | 65 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 63 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 6187bd02..cfdc0765 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -110,6 +110,9 @@ 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, @@ -134,6 +137,64 @@ 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. -## Meson Coding Style +## Example -The Meson style is similar to the C style, but indentations are 2 spaces. +```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; + } + } + + const char *x11_display = getenv("DISPLAY"); + if (x11_display) { + return wlr_x11_backend_create(display, x11_display); + } + + // 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; +} +``` From 8e45d4beb7d50afe280eeed28a4eef1c87a0b527 Mon Sep 17 00:00:00 2001 From: Scott Anderson Date: Sat, 30 Sep 2017 15:18:40 +1300 Subject: [PATCH 5/5] Change to irc.freenode.net --- CONTRIBUTING.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index cfdc0765..242c8bd4 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,8 +2,8 @@ Contributing just involves sending a pull request. You will probably be more successful with your contribution if you visit -[#sway-devel](https://webchat.freenode.net/?channels=sway-devel) on freenode -upfront and discuss your plans. +[#sway-devel](https://webchat.freenode.net/?channels=sway-devel) on +irc.freenode.net upfront and discuss your plans. ## Pull Requests