Skip to content

contributing.md

Latest commit

 

History

History
469 lines (385 loc) · 22.9 KB

contributing.md

File metadata and controls

469 lines (385 loc) · 22.9 KB
Raw

Contributing to nvim-dapper

Handler Blocking Convention

Session manager handlers run synchronously on nvim-dap's event thread (the Neovim main thread). A handler that blocks stalls subsequent handlers and the editor.

The CI lint (lint-blocking-handlers) rejects any of these primitives in the syntactic body of a function literal passed to manager.subscribe(...):

  • vim.fn.system
  • vim.fn.systemlist
  • io.read
  • io.popen
  • vim.wait with a non-zero timeout argument

The lint is shallow by design (design D3): it inspects only the function literal's body, not helper functions it calls. Therefore the convention also applies recursively:

Do not call any of the above primitives from any function reachable from a session-manager handler, even indirectly.

If a future real incident justifies deeper call-graph analysis (e.g. a luacheck custom rule that walks one level into helper functions), revisit the lint implementation.

The complementary check is a 50 ms wall-clock backstop in the unit tests (5.25). That backstop is a sanity check for accidental tight loops; the structural lint is the load-bearing enforcement.

Downstream Isolation Rules

Every later Phase 1 capability (add-main-panel, add-variables-view, add-stack-view, add-inline-virtual-text-values) and the Phase 2 agent tool registry depend on add-dap-session-manager. Two CI lint jobs enforce the isolation boundary:

1. lint-dap-isolation

No file under lua/nvim-dapper/ outside lua/nvim-dapper/session/ may reference dap.listeners, require("dap").session, or dap.session().

Why: every Phase 1 view that hooked nvim-dap directly would accumulate listener leaks on mount/unmount cycles and produce divergent "current frame" state. The session manager absorbs all of that complexity.

2. lint-event-data

No file under lua/nvim-dapper/ outside lua/nvim-dapper/session/ may read the .data field on a dapper.SessionEvent.

Why: .data is an escape hatch holding raw nvim-dap payloads. Any information a downstream consumer needs must be promoted to a named field on the event payload so the session module's API shape remains the authoritative contract.

Phase 1 View Proposals

Each Phase 1 view proposal (add-main-panel, etc.) must:

  1. Reference add-dap-session-manager as a dependency in its proposal.
  2. Consume session state exclusively through require("nvim-dapper.session").
  3. Subscribe on mount; unsubscribe on unmount.
  4. Use pull accessors for the initial render (not a cached copy from an earlier event).

See docs/session-manager.md for the full public API and a worked example.

Namespace Discipline

Project rule §3.5: all extmark namespaces in nvim-dapper are allocated through lua/nvim-dapper/extmark_pool/. Direct calls to vim.api.nvim_create_namespace from anywhere else are forbidden so that bulk cleanup operations stay O(tracked buffers), not O(all buffers).

Two CI lint jobs enforce the boundary:

1. namespace-isolation

No file under lua/nvim-dapper/ outside lua/nvim-dapper/extmark_pool/ may reference nvim_create_namespace. Use pool.concern(name) for plugin-lifetime namespaces and pool.instance(name, owner_id) for namespaces bound to an owner lifecycle.

Why: without a central allocator, each view would call nvim_create_namespace ad-hoc, bulk cleanup would need to scan every tracked buffer, and the §3.5 rule would be unenforceable in review.

2. extmark-pool-imports

Files under lua/nvim-dapper/extmark_pool/ may not require dap, the session manager, or any AI module. The pool's only runtime dependency is vim.api.

Why: coupling the pool to any of those would re-introduce the same scanning problem at a different layer, and would prevent reuse of the pool from infrastructure that runs before the session manager initialises.

Phase 1 View Rules

Each Phase 1 view that writes extmarks must:

  1. Allocate via pool.concern(name) or pool.instance(name, owner_id).
  2. Prefer pool.set_extmark(ns, buf, line, col, opts) for single-mark writes; use pool.track(ns, buf) + raw nvim_buf_set_extmark only when fine-grained control is required.
  3. Clear via pool.clear(name) / pool.clear_in_buffer(name, buf) for concern-scoped namespaces, or pool.retire(owner_id) for instance namespaces. Never call nvim_buf_clear_namespace against a pool-owned id directly — going through the pool keeps the tracked set consistent.

