Skip to content

CONTRIBUTING.md

Latest commit

 

History

History
126 lines (85 loc) · 11.6 KB

CONTRIBUTING.md

File metadata and controls

126 lines (85 loc) · 11.6 KB
Raw

Contributing to binvim

Thanks for considering a contribution. binvim is a small project with a small surface area, and the bar for merging is "the code matches the existing style and the change is something the maintainer wants in the editor." This document covers what you need to know before opening a PR.

Licence and what "contribution" means here

binvim is source-available, not open source — see LICENSE for the full text. The short version that matters for contributors:

  • You may clone, build, and modify binvim for your own use on hardware you control.
  • Fork away to open a PR — publicly is fine, and on GitHub it's the only option, since a PR can't be opened from a private fork. What you can't do is keep it running as a project of its own: leave the fork relationship intact, don't publish releases or registry entries from it, and delete it (or flip it private) once your PR is merged or closed. LICENSE §3 spells this out as a "Contribution Fork".
  • You may not redistribute binvim, publish binaries, or stand up a copy of the repo as its own project.
  • By submitting a PR you grant the maintainer a perpetual, irrevocable, sublicensable licence to use your contribution as part of binvim, including under different licence terms in the future (LICENSE §4). You also represent that the work is yours to grant.

If you can't agree to that, please don't open a PR.

Before you start

For anything bigger than a one-line fix, open an issue first. binvim is opinionated about what it includes — pre-agreed scope avoids the awkward case where a working PR gets closed because the feature isn't wanted. Good things to flag up front:

  • New language / LSP support.
  • New keybindings or operators — the parser is a Vim-grammar state machine; new verbs need to fit it, not bolt onto it.
  • New configuration surface — ~/.config/binvim/config.toml is intentionally minimal.
  • New external-tool dependencies — every external binary in the README install table is one more thing that can be missing on a user's machine.
  • New DAP adapters — the adapter registry in src/dap/specs.rs is the only entry point; adapter-specific behaviour belongs there, not in manager.rs.

Bug fixes, missing-LSP arms, and tree-sitter additions don't need a pre-discussion — just open the PR.

Development setup

cargo build                                  # debug build
cargo build --release                        # release build (target/release/binvim)
cargo test                                   # full suite, ~640 unit tests
cargo test motion::tests                     # one module
cargo test motion::tests::word_forward_basic # one test
cargo run -- path/to/file                    # debug-build run

CI runs cargo test, cargo clippy and cargo fmt --check on every PR. Both cargo fmt --check and clippy are gating — run cargo fmt before you push, and cargo +1.98.0 clippy --locked --all-targets -- -D warnings (CI pins clippy to 1.98.0 and fails on any warning; a newer local clippy flags different things). The formatting config is rustfmt.toml at the repo root; max_width = 100 plus single_line_let_else_max_width = 100 keeps compact let … else and single-line method chains intact.

If this is your first PR to the repo, GitHub holds the workflow run until the maintainer approves it — a PR sitting with no checks reported is waiting on that, not broken.

If you're testing changes by running binvim interactively, remember that the install/alias path is target/release/binvim — a debug build will not be picked up. Run cargo build --release after the change you want to exercise.

Repo conventions

These are not stylistic preferences — they are how the codebase is structured, and PRs that fight them tend to get bounced.

  • Mostly flat src/ layout, with three sub-module dirs. app/, lsp/, and dap/ are split across multiple files (each parent file — src/app.rs, src/lsp.rs, src/dap.rs — is a slim entry that declares children and re-exports the public API). Other modules stay flat — don't introduce new src/foo/ directories without a real reason. Inside app/, sibling-visible methods are pub(super).
  • No new files unless necessary. Prefer extending an existing module. New top-level files need to justify themselves.
  • Tests live inline, in #[cfg(test)] mod tests at the bottom of the file under test. No separate tests/ directory, no tests/integration/. motion.rs and text_object.rs have the densest coverage and are the model.
  • Comments explain why, not what. The existing comments in lang.rs (priority resolution), lsp/manager.rs (debounce/drain cap), and app/state.rs (BufferStash shape) are the pattern: load-bearing context that isn't obvious from the code. Don't add what-comments. Don't add multi-paragraph docstrings.
  • No backwards-compatibility shims, feature flags, or // removed markers for code that's been deleted. Just delete it.
  • Don't over-abstract. Three similar lines is better than a premature abstraction. Don't design for hypothetical future requirements.
  • LF line endings only. No CRLF.

Architecture quick reference

