ferret/CONTRIBUTING.md

Contributing to ferret

Patches are welcome at http://git.dcos.net/dcosnet/ferret.

Development setup

git clone http://git.dcos.net/dcosnet/ferret.git
cd ferret
./scripts/setup-libmpv.sh
source ./mpv-prefix/env.sh
cargo build --release

See QUICKSTART.md for the 5-minute get-started guide and README.md for full architecture documentation.

Code style

• Rust 2021 edition, MSRV 1.75. No features requiring a newer compiler.
• cargo fmt before committing. No manual formatting.
• cargo clippy --release -- -D warnings must be clean. The CI lint script (./scripts/build.sh --lint) enforces this.
• All FFI lives in mpv-bindings. No unsafe blocks outside that crate except where documented with a SAFETY comment explaining the invariant.
• No async. Thread-per-subsystem with crossbeam channels. See BLOG.md → "Why thread-per-subsystem, not async?" for the rationale.
• Table-driven dispatch over branch ladders. See "Coding standards" below.

Coding standards

This codebase follows four standards, adapted to Rust where the standard is language-specific:

Standard	Application
PEP 868	Idiomatic clarity: data over branches, comprehensions over loops, one statement per line
POSIX	Shell scripts: set -euo pipefail, double-quoted expansions, command -v for tool detection
SEI CERT	Defensive FFI: every unsafe block carries a SAFETY comment; no ad-hoc extern "C" outside mpv-bindings::sys
MISRA	Table-driven control flow; closed-set match → const TABLE lookup; single exit per logical path

Table-driven dispatch (PEP 868 + MISRA)

When you have a closed-set match on an enum or a known string set, use a const TABLE lookup instead of a match ladder. Example:

// PREFERRED: table-driven
const TABLE: &[(LoopMode, &str, &str)] = &[
    (LoopMode::Off,       "no",  "no"),
    (LoopMode::File,      "inf", "no"),
    (LoopMode::Playlist,  "no",  "inf"),
];
let (_, file_v, list_v) = TABLE
    .iter()
    .copied()
    .find(|(m, _, _)| *m == mode)
    .expect("LoopMode is exhaustive over TABLE");

// AVOID: branch ladder
match mode {
    LoopMode::Off => { /* ... */ }
    LoopMode::File => { /* ... */ }
    LoopMode::Playlist => { /* ... */ }
}

Why: the table is data — it can be inspected, serialized, and extended without touching control flow. New variants add one row, not one arm.

Step-down logic (Unix philosophy)

When you have a fork of choices (multiple backends, multiple formats, multiple fallbacks), use or_else chaining — not if/else ladders. Each backend is a single-purpose function returning Option<T>; the entry point is a flat chain.

// PREFERRED: step-down chain
pub fn pick_file(title: &str, filters: &[(&str, &[&str])]) -> Option<String> {
    pick_file_zenity(title, filters)
        .or_else(|| pick_file_kdialog(title, filters))
        .or_else(|| pick_file_rfd(title, filters))
}

// AVOID: nested if/else
pub fn pick_file(title: &str, filters: &[(&str, &[&str])]) -> Option<String> {
    if let Some(p) = pick_file_zenity(title, filters) {
        Some(p)
    } else if let Some(p) = pick_file_kdialog(title, filters) {
        Some(p)
    } else {
        pick_file_rfd(title, filters)
    }
}

Why: the chain is composable, testable per-backend, and reads top-to-bottom as "try this, then this, then this". Adding a new backend is one line.

Decisive language

Code comments and docs use present-tense, decisive statements. Avoid phrasing that suggests back-and-forth or indecision:

• ❌ "falls back to", "restored", "brought back", "kept for compatibility", "non-breaking", "previously", "original behavior was"
• ✅ "steps down to", "routes through", "uses", "selects"

Comments should read as design decisions, not as a record of changes.

Pre-commit checks

Run before pushing:

./scripts/build.sh --lint    # clippy + bracket audit + deref audit
./scripts/build.sh --test    # cargo test
./scripts/build.sh --ci      # both + build (full CI pass)

The bracket audit (scripts/audit_brackets.py) is a Rust-aware tokenizer that catches unbalanced ()/{}/[] in milliseconds — faster than waiting for cargo check to reach the affected crate.

The deref audit (scripts/audit_deref.py) flags match arms on &Enum where a captured reference might be forwarded without * — the bug class that caused the SetVideoRotate compile error in §9 of the QA report.

Adding a new hotkey

1. Add the Key → Cmd row to const TABLE in crates/player-app/src/keymap.rs.
2. Add a menu entry for the hotkey in crates/player-ui/src/app.rs (File / Playback / Audio / Subtitles / Video / Help menu). Append the shortcut key in parentheses to the label: "Play / Pause (Space)".
3. If the hotkey has no existing control-bar button, consider whether it needs one. Every hotkey must have a menu entry and/or a button — no orphan hotkeys.
4. Update the Keyboard Shortcuts table in README.md.
5. Run ./scripts/build.sh --lint to verify the table compiles clean.

Adding a new mpv property

1. Add a const PROP_<NAME>: EventId = N; to crates/player-core/src/engine.rs (next available integer).
2. Add the property to the observed array in engine_main.
3. Add a (PROP_<NAME>, PropertyValue::Variant(value)) arm to apply_property_change.
4. Add the field to PlaybackState in crates/player-core/src/state.rs.
5. Expose the state in the UI if user-visible (crates/player-ui/src/app.rs).

Reporting bugs

Please include:

• ferret version (ferret --version)
• libmpv version (pkg-config --modversion mpv)
• Desktop environment / window manager
• Steps to reproduce
• Log output (RUST_LOG=info ferret ...)

License

By contributing, you agree that your contributions are licensed under the GPL-2.0-or-later license that covers the project.