See docs/extmark-pool.md for the full API.

View Author Contract

Phase 1 views (variables, stack, breakpoints, watches, REPL, output) and any future view that lives inside the main panel must follow these boundary rules. They are CI-enforced by scripts/lint.lua.

1. Register through the panel; never own windows or buffers

A view exists by calling panel.register_view(spec). The panel owns the split window(s) and the per-view scratch buffer; views must not call vim.api.nvim_open_win, vim.api.nvim_create_buf, vim.api.nvim_win_set_buf, :split, or :vsplit from any file under lua/nvim-dapper/**/*.lua outside lua/nvim-dapper/panel/. The window-ownership lint enforces this and may be opted out per file with a top-of-file -- @allow-window-api magic comment when a view legitimately needs an auxiliary popover buffer.

The spec passed to register_view must not declare layout ownership. Fields such as pane or split raise at registration. View placement is configured only through panel.splits, where users assign view ids to flat indexed split groups.

Why: every view that hand-rolled its own window would re-invent placement, focus, and switching logic, and configurable splits would have to retrofit each one. Centralising window ownership in the panel makes view-switching nvim_win_set_buf and keeps the panel-frame contract spelled out in one place.

2. Observe session state through the session manager

Views must not require("dap") or require("dap.listeners") or call dap.session() directly. Inside the panel itself, the panel-no-dap lint reinforces this for lua/nvim-dapper/panel/main/; everywhere else the existing dap-isolation lint covers it. The supported path is the panel's on_session_event(buf, event, ctx) callback, which fans out every session-manager event into mounted views.

Views that need lifecycle work regardless of activation (i.e. work that must happen even when the view has never been opened) should subscribe to nvim-dapper.session directly under a distinct subscriber_id — but this is a strong code smell, since it defeats the mount-on-first-focus laziness invariant. Question whether the work genuinely needs to happen before the user has even looked at the view.

3. Allocate extmark namespaces through the pool

Views must not call vim.api.nvim_create_namespace. Use pool.concern("<view-id>-<purpose>") or pool.instance("<name>", "<owner_id>") from nvim-dapper.extmark_pool. The namespace-isolation lint enforces this.

4. Lifecycle callbacks must not block

on_mount, on_focus, on_blur, render, and on_session_event all run synchronously on the Neovim main thread. Do not call blocking primitives (vim.fn.system, io.read, io.popen, vim.wait with a non-zero timeout) from any function reachable from these callbacks. The session-manager blocking-handler lint is being extended to cover view callback bodies with the first view proposal that introduces production register_view call sites; until then the convention is documented but not lint-enforced.

5. Reserved subscriber id

The main panel reserves subscriber_id = "main-panel" on the session manager. No other module — including hypothetical future panels such as an investigation panel — may subscribe, unsubscribe, or otherwise reference "main-panel" as a subscriber id. Future panels choose distinct ids (e.g. "investigation-panel").

Shared view-author conventions

The module nvim-dapper.views.conventions (lua/nvim-dapper/views/conventions.lua) is the SINGLE SOURCE OF TRUTH for the shared view-author surface used by every panel view (variables, stack, breakpoints, watches, and any future view). Its published table exposes six top-level fields — tree, actions, empty_states, loading, jump_to_source, retry — and the names of the DapperVariables{String,Number,Boolean,Function,Nullish,Object,Error,Name,Meta} highlight family.

The module also owns idempotent installation of the type-aware DapperVariables* hi default link defaults: the install runs once on first require, so any view that requires the module transitively gets the defaults — no view installs them itself.

Per-view subsections (variables/stack/breakpoints/watches) consume this module rather than re-derive any of its constants locally. A CI lint rejects local re-derivation and rejects require of the now-deleted nvim-dapper.views.variables.conventions path.