For a longer tour see CLAUDE.md. The 30-second version:

  • app.rs + app/ own the event loop, active buffer, per-buffer stashes, and all transient UI state. The App struct lives in app.rs; child files in app/ (state, view, search, registers, buffers, save, edit, visual, comment, multi_cursor, dispatch, input, lsp_glue, dap_glue, git_glue, copilot, picker_glue, quickfix, windows, health, pair) hold impl super::App blocks grouped by concern. Action dispatch is app/dispatch.rs.
  • parser.rs turns KeyEvents into Action values via the Vim-grammar state machine. Operators, motions, text-objects, counts, registers, leader, surround — all resolved here before app/dispatch.rs sees them.
  • motion.rs and text_object.rs are pure functions over (buffer, cursor). New motions or text objects belong here, with tests inline.
  • window.rs + layout.rs carry the split system: Window is a view (cursor, viewport, visual anchor, buffer index); Layout is the binary split tree whose partition() emits (WindowId, Rect) per leaf and whose focus_neighbor() does geometric h/j/k/l navigation, not tree-order.
  • lang.rs owns tree-sitter. The non-obvious bit: highlight captures resolve by pattern_index priority — later patterns win. JSON ships its own embedded query because the upstream pattern order is incompatible with that scheme. If you change the priority logic, the JSON block at lang.rs:88 is the canary.
  • lsp.rs + lsp/ is a from-scratch JSON-RPC client (entry + types/specs/client/io/manager/parse). Multiple servers per buffer is supported and used (e.g. tsserver + Tailwind on .tsx). didChange is debounced with a 50ms burst window in app/lsp_glue.rs.
  • dap.rs + dap/ is the Debug Adapter Protocol client, structurally parallel to lsp/ (types/specs/client/io/manager). Adapter-specific behaviour stays in dap/specs.rs; manager.rs and the wire layer are adapter-agnostic.
  • git.rs shells out to git diff --unified=0 for the per-line gutter stripe; markdown_render.rs produces the Normal-mode conceal transforms; session.rs persists the open-buffer set per cwd to ~/.cache/binvim/sessions/<hash>.json.
  • render.rs is the only module that talks to crossterm for drawing.

Adding a new LSP

The five-file change is always:

  1. New arm in primary_spec_for_path (src/lsp/specs.rs).
  2. Lang variant + extension/basename entry in Lang::detect() plus matching ts_language() / highlights_query() arms in src/lang.rs (skip the tree-sitter arms only if you don't want highlighting).
  3. Icon + lang_name in the two exhaustive Lang matches in src/render.rs.
  4. Formatter arm in format_buffer (src/format.rs) plus tree-sitter-<lang> crate in Cargo.toml.
  5. New rows in the README install table for the LSP and the formatter.

There is no plugin system. Every server is hard-wired in lsp/specs.rs. That is a deliberate choice; please don't propose a plugin loader as part of an LSP PR.

Adding tree-sitter highlighting for an existing LSP

Add the crate to Cargo.toml, then a Lang variant + ts_language() arm + highlights_query() arm. If the upstream highlights query is wrong under "later pattern wins" priority (see JSON), embed a corrected query inline rather than patching the priority logic.

Adding a new DAP adapter

Three touchpoints:

  1. src/dap/specs.rs — append a DapAdapterSpec to BUILTIN_ADAPTERS with key, adapter_id, cmd_candidates, args, root_markers, a prelaunch fn (return None if the adapter builds implicitly, e.g. delve), and a build_launch_args fn that produces the launch request JSON. Add per-adapter target discovery here (find_<lang>_* helper) if the picker needs to enumerate something beyond the file the user is on.
  2. src/app/dap_glue.rs — add a dap_resolve_<lang> method that wraps the 0/1/many discovery → auto-pick or open the DebugTarget picker → call dap_start_target. Register the new key in dap_start_session's match arm.
  3. src/dap.rs — re-export any new public helpers / types.

Adapter-specific behaviour stays in dap/specs.rs + app/dap_glue.rs; manager.rs and the wire layer in types/io/client are adapter-agnostic. Like the LSP layer, there is no plugin system — every adapter is hard-wired. The four shipped adapters (netcoredbg, delve, debugpy, lldb-dap) are useful blueprints — pick the one whose discovery / prelaunch shape is closest to your target.

Verifying a change

Before opening a PR:

  • cargo test is green.
  • cargo build --release succeeds.
  • For LSP / language changes: open a representative file, run :health, and confirm the server appears under LSP servers with the expected key, language_id, and detected root. Trigger completion and hover on a known symbol.
  • For UI / rendering / keybinding changes: actually run the release binary and use the feature. Type-checks and tests verify code correctness, not feature correctness.

Pull requests

  • One logical change per PR. If you're tempted to write "and also fixed X" in the description, X is a separate PR.
  • Branch from main. Rebase on top of main before opening; no merge commits.
  • PR description should explain the why — what the user-visible behaviour was before, what it is after, and what motivated the change. The maintainer can read the diff for the what.
  • No Claude / AI / "Co-Authored-By" attribution in commit messages, branch names, or PR descriptions. The attribution CI check enforces this over every commit, the PR title and description, the branch name and the tracked files. Run git config core.hooksPath .githooks once per clone and a commit-msg hook refuses the commit instead, before it can reach a push.
  • Reference the issue number if you opened one.

Reporting bugs

Open a GitHub issue with:

  • binvim version (binvim --version or the commit SHA you built from).
  • OS and terminal emulator.
  • A minimal reproduction — file contents (or a path to a public repo), exact keystrokes, what you expected, what happened. :health output is often the fastest way to tell whether an LSP-shaped bug is a binvim issue or a missing server.

For LSP-specific bugs, the server logs go to stderr; running binvim 2> /tmp/binvim.log and attaching the relevant section is the most useful thing you can include.

Contact

For licensing questions outside the scope of LICENSE — redistribution, commercial use, hosted-service provision — contact the maintainer on Twitter/X at @bgunnarssonis. For everything else, the issue tracker is the right place.