# 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:
  ```C
  // Don't do this:
  ...
  call_func ();
  {
    gint new_var;
    call_func (&new_var);
    g_print ("new_var: %d\n", new_var);
  }
  ...
  ```

* 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 → ...).
* Callbacks. They should be named `objid_action_cb()` (ex `clear_button_clicked_cb()`)
* Private methods
* Public methods
* Global helper functions
* Test code. Some files have their tests built into the source file
  instead of externally. This should go at the very end.

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.