The DapperVariables* highlight group names are preserved from variables-view's prior ownership of the family: renaming them would break user re-links in colorschemes, and the name DapperVariables* is deliberately treated as the shared visual surface, not a variables-view concern, in spite of the prefix.

Reserved view identifiers

The following identifiers are reserved by the variables view (add-variables-view) and MUST NOT be reused by other modules:

  • Session-manager subscriber id: "variables-view".
  • Extmark-pool concern name: "variables-view".
  • Filetype: dapper-variables (set by the panel from the view id).
  • Named actions published by the variables view module: expand, collapse, expand-all-children, collapse-all-children, jump-to-source, retry, load-more. The first six are conventions exposed via nvim-dapper.views.conventions.actions and reused by sibling Group A views; load-more is view-local.

The following identifiers are reserved by the watches view (add-watches-view) and MUST NOT be reused by other modules:

  • Session-manager subscriber id: "watches-view".
  • Extmark-pool concern name: "watches-view".
  • Filetype: dapper-watches (set by the panel from the view id).
  • Named actions published by the watches view module: the six universal conventions (expand, collapse, expand-all-children, collapse-all-children, jump-to-source, retry), load-more (paging affordance for the child-tree layer), and three view-local human side- effecting actions: add-watch, remove-watch, edit-watch-expression.
  • On-disk persistence file: <vim.fn.stdpath('data')>/nvim-dapper/watches.json (schema {"version": 1, "watches": [<string>, ...]}, atomic write via temp + vim.uv.fs_rename). Tests redirect this path via require("nvim-dapper.views.watches")._set_persist_path(<temp>) so they never touch the user's real stdpath('data').

The following identifiers are reserved by the breakpoints view (add-breakpoints-view) and MUST NOT be reused by other modules:

  • Session-manager subscriber id: "breakpoints-view".
  • Extmark-pool concern name: "breakpoints-view".
  • Filetype: dapper-breakpoints (set by the panel from the view id).
  • Named actions published by the breakpoints view module: the six universal conventions (expand, collapse, expand-all-children, collapse-all-children, jump-to-source, retry) and three view- local human side-effecting actions: toggle-enabled, remove, edit-condition.
  • Highlight group: DapperBreakpointsDisabled (default-linked to Comment). No other parallel DapperBreakpoints* group is permitted; verified vs. unverified is rendered via glyphs and failed-to-verify via DapperVariablesError.

The following identifiers are reserved by the stack view (add-stack-view) and MUST NOT be reused by other modules:

  • Session-manager subscriber id: "stack-view".
  • Extmark-pool concern name: "stack-view".
  • Filetype: dapper-stack (set by the panel from the view id).
  • Named actions published by the stack view module: the six universal conventions (expand, collapse, expand-all-children, collapse-all-children, jump-to-source, retry), load-more (paging affordance for deep stacks), and one view-local human side-effecting action: jump-to-frame (focuses a frame in the session manager so downstream views — chiefly variables — re-fetch).

The stack view drives a new session-manager mutator, session.focus_frame(frame_id), which is the sole caller-side write surface on the manager for UI-level frame focus. The mutator validates the supplied frame_id against a per-thread known-frame set the manager populates from its own internal after.stackTrace listener (registered under the existing LISTENER_KEY). No caller-side helper is exposed to push observed frame ids; the manager preserves its "single source of truth" role. focus_frame issues no DAP request and creates no buffer or namespace — it only updates focused-frame state and dispatches one frame_changed event, consistent with the manager's existing role.

The breakpoints view models §3.4 differently from variables/stack/watches: the breakpoint list itself is read from the session manager's accessors (current_breakpoints() / current_function_breakpoints() / current_exception_filters()) — no DAP request is issued by the view to fetch the list. The view's dap.session():request(...) call sites are restricted to three side-effecting human actions (setBreakpoints/setFunctionBreakpoints/setExceptionBreakpoints for toggle-enabled, remove, edit-condition). Per project.md §3.2 the side-effecting actions are human tools invoked by an explicit user keybind — they do NOT present a confirmation prompt. The §3.2 confirmation-flow requirement applies to agent-proposed side effects.

