# Style Guide

We mostly use the [GTK style
guide](https://gitlab.gnome.org/GNOME/gtk/-/blob/main/docs/CODING-STYLE.md)
by default. Some exceptions are listed below.

We also target the same [toolchain
requirements](https://github.com/GNOME/glib/blob/main/docs/toolchain-requirements.md)
as glib.

## General Style hints
* Overally, we prefer code legibility over brevity or cleverness.
* Declare variables at the top of the scope, in order they're
  used. Mid-function declarations are discouraged.
* Memory Management: Always use `g_autofree` and `g_autoptr` where possible. Do one per
  line, and make sure they're initialized to NULL. static-scan will complain
  if they aren't.
  ```C
  g_autofree gchar *str = NULL; // This is fine
  g_autofree gchar *str2; // Not initialized to NULL. This is wrong
  ```
    * Similarly, use `g_steal_pointer()` and friends. They make the
      lifecycle of variables more understandable.
    * Use `g_set_object()` in property setters to handle reference
      counting and NULL checks concisely.
    * Use `g_clear_object()` and `g_clear_pointer()` in
      dispose/finalize methods, and in general. They make the code
      clearer and lessen the chances of accessing a freed pointer.
* Include the minimum number of header files, and list them in
  alphabetical order.
* Vertically align parameters in forward declarations and function definitions to
  improve scannability:
  ```C
  static void edit_widget_get_property (GObject    *object,
                                        guint       prop_id,
                                        GValue     *value,
                                        GParamSpec *pspec);
  ```
* Use spaces instead of tabs. We use 2 spaces for indentation. Run
  this command to check for any occurances of tabs:
  ```bash
  $ grep -r $'\t'
  ```
* Use C-style comments for explanitary notes. `/* . . . */` instead of
  C++-style `//`. The latter are reserved for transient issues.
   * Example: If you have a debugging print-statement that's often
     used, feel free to use `//`. Ex:
     ```C
       //puzzle_set_model_print (self);
     ```
   * Example 2: For things that are broken and need to be fixed, we
     sometimes temporarily will use `//` as well.
* We follow the GTK brace convention with if statements, though are a
  bit more liberal in using braces:
  ```C
  if (condition)
    do_one_thing ();

  if (condition)
    do_one_thing ();
  else
    do_another_thing ();

  if (condition)
    {
      do_one_thing ();
      do_another_thing ();
    }
  else
    {
      do_a_third_thing (); // Different than GTK convention
    }
  ```
  That last example is a little different than the GTK style-guide,
  which might omit the last braces.

## Naming Conventions
* **Functions and variables**: Use `snake_case`.
* **Types**: Use `PascalCase` (e.g., `PlayWindow`, `GridState`).
* **CSS classes**: Use `kebab-case`.
* **Callbacks**: Always use `verb_cb` suffix (e.g., `button_clicked_cb`).
* **Internal functions**: Functions that are shared across files but
  not part of the public API should be prefixed with an underscore
  (e.g., `_play_window_update()`). These are often declared in a
  `-private.h` header.

## Error Handling
* Use `g_return_if_fail()` and `g_return_val_if_fail()` at the
  beginning of public functions to validate arguments.
* Use `g_assert()` for internal consistency checks that should never
  fail if the code is correct.
* Use `g_warning()` or `g_critical()` for unexpected runtime errors
  that aren't fatal.
* Don't keep any `g_debug()` calls in the code, unless it's not
  compiled normally.

## UI Development
* We use
  [Blueprint](https://gitlab.gnome.org/GNOME/blueprint-compiler/)
  for UI definitions. These are the files ending in `.blp`.
* Widget IDs in Blueprint should use `snake_case` to match the C
  variables they are bound to.
* Complex widgets should use UI templates. Bind children and callbacks
  in the `class_init` function using
  `gtk_widget_class_bind_template_child()` and
  `gtk_widget_class_bind_template_callback()`.

## Private Headers
If a widget or class has internal state or methods that need to be
shared across multiple source files (e.g., `play-window.c` and
`play-window-actions.c`), use a private header file named
`filename-private.h`. This helps keep the public header clean and
encapsulates internal implementation details.

## Stateless Model
Much of the editor logic and the `PlayGrid` use a stateless
model. Actions on the grid state should not modify the existing state
object but instead return a new state object (often a clone with
modifications).
* Functions that return a new state should be marked with
  `G_GNUC_WARN_UNUSED_RESULT`.
* The `GridState` struct is a plain C struct (not a GObject) and
  should be managed with `grid_state_new()`, `grid_state_clone()`, and
  `grid_state_free()`.

## FIXME and TODO messages

If you're going to leave a FIXME in the code, please put the _domain_
in parens. That make it easy to grep for all common areas that may
need fixing. As an example:

```C
/* FIXME(css): This color is hardcoded instead of css based. See bug #xxx */
```

We don't use TODO or have a well maintained TODO.md file. Please file
bugs for issues.

Common FIXME domains:
* *error*: an unhandled error, that probably needs a better message to
  the user than a g_warning.
* *refactor*: clean up an egregiously messy area of code
* *optimization*: a chunk of code is unnecessarily slow/big
* *css*: Something that should be moved to be a CSS setting
* *gtk/adwaita/libfoo*: Work around for an issue in a
  dependency. Often includes the version number it was fixed or
  introduced.
* *magichars*: A hardcoded constant in the code, that should be
  centralized or made configurable
* *mystery*: Something we don't understand in the code and couldn't
  figure out.

## File order

Within a file, we use the following section order. Keep two newlines
between each section, and one newline within each section:

* License
* Includes
* Local #defines and enums.
* Global tables (e.g. `obj_props`, signal IDs)
* Object structs
* Forward declarations (vertically aligned, as mentioned above)
* G_DEFINE_TYPE()
* Object `_init()` and `_class_init()`
* Parent overrides (`get_property`, `set_property`, `dispose`, `snapshot`, etc)
    * These should be in the order declared in `_class_init()` and be
      in order from the furthest parent in. (eg, object → widget → ...).
* Private methods
* Public methods
* Global helper functions

Note, this is mostly aspirational, and has settled over time. Very few
files follow this to the letter. We fix them as we see them.

## Trailing whitespace

We kill all whitespace after any line. To check if the changes in your
current branch have any trailing whitespace, run this command:
```bash
$ git diff --check main
```
If your branch isn't based on `main`, then replace `main` with
the actual base branch or commit.

To check for all trailing whitespace in the codebase (not just
those from your current branch), run this command:
```bash
# Do a diff check against the first ever commit,
# and exclude any dirs that we expect to contain trailing whitespace.

# NOTE: You must run this command in the root crosswords directory,
# in order for the exclusions to work.

$ git diff --check 460554e5 -- ':(exclude)word-lists/' ':(exclude)puzzle-sets/'
```

Please include a trailing newline at the end of the file.