diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0da08c892..c89492b32 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -2,15 +2,15 @@
- Note + Disclaimer - This document is generated by AI, and mainly for AI use. It is not a substitute for human review, and may contain errors or omissions. Please verify any information before relying on it. + This document is generated by AI and reviewed by humans. It is primarily intended for AI-assisted development, while remaining useful to human developers.
-Thanks for your interest in fastfetch. This document covers building, architecture, how to add a module or a logo, code style, commit conventions, and the pull request workflow. +Thank you for your interest in fastfetch. This document covers building, architecture, how to add a module or a logo, code style, commit conventions, and the pull request workflow. -fastfetch is a system information tool written in C23, supporting Linux, macOS, Windows, the BSDs, Solaris, Haiku and Android. The project is borderline obsessive about **startup time** and **optional dependencies** — many design decisions only make sense under that constraint, and this document keeps coming back to it. +fastfetch is a system information tool written in C23, supporting Linux, macOS, Windows, the BSDs, Solaris, Haiku and Android. The project places strong emphasis on **startup time** and on keeping **dependencies optional**; many design decisions only make sense under that constraint, and this document returns to it repeatedly. --- @@ -29,7 +29,7 @@ fastfetch is a system information tool written in C23, supporting Linux, macOS, - [Changelog](#changelog) - [Tests](#tests) - [Pull requests](#pull-requests) -- [Gotchas](#gotchas) +- [Pitfalls](#pitfalls) --- @@ -38,14 +38,14 @@ fastfetch is a system information tool written in C23, supporting Linux, macOS, | Task | Start here | Verify with | |---|---|---| | Build and run fastfetch | `run.sh`, `CMakeLists.txt` | `./run.sh` | -| Add a module | `src/modules//`, `src/detection//`, `src/modules/modules.c` | `cmake -B build && cmake --build build -j` | -| Add a platform implementation | `src/detection//_.c` | Build on the target platform or CI | +| Add a module | `src/modules//`, `src/detection//`, `src/modules/modules.c`, `CMakeLists.txt` | `cmake -B build && cmake --build build -j` | +| Add a platform implementation | `src/detection//_.c`, the matching platform block in `CMakeLists.txt` | Build on the target platform or CI | | Add an ASCII logo | `src/logo/ascii//.txt`, matching `.inc` | `./build/fastfetch --logo ` | | Change formatting or JSON output | `src/modules//.c` | `./build/fastfetch -s --format json` | | Change shared formatting or containers | `src/common/format.h`, `src/common/color.h`, `src/common/FFstrbuf.h` | Build and run the matching test | | Run the test suite | `tests/`, `build/` | `cd build && ctest --output-on-failure` | -For a normal code change, the shortest useful loop is: configure, build the `fastfetch` target, run the affected module, then run the tests. If you add a directory or a generated input, re-run `cmake -B build` so CMake refreshes its file globs. +For a typical code change, the shortest useful iteration is: configure, build the `fastfetch` target, run the affected module, then run the tests. If you add a directory or a generated input, re-run `cmake -B build` so that CMake refreshes its file globs. --- @@ -53,7 +53,7 @@ For a normal code change, the shortest useful loop is: configure, build the `fas ### Building -The quickest way is `run.sh` in the repository root — it creates `build/`, configures, compiles and runs the binary: +The quickest way is `run.sh` in the repository root: it creates `build/`, configures, compiles and runs the binary. ```sh ./run.sh # build and run @@ -68,7 +68,7 @@ cmake --build build --target fastfetch -j$(nproc) ./build/fastfetch ``` -The default build type is `RelWithDebInfo`. LTO is enabled whenever `ENABLE_LTO=ON` **and** `CMAKE_BUILD_TYPE != Debug` — so the default already has it, and only `Debug` builds lack it. This matters because LTO here is not merely an optimization: it is what strips the code of disabled modules. See [Gotchas](#gotchas). +The default build type is `RelWithDebInfo`. LTO is enabled whenever `ENABLE_LTO=ON` **and** `CMAKE_BUILD_TYPE != Debug`, so the default configuration enables it and only `Debug` builds omit it. This is significant because LTO here is not merely an optimization: it is what removes the code of disabled modules. See [Pitfalls](#pitfalls). ### Common build options @@ -123,12 +123,12 @@ fastfetch.c → options/ (global configuration) → common/ (infrastructure) ``` -The number of module directories and detection directories does not match, and that is expected. The following relationships are structural rather than a fixed inventory: +The number of module directories does not match the number of detection directories, which is expected. The following relationships are structural rather than a fixed inventory: -- **13 modules have no detection directory** — they are either pure layout (`break`, `separator`, `colors`, `title`, `logo`) or reuse someone else's detection result (`display`, `monitor`, `kernel`, `shell`, `terminal`, `player`, `custom`, `datetime`). +- **13 modules have no detection directory** — they are either pure layout (`break`, `separator`, `colors`, `title`, `logo`) or reuse another module's detection result (`display`, `monitor`, `kernel`, `shell`, `terminal`, `player`, `custom`, `datetime`). - **4 detection directories serve modules with different names** — `displayserver` (→ `display`, `monitor`), `gtk_qt` (→ `theme`, `icons`, `font`, `cursor`), `terminalshell` (→ `terminal`, `shell`) and `libc`. -So do not assume `modules//` always has a matching `detection//`. +Therefore, do not assume that `modules//` always has a matching `detection//`. ### Layer boundaries @@ -139,13 +139,13 @@ So do not assume `modules//` always has a matching `detection//`. | `detection/` | cross-platform data retrieval, clean result structs | print anything, read display config | | `common/` | general-purpose utilities, no business logic | depend on a specific module | -The test is simple: **no file under `detection/` should contain `printf` or read `instance.config`.** Symmetrically, no file under `modules/` should contain `#ifdef __linux__`. +A simple check: **no file under `detection/` should contain `printf` or read `instance.config`.** Symmetrically, no file under `modules/` should contain `#ifdef __linux__`. --- ## Core architecture: modules vs. detection -This is the single most important convention in the codebase. Every feature is a fixed set of files — using CPU as the example: +This is the most important convention in the codebase. Every feature consists of a fixed set of files; using CPU as the example: ``` modules/cpu/option.h module option struct @@ -158,7 +158,7 @@ detection/cpu/cpu_bsd.c │ detection/cpu/cpu_nosupport.c ┘ ``` -The interface is deliberately minimal — usually just two things: +The interface is deliberately minimal, typically consisting of only two declarations: ```c typedef struct FFCPUResult { ... } FFCPUResult; // result struct @@ -169,7 +169,7 @@ The `const char*` return value is an **error string**; `nullptr` means success. Helper functions are only exposed when they are genuinely shared between platform implementations (as in `cpu.h`: `ffCPUAppleCodeToName`, `ffCPUDetectByCpuid`). -**The payoff:** adding a platform never touches `modules/`; fixing output formatting never touches `detection/`. +**The benefit of this separation:** adding a platform never touches `modules/`; fixing output formatting never touches `detection/`. --- @@ -197,7 +197,7 @@ typedef struct FFModuleBaseInfo { } FFModuleBaseInfo; ``` -The source comment openly admits this is UB — `void*` is not compatible with `FF*Options*`. It is a pragmatic compromise to get polymorphism in C. Don't try to "fix" it. +The source comment acknowledges that this is undefined behavior, since `void*` is not compatible with `FF*Options*`. It is a pragmatic compromise to obtain polymorphism in C; do not attempt to "fix" it. ### The registry: a first-letter hash bucket @@ -210,7 +210,7 @@ for (; *modules; ++modules) { // linear s } ``` -The registry is small enough that a subtraction plus a few string comparisons is preferable to a general-purpose hash table. When adding a module, place its descriptor in the bucket matching the first letter of `.name`; keep the existing `nullptr` terminator at the end. +Because the registry is small, a single arithmetic bucket lookup followed by a few string comparisons is preferable to a general-purpose hash table. When adding a module, place its descriptor in the bucket matching the first letter of `.name`, and keep the existing `nullptr` terminator at the end. ### The calling convention: zero heap allocation @@ -230,7 +230,7 @@ baseInfo->destroyOptions(optionBuf); static_assert(sizeof(FFCPUOptions) <= FF_OPTION_MAX_SIZE, "FFCPUOptions size exceeds maximum allowed size"); ``` -**Meaning: no module option struct may exceed 256 bytes.** Hard constraint, enforced at compile time. +**In other words, no module option struct may exceed 256 bytes.** This is a hard constraint, enforced at compile time. The three dispatch entry points: @@ -238,20 +238,49 @@ The three dispatch entry points: |---|---|---| | `parseModuleJsonObject` | `common/impl/jsonconfig.c:90` | the `modules[]` array in a JSONC config | | `parseStructureCommand` | `common/impl/commandoption.c:181` | the colon-separated structure string | -| `ffParseModuleOptions` | `common/impl/commandoption.c:13` | **deprecated** — see [Gotchas](#4-cli-module-options-are-removed) | ### Build-time module discovery -`CMakeLists.txt:131` globs `src/modules/*/` and generates a `MODULE_DISABLE_` option per directory: +`CMakeLists.txt:134` globs `src/modules/*/*.c`, keeps only the entries whose file name matches their directory name, and generates a `MODULE_DISABLE_` option for each: ```cmake -file(GLOB FF_MODULE_DIRS RELATIVE "..." ".../src/modules/*/") +file(GLOB FF_MODULE_SRCS CONFIGURE_DEPENDS RELATIVE "..." ".../src/modules/*/*.c") +set(FF_MODULE_DIRS "") +foreach(FF_MODULE_SRC ${FF_MODULE_SRCS}) + get_filename_component(FF_MODULE_DIR "${FF_MODULE_SRC}" DIRECTORY) + get_filename_component(FF_MODULE_NAME "${FF_MODULE_SRC}" NAME_WE) + if("${FF_MODULE_DIR}" STREQUAL "${FF_MODULE_NAME}") + list(APPEND FF_MODULE_DIRS "${FF_MODULE_DIR}") + endif() +endforeach() + foreach(FF_MODULE_DIR ${FF_MODULE_DIRS}) + string(TOUPPER "${FF_MODULE_DIR}" FF_MODULE_UPPER) option(MODULE_DISABLE_${FF_MODULE_UPPER} "Disable module ${FF_MODULE_DIR}" OFF) endforeach() ``` -**Adding a module requires no CMake change.** The same trick generates the package manager switches (`:146` regex-scans `src/modules/packages/option.h` for `FF_PACKAGES_FLAG_*_BIT`). +Because the glob matches sources rather than directories, a directory counts as a module only when `src/modules//.c` exists. Empty directories — which git does not track, so they are easy to leave behind — and stray files are ignored instead of breaking the configure step with `Cannot find source file`. + +### Module sources are discovered automatically + +`FF_MODULE_DIRS` also drives the source list (`CMakeLists.txt:516`): + +```cmake +foreach(FF_MODULE_DIR ${FF_MODULE_DIRS}) + list(APPEND LIBFASTFETCH_SRC + src/modules/${FF_MODULE_DIR}/${FF_MODULE_DIR}.c + ) +endforeach() +``` + +Consequences: + +- `src/modules//.c` is compiled as soon as the file exists — **there is no source list to edit for the module layer**. +- The file name must match the directory name. A directory whose `.c` file is named differently, or has none at all, is silently not a module: a typo therefore shows up as a module missing from `fastfetch --list-modules`, not as a build error. +- The glob uses `CONFIGURE_DEPENDS`, so with the Makefile and Ninja generators the build re-evaluates it and re-runs CMake when a module source is added or removed. With other generators, or to be safe, re-run `cmake -B build` explicitly. + +Only the module layer is automated. Sources under `src/detection/` and `src/common/impl/` are **not** globbed and must still be listed by hand — see [step 5](#5-add-the-platform-sources-to-cmakeliststxt). --- @@ -276,9 +305,7 @@ typedef struct FFFooOptions { static_assert(sizeof(FFFooOptions) <= FF_OPTION_MAX_SIZE, "FFFooOptions size exceeds maximum allowed size"); ``` -`FFModuleArgs` must come first. It provides `key`, `format`, `outputColor`, `keyColor`, `keyIcon` and `keyWidth`; because it is the first field, `ffJsonConfigParseModuleArgs()` can handle these generically and you get them **for free — no parsing code to write**. - -Note these fields can only be set through the JSON config; CLI module options have been removed (see [Gotchas](#4-cli-module-options-are-removed)). +`FFModuleArgs` must come first. It provides `key`, `format`, `outputColor`, `keyColor`, `keyIcon` and `keyWidth`; because it is the first field, `ffJsonConfigParseModuleArgs()` handles these generically, **so no parsing code is required**. ### 2. Implement detection @@ -307,7 +334,7 @@ const char* ffDetectFoo(const FFFooOptions* options, FFFooResult* result) { } ``` -**Write one file per target platform, and a `foo_nosupport.c` fallback for the rest.** There are already 45 `*_nosupport.c` files doing exactly this. +**Write one file per target platform, plus a `foo_nosupport.c` fallback for the remaining platforms.** The repository currently contains 45 `*_nosupport.c` files serving this purpose. ### 3. Implement the module @@ -339,11 +366,11 @@ FFModuleBaseInfo ffFooModuleInfo = { Points to note: -- **`displayName`** is a literal block of all 20 language fields — there is no shortcut macro, so copy the layout from a neighboring module: **`en`, `ar`, `cs`, `de`, `es`, `fr`, `gl`, `he`, `id`, `it`, `ja`, `ko`, `pl`, `pt`, `ru`, `tr`, `uk`, `vi`, `zh_CN`, `zh_TW`**. - **Fill in all 20.** The struct is read by byte offset (see [Gotchas](#6-localization-uses-byte-offsets-not-enums)) and there is **no fallback** — a missing field means the key prints empty in that language. -- **`formatArgs`** must be exhaustive. It drives the placeholder list printed by `fastfetch -h foo-format`; **anything you omit is invisible and unusable to the user.** +- **`displayName`** is a literal block of all 20 language fields; there is no shortcut macro, so copy the layout from an existing module: **`en`, `ar`, `cs`, `de`, `es`, `fr`, `gl`, `he`, `id`, `it`, `ja`, `ko`, `pl`, `pt`, `ru`, `tr`, `uk`, `vi`, `zh_CN`, `zh_TW`**. + **Fill in all 20.** The struct is read by byte offset (see [Pitfalls](#6-localization-uses-byte-offsets-not-enums)) and there is **no fallback** — a missing field means the key prints empty in that language. +- **`formatArgs`** must be exhaustive. It drives the placeholder list printed by `fastfetch -h foo-format`; **any placeholder omitted here is neither listed nor usable by the user.** - The `moduleFormat` section in `doc/json_schema.json` is generated by `fastfetch -h format-json`. Its metadata comes from `FFModuleBaseInfo::formatArgs`. To keep the schema and module descriptors consistent, do not edit the `moduleFormat` section in `doc/json_schema.json` by hand; update the module metadata and regenerate it instead. -- **`defaultOrder`**: use the current maximum plus 1. Search the existing descriptors for `.defaultOrder =` before choosing a value; do not copy a hard-coded value from this document. Leaving it out, or setting it to `0`, makes the module **disappear** from the interactive `--gen-config` picker. Only `logo`, `command` and `custom` do this deliberately, because they need user arguments or are invoked directly by the display layer. +- **`defaultOrder`**: use the current maximum plus 1. Search the existing descriptors for `.defaultOrder =` before choosing a value; do not copy a hard-coded value from this document. Leaving it out, or setting it to `0`, excludes the module from the interactive `--gen-config` picker. Only `logo`, `command` and `custom` rely on this behavior, because they require user arguments or are invoked directly by the display layer. ### 4. Register it @@ -358,11 +385,46 @@ Points to note: `FF_MODULE_DISABLE_FOO` is generated by CMake — you do not define it yourself. -### 5. Modules that need warm-up +### 5. Add the platform sources to `CMakeLists.txt` -If your module needs a sampling interval (CPU usage) or a network round-trip (public IP, weather), also implement `ffPrepareFoo()` and register it in the switch inside `ffPrepareCommandOption()` in `common/impl/commandoption.c`, under the right first-letter `case`. Six modules do this today: CPUUsage, DiskIO, NetIO, PublicIP, Top and Weather. +The module layer is discovered by glob (see [Build-time module discovery](#module-sources-are-discovered-automatically)), **but `src/detection/**` and `src/common/impl/**` are not**. `CMakeLists.txt` lists them explicitly, inside one mutually exclusive chain of platform blocks: -### 6. Verify +| Block | Line | Covers | +|---|---|---| +| `if(LINUX)` | `CMakeLists.txt:522` | Linux | +| `elseif(ANDROID)` | `CMakeLists.txt:608` | Android (Termux) | +| `elseif(FreeBSD)` | `CMakeLists.txt:691` | FreeBSD, MidnightBSD, DragonFly | +| `elseif(NetBSD)` | `CMakeLists.txt:788` | NetBSD | +| `elseif(OpenBSD)` | `CMakeLists.txt:872` | OpenBSD | +| `elseif(APPLE)` | `CMakeLists.txt:959` | macOS / iOS | +| `elseif(WIN32)` | `CMakeLists.txt:1046` | Windows | +| `elseif(SunOS)` | `CMakeLists.txt:1121` | Solaris / illumos | +| `elseif(Haiku)` | `CMakeLists.txt:1204` | Haiku | +| `elseif(GNU)` | `CMakeLists.txt:1282` | GNU/Hurd | + +Add the platform implementation to every block whose platform it supports, and `foo_nosupport.c` to every remaining block, so that all ten platforms still link: + +```cmake +elseif(FreeBSD) + list(APPEND LIBFASTFETCH_SRC + ... + src/detection/foo/foo_bsd.c + ) +``` + +Points to note: + +- Exactly one block is compiled per build, so a file omitted from a block does not exist for that platform, and the link fails with an undefined reference to `ffDetectFoo()` — **on that platform only**. A missing entry therefore builds fine locally and fails in CI; this is why every block must be covered. +- A platform may reuse another platform's implementation instead of a stub. `src/common/impl/networking_linux.c`, for example, is listed in nine of the ten blocks (all but `WIN32`). Check where the closest sibling module points before adding a new file. +- `DragonFly` is handled by the inner `if(DragonFly)` sub-block inside the FreeBSD block (`CMakeLists.txt:773`); add the variant there, as `processes`, `top` and `wifi` do. +- New helpers under `src/common/impl/` follow the same rule: they are not globbed, and each block that needs one must list it. +- A few files are appended outside the platform chain because they depend on an option or on a specific feature — for example the proprietary GPU backends (`CMakeLists.txt:1379`) and `src/common/impl/wcwidth.c` (`CMakeLists.txt:1408`). Those are written by hand as well. + +### 6. Modules that need warm-up + +If your module needs a sampling interval (CPU usage) or a network round-trip (public IP, weather), also implement `ffPrepareFoo()` and register it in the switch inside `ffPrepareCommandOption()` in `common/impl/commandoption.c`, under the matching first-letter `case`. Six modules currently do this: CPUUsage, DiskIO, NetIO, PublicIP, Top and Weather. + +### 7. Verify ```sh cmake -B build && cmake --build build -j @@ -371,7 +433,9 @@ cmake -B build && cmake --build build -j ./build/fastfetch --gen-config # confirm it appears in the picker ``` -Note the `-format` suffix on the help flag: `fastfetch -h foo` does not work, only `fastfetch -h foo-format`. +Note the `-format` suffix on the help flag: `fastfetch -h foo` is not supported; only `fastfetch -h foo-format` works. + +If you added detection sources, re-check [step 5](#5-add-the-platform-sources-to-cmakeliststxt) before pushing: a platform block you missed compiles fine locally and fails only when that platform is built. --- @@ -379,13 +443,13 @@ Note the `-format` suffix on the help flag: `fastfetch -h foo` does not work, on A new logo **must** have a corresponding "Logo Request" issue, linked from the PR with `Closes #1234`. Logo PRs without a linked issue are not accepted. -### 1. Drop in the ASCII file +### 1. Add the ASCII file ``` src/logo/ascii//.txt ``` -For example `src/logo/ascii/o/omarchy.txt`. Directories are already split by first letter (`a/` … `z/`, plus `_/`). +For example `src/logo/ascii/d/distro.txt`. Directories are already split by first letter (`a/` … `z/`, plus `_/`). The file is plain ASCII art with `$1` … `$9` as color placeholders: @@ -395,26 +459,29 @@ $3 /\\\/// $2refined.$1 /\\\\// ``` -- `$1` … `$9` map to palette slots 1–9; at most 9 (`FASTFETCH_LOGO_MAX_COLORS = 9`) +- `$1` … `$9` map to palette slots 1–9; at most 9 are supported (`FASTFETCH_LOGO_MAX_COLORS = 9`) - `$$` is a literal `$` -- Characters without a placeholder inherit the current color. Of the 530 existing logos, 239 use no placeholders at all (monochrome) and 291 do — **new logos should use placeholders**; monochrome is a legacy style +- Characters without a placeholder inherit the current color. Of the 530 existing logos, 239 use no placeholders at all (monochrome) and 291 do; **new logos should use placeholders**, as monochrome is a legacy style - Tabs are expanded to 4 spaces - Colors can be overridden with `--logo-color-1` … `--logo-color-9` ### 2. Register it in the `.inc` -CMake turns each `.txt` into a `FASTFETCH_DATATEXT_LOGO_` macro (`CMakeLists.txt:418`), but **the registry itself is maintained by hand**. Edit `src/logo/ascii/.inc`: +CMake turns each `.txt` file into a `FASTFETCH_DATATEXT_LOGO_` macro (`CMakeLists.txt:418`), but **the registry itself is maintained by hand**. Edit `src/logo/ascii/.inc`: ```c -#ifdef FASTFETCH_DATATEXT_LOGO_OMARCHY -// Omarchy +// src/logo/ascii/d.inc + +#ifdef FASTFETCH_DATATEXT_LOGO_DISTRO +// Distro { - .names = { "omarchy", "Omarchy" }, - .lines = FASTFETCH_DATATEXT_LOGO_OMARCHY, + .names = { "Distro" }, // ID (preferred) or NAME from /etc/os-release, do NOT add both + .lines = FASTFETCH_DATATEXT_LOGO_DISTRO, .colors = { - FF_COLOR_FG_BLUE, - FF_COLOR_FG_WHITE, - FF_COLOR_FG_CYAN, + FF_COLOR_FG_PRIMARY, // recommended for using as WHITE replacement, light-theme terminal friendly + FF_COLOR_FG_BLUE, // preferred + FF_COLOR_FG_256 "34", + FF_COLOR_FG_RGB "0;255;0", // not recommended because of bad compatibility with raw TTY }, }, #endif @@ -431,13 +498,13 @@ If the same OS has multiple logo variants, mark the variant explicitly with `.ty .type = FF_LOGO_LINE_TYPE_SMALL_BIT, ``` -Use `FF_LOGO_LINE_TYPE_ALTER_BIT` for an alternate logo, `FF_LOGO_LINE_TYPE_SMALL_BIT` for a small logo, or combine the flags when both apply. This is also a lookup optimization: a logo marked `FF_LOGO_LINE_TYPE_SMALL_BIT` is considered only for `type = small`, while a logo marked `FF_LOGO_LINE_TYPE_ALTER_BIT` is never selected by automatic detection. Alternate logos are available only when explicitly requested through `--logo `. +Use `FF_LOGO_LINE_TYPE_ALTER_BIT` for an alternate logo, `FF_LOGO_LINE_TYPE_SMALL_BIT` for a small logo, or combine the flags when both apply. This is also a lookup optimization: a logo marked `FF_LOGO_LINE_TYPE_SMALL_BIT` is considered only for `type = small`, while a logo marked `FF_LOGO_LINE_TYPE_ALTER_BIT` is never selected by automatic detection. Alternate logos are available only when explicitly requested through `-l `. ### 3. Verify ```sh -./build/fastfetch --logo omarchy -./build/fastfetch --list-logos | grep -i omarchy +./build/fastfetch -l distro +./build/fastfetch --list-logos | grep -i distro ``` --- @@ -474,9 +541,9 @@ Current distribution of platform files in `detection/`: (These are repository inventory figures, counting `.c`, `.m`, `.cpp`, and `.h`; they may change as platforms and modules are added.) -**Don't pile `#ifdef __linux__` into one file.** When adding platform support, copy the closest existing implementation and change the suffix. +**Do not accumulate `#ifdef __linux__` blocks in a single file.** When adding platform support, copy the closest existing implementation and change the suffix. -`common/` uses the same idea: `common/impl/` contains `io_unix.c` / `io_windows.c`, `netif_linux.c` / `netif_apple.c` / `netif_bsd.c` and so on, with `common/apple/`, `common/windows/` and `common/haiku/` holding platform-specific helpers. +`common/` follows the same convention: `common/impl/` contains `io_unix.c` / `io_windows.c`, `netif_linux.c` / `netif_apple.c` / `netif_bsd.c` and similar files, with `common/apple/`, `common/windows/` and `common/haiku/` holding platform-specific helpers. When several platform suffixes could apply, use the most specific implementation supported by the build system (for example, `_nbsd.c` instead of the generic `_bsd.c` on NetBSD). Keep the generic file as the fallback for platforms that share its conventions. @@ -507,7 +574,7 @@ clang-format -i src/modules/foo/*.c src/modules/foo/*.h `src/3rdparty/**`, `build/**` and `src/logo/builtin.c` are listed in `.clang-format-ignore` — **do not reformat them.** -`.editorconfig` adds LF line endings, 4-space indent, a final newline, and trailing whitespace trimmed (except in Markdown). +`.editorconfig` specifies LF line endings, a 4-space indent, a final newline, and trailing whitespace trimmed (except in Markdown). ### Naming @@ -522,7 +589,7 @@ clang-format -i src/modules/foo/*.c src/modules/foo/*.h ### Spelling -CI runs codespell (`.codespellrc`). Known false positives live in `ignore-words-list` (`iterm`, `compiletime`, and various non-English distro words). Add new words there rather than changing the code. +CI runs codespell (`.codespellrc`). Known false positives are listed in `ignore-words-list` (`iterm`, `compiletime`, and various non-English distro words). Add new words there rather than changing the code. ### Compiler warnings @@ -566,7 +633,7 @@ Common verbs: `adds`, `removes`, `fixes`, `improves`, `updates`, `corrects`, `pr Scopes in use: `Top`, `Processes`, `Memory`, `Logo (Builtin)`, `CI`, `Doc`, `Presets`, `Global`, plus individual module names. -Documentation-only changes get a `[ci skip]` suffix. +Documentation-only changes use a `[ci skip]` suffix. --- @@ -617,21 +684,21 @@ cmake --build build cd build && ctest --output-on-failure ``` -Coverage focuses on the core data structures and the formatting engine in `common/`. The `detection/` layer has no automated tests (it depends too heavily on a real system) and is instead covered by the CI matrix — 20 workflows under `.github/workflows/` spanning Linux (including musl, loong64, armv7l, i686), macOS, Windows, FreeBSD, NetBSD, OpenBSD, DragonFly, Solaris, OmniOS and Haiku, plus spellcheck and benchmark jobs. +Coverage focuses on the core data structures and the formatting engine in `common/`. The `detection/` layer has no automated tests, because it depends on the state of a running system; it is instead covered by the CI matrix — 20 workflows under `.github/workflows/` spanning Linux (including musl, loong64, armv7l, i686), macOS, Windows, FreeBSD, NetBSD, OpenBSD, DragonFly, Solaris, OmniOS and Haiku, plus spellcheck and benchmark jobs. -**If you touch `common/FFstrbuf.h`, `common/format.h` or `common/color.h`, please extend the matching test.** +**If you modify `common/FFstrbuf.h`, `common/format.h` or `common/color.h`, extend the corresponding test.** --- ## Pull requests -1. **Open an issue first** (feature request / bug report / logo request) so you don't waste effort. Templates are in `.github/ISSUE_TEMPLATE/`. +1. **Open an issue first** (feature request / bug report / logo request) to confirm that the change is wanted before investing effort. Templates are in `.github/ISSUE_TEMPLATE/`. 2. Branch off `dev` — **`dev` is the main development branch**, not `master`. 3. Follow the [commit message convention](#commit-messages). 4. Update `CHANGELOG.md` for user-visible changes. 5. Open the PR against `dev` and fill in `.github/pull_request_template.md`: - Summary - - Related issue (**required for new logos**, otherwise the PR won't be accepted) + - Related issue (**required for new logos**; otherwise the PR is not accepted) - Changes - Screenshots (required for visual changes) - Checklist: confirm you tested locally @@ -649,13 +716,13 @@ cd build && ctest --output-on-failure # tests --- -## Gotchas +## Pitfalls -Things that are easy to get wrong when reading this codebase. Most of them follow from the "obsessively fast startup" goal. +The following points are easy to misinterpret when reading this codebase. Most of them follow from the startup-time objective described above. ### 1. Option structs must not exceed 256 bytes -`FF_OPTION_MAX_SIZE = 1 << 8`. Exceeding it is caught at compile time by `static_assert`. Don't try to raise the value — it determines the stack cost of every module invocation. +`FF_OPTION_MAX_SIZE = 1 << 8`. Exceeding it is caught at compile time by `static_assert`. Do not attempt to increase the value: it determines the stack cost of every module invocation. ### 2. `FF_MODULE_DISABLE_*` controls registration, not compilation @@ -668,9 +735,11 @@ Things that are easy to get wrong when reading this codebase. Most of them follo **Disabling modules in a Debug build does not shrink the binary.** LTO is enabled whenever `CMAKE_BUILD_TYPE != Debug`, and the default `RelWithDebInfo` already satisfies that — so measure size with `RelWithDebInfo` or `Release`, never with `Debug`. (The "Release mode" wording in the comment above is imprecise.) -### 3. CMake globs don't trigger reconfiguration +### 3. Only two things are discovered by glob -Both the module directories and the logo files are discovered by glob. CMake will not notice new files on its own — you need to re-run `cmake -B build` (or delete `build/CMakeCache.txt` and start over). This is the classic CMake footgun. +The module sources (`CMakeLists.txt:134`) and the logo `.txt` files (`CMakeLists.txt:429`). Both use `CONFIGURE_DEPENDS`, so with the Makefile and Ninja generators the build re-checks the glob and re-runs CMake when the result changes; other generators (Visual Studio and Xcode in particular) do not track it as reliably. Re-run `cmake -B build` after adding a module source or a logo file rather than relying on that behavior. + +Everything else — `src/detection/**` and `src/common/impl/**`, and any source outside the platform chain — must be listed in `CMakeLists.txt` by hand; see [step 5](#5-add-the-platform-sources-to-cmakeliststxt). ### 4. CLI module options are removed @@ -686,9 +755,9 @@ Error: Unsupported module option: --cpu-temp ### 5. `defaultOrder = 0` hides a module from `--gen-config` -`collectModuleInfos` (`genconfig.c:186`) skips any module whose `defaultOrder` is `0`. C zero-initializes the field, so **omitting it is the same as setting it to 0**. Only `logo`, `command` and `custom` do this on purpose. +`collectModuleInfos` (`genconfig.c:186`) skips any module whose `defaultOrder` is `0`. C zero-initializes the field, so **omitting it is the same as setting it to 0**. Only `logo`, `command` and `custom` rely on this intentionally. -`defaultOrder` affects only the ordering in the interactive `--gen-config` picker; it has no effect on runtime output order. +`defaultOrder` affects only the ordering in the interactive `--gen-config` picker; it has no effect on the runtime output order. ### 6. Localization uses byte offsets, not enums @@ -699,19 +768,23 @@ Error: Unsupported module option: --cpu-temp (*(const char**) ((uint8_t*) &ff ## moduleName ## ModuleInfo.displayName + instance.config.display.keyLanguage)) ``` -Consequence: **the field order of `FFModuleDisplayName` must never change** — reordering silently scrambles every language. +Consequence: **the field order of `FFModuleDisplayName` must never change** — reordering the fields silently corrupts the output for every language. -### 7. The `multithreading` switch barely does anything +### 7. The `multithreading` option has limited effect The global `multithreading` option currently takes effect in exactly one place: `common/impl/networking_linux.c:339`. Modules are still printed sequentially. Modules that need concurrency go through the `ffPrepare*` warm-up hooks instead, which start sampling or fire off requests before the print loop begins. -### 8. Don't print or read config inside `detection/` +This may change in the future. The main blocking issue is that there are dependencies between different modules. + +### 8. Do not print or read configuration inside `detection/` The `detection/` layer must stay pure: read system state, fill a struct, return an error string. Any `printf` or any read of `instance.config` there is a design error. +Use `FF_DEBUG` (`src/common/debug.h`) for logging. + ### 9. `src/logo/builtin.c` and `3rdparty/` are formatting-exempt -The former is a large generated/hand-maintained data table, the latter is upstream code. Both are excluded via `.clang-format-ignore` — leave them alone. +The former is a large generated and hand-maintained data table; the latter is upstream code. Both are excluded via `.clang-format-ignore`; do not reformat either. ---