The session-manager delta this proposal lands adds a breakpoint event kind to the typed event stream, plus the three accessors above. The manager observes (a) event_breakpoint arrivals, (b) setBreakpoints/ setFunctionBreakpoints/setExceptionBreakpoints response listeners, and (c) seeds from require("dap.breakpoints").get() at session_started, session_terminated, and via the public sync_breakpoints() accessor for out-of-session keymap-driven mutations. The manager remains a pure state/event module: no DAP request orchestration is added.

Breakpoints-view–specific CI lints (in addition to the shared view rules above):

  1. breakpoints-no-parallel-hl rejects any reference to a DapperBreakpoints* group other than DapperBreakpointsDisabled.
  2. breakpoints-no-fs-api rejects vim.fn.readfile, io.open, io.read, and vim.fn.system inside lua/nvim-dapper/views/breakpoints/. The only permitted read path is the async vim.uv.fs_* shape used by preview.lua.
  3. breakpoints-no-signs rejects vim.fn.sign_define, vim.fn.sign_place, and vim.fn.sign_unplace inside lua/nvim-dapper/views/breakpoints/. Source-buffer sign rendering is the subject of the separate add-breakpoints-signs proposal, which will own its own extmark-pool concern.

Watches-view–specific CI lints (in addition to the shared view rules above):

  1. watches-no-parallel-hl rejects any reference to a highlight group whose name begins with DapperWatches. Watches render values through the shared DapperVariables* family installed by the conventions module; a parallel family would split the type-aware visual surface.
  2. watches-no-fs-api rejects vim.fn.readfile, io.open, io.read, and vim.fn.system inside lua/nvim-dapper/views/watches/. The only permitted read/write path is the vim.uv.fs_* shape used by persist.lua. The blanket ban exists because the lint cannot statically tell which path is being read.

The DAP-isolation lint is narrowed so that files under lua/nvim-dapper/views/ MAY use the exact call shape require("dap").session():request(<command>, <args>, <cb>) to issue DAP requests for the data they render. Any other access to require("dap").session() from a view file (storing it in a local, reading a field, calling any method other than :request) remains rejected, as does any reference to dap.listeners anywhere outside lua/nvim-dapper/session/.

Keybinds are owned by the dedicated module lua/nvim-dapper/keymaps/. No file outside that module — view, panel, top-level init, or otherwise — MAY call vim.keymap.set, vim.api.nvim_buf_set_keymap, vim.api.nvim_set_keymap, or any :map-family ex-command. The keymap-isolation lint enforces this across all lua/nvim-dapper/ files. Views publish behaviour through the named-action surface (view.action(name, handler) / view.dispatch(name, ctx)); the keymaps module is the single installer that turns actions into keys via require("nvim-dapper.keymaps").apply_view_keymaps and apply_global_keymaps. See docs/keymaps.md for the default tables and the override snippet.

§3.4 enforcement (canonical example for sibling Group A views)

The variables view ships a full request-count integration test at tests/views/variables/integration_spec.lua (§13.1). It drives the view through session_started → stopped (mixed normal / expensive / lazy / object children) → expand → page → frame_changed → expand → continued → session_terminated, and asserts the cumulative scopes / variables request count at every milestone. Sibling Group A views (stack, breakpoints, watches) SHOULD ship an analogous spec — the same harness (tests/helpers/dap_stub.lua + the mock session adapter) makes it cheap to add. Treat the integration spec as load-bearing: a regression there means a view started enumerating data the user didn't ask for.

The watches view ships its §3.4 enforcement spec at tests/views/watches/integration_spec.lua. It drives the view through setup({}) with a persisted 3-watch list → stopped (asserting exactly 3 evaluate requests, all with context = "watch") → expand on a watch whose evaluate result has variablesReference > 0 (exactly 1 variables request, page-sized to the configured page_size) → load-more on the synthetic affordance row (exactly 1 paged variables request) → frame_changed (3 fresh evaluate requests; the prior child-tree state is discarded, NOT refetched) → add-watch (exactly 1 evaluate for the new slot, not a full-list re-evaluate) → edit-watch-expression (exactly 1 evaluate for the edited slot, with the prior child-tree state discarded) → remove-watch (zero DAP requests) → continued (zero DAP requests, values greyed) → session_terminated (zero DAP requests; with the default clear_on_terminate = false the last values remain visible but greyed).

The two load-bearing differences from variables-view's §3.4 spec:

  1. The per-watch evaluation cost is O(N watches) per stopped/ frame_changed, not O(scopes). The serial-drain rule (one DAP request in flight at a time) makes this acceptable for short lists; users with very long watch lists must remove watches.
  2. List-mutation actions (add-watch, remove-watch, edit-watch-expression) bump the generation counter even though no session event fired. This is what makes the stale-response rule cover the case where the user edits an expression while its evaluate is in flight: the gen-old response is dropped at completion time, exactly as it would be after a fresh stopped.

The stack view ships its §3.4 enforcement spec at tests/views/stack/integration_spec.lua. It drives the view through session_started → stopped for thread T with 200 frames (exactly 1 stackTrace request, startFrame = 0, levels = eager_window) → load-more on the synthetic affordance row (exactly 1 paged stackTrace request with startFrame = next_offset, levels = page_size) → stopped for a non-focused thread U while T is still stopped (zero stackTrace requests for U — non-focused threads auto-fetch only on expand) → expand on U's row (exactly 1 stackTrace request for U) → frame_changed via session.focus_frame(F) to an in-page frame (zero stackTrace requests; emphasis moves) → thread_changed to U (exactly 1 stackTrace request for U; previously focused thread's row collapses) → continued (zero stackTrace requests; tree greyed) → session_terminated (zero stackTrace requests; tree cleared, the no-session empty state rendered).

The stack-view §3.4 spec mirrors the variables-view canonical example in shape but watches for stack-specific traps: lookahead must never auto-expand a thread row (collapsed threads have no frames fetched), a frame_changed to a frame beyond the rendered eager-window must NOT trigger an auto-fetch (the user reveals the new frame via load-more), and the manager's internal after.stackTrace listener is the sole source of the per-thread known-frame set used by focus_frame validation — the view itself never pushes frame ids to the manager.

The following identifiers are reserved by the REPL view (add-repl-view) and MUST NOT be reused by other modules:

  • Session-manager subscriber id: "repl-view".
  • Extmark-pool concern name: "repl-view".
  • Panel view id: "repl" / human name: "REPL".
  • Filetype: dapper-repl.
  • Named actions published by the REPL view module: the six universal conventions (expand, collapse, expand-all-children, collapse-all-children, jump-to-source, retry), load-more (paged child-fetch affordance for expanded evaluate results), and four view-local actions: submit, recall-previous, recall-next, clear-transcript.
  • No on-disk state. The REPL view writes no files under stdpath('data')/nvim-dapper/. Both transcript and history are in-process and ephemeral. The CI lint (repl-no-fs-api) enforces this by banning all filesystem-read and filesystem-write primitives unconditionally inside lua/nvim-dapper/views/repl/.

The following identifiers are reserved by the output view (add-output-view) and MUST NOT be reused by other modules:

  • Session-manager subscriber id: "output-view".
  • Extmark-pool concern name: "output-view".
  • Panel view id: "output" / human name: "Output".
  • Filetype: dapper-output.
  • Named actions published by the output view module: jump-to-source (universal convention) and clear-output (view-local).
  • Highlight groups: DapperOutputStdout (default-linked to Normal), DapperOutputStderr (default-linked to DiagnosticError), DapperOutputConsole (default-linked to Comment), DapperOutputTelemetry (default-linked to NonText).
  • No on-disk state. The output view writes no files under stdpath('data')/nvim-dapper/. The CI lint (output-no-fs-api) enforces this by banning all filesystem-read and filesystem-write primitives inside lua/nvim-dapper/views/output/.

Reserved Future Hook

A forthcoming Phase 1 stack/threads view will need user-driven thread selection: manager.set_focused_thread(session_id, thread_id). Do not implement this ahead of that proposal. The semantics are documented in design.md (§D8, "Reserved future hook") so the proposal can land without re-litigating focus semantics.