Code Review

Pending code quality improvements and refactoring opportunities.

Note: file/line references are best-effort and can drift as the code changes; treat line numbers as approximate.

Large Functions

• src/app/render.rs:5 — top-level render runs ~450 lines covering layout calculation, plugin hook firing, file explorer rendering, and status/prompt UI. The amount of responsibility in one function hurts readability and testing; factoring into helpers (layout, plugin hook prep, explorer rendering, status/prompt rendering) would make regressions easier to spot.
• src/input/keybindings.rs:376 — Action::from_str is a ~260-line string-to-enum match. A data-driven table would reduce boilerplate and avoid missing new actions when added to the enum.
• src/input/commands.rs:89 — get_all_commands is a ~540-line literal list in one function. This is brittle to maintain (hard to diff/review additions) and couples command metadata to code; consider a data table or config-driven source with tests for completeness.
• src/primitives/highlighter.rs:78 — highlight_config is ~450 lines of repetitive per-language setup; moving to a data table (language -> queries/config) would reduce duplication and make it harder to forget highlight names when adding languages.
• src/config.rs:566 — default_menus is a 420-line literal definition. Consider moving menu data to structured config or a table to make changes easier to diff/test and to avoid bloating code with data.
• src/view/viewport.rs:521 — ensure_visible runs ~230 lines of layout math and clamping in one method; breaking into smaller helpers (e.g., horizontal/vertical logic, scroll computations) would make correctness checks and future changes safer.
• src/input/keybindings.rs:1196 — format_action spans ~200 lines mirroring from_str; another sign a data-driven action registry would reduce duplication and risk of drift.
• src/view/ui/view_pipeline.rs:134 — next is a 160+ line iterator step that handles multiple line types and wrapping; decomposing per-case helpers would make it easier to verify wrapping and newline behaviors.
• src/model/buffer.rs:575 — get_text_range_mut is a 160+ line method with intertwined chunk handling, file streaming, and cache management. The dense control flow increases the risk of off-by-one or range bugs; extract helpers per buffer variant (loaded vs unloaded) and centralize bounds checks.
• src/services/plugins/runtime.rs:1861/1967/2066/2143/2159/etc. — repeated pending_responses.lock().unwrap() (and other mutex unwrap()s) in ops will panic if the mutex is poisoned by a plugin panic. These should handle poisoning and return an error to the plugin rather than crashing the runtime.
• src/view/ui/file_explorer.rs:148 — renderer calls view.tree().get_node(node_id).expect("Node should exist"). A desynced tree (e.g., after FS errors or refresh races) will panic the UI; prefer graceful fallback or placeholder row.
• src/view/file_tree/tree.rs:138 — expand_node uses self.get_node(id).unwrap() while doing async FS reads. If the node was removed between calls, this panics instead of returning an IO error; return a proper error for missing nodes to avoid crashing the explorer.
• src/view/ui/status_bar.rs:149 — render_status pulls cursor info with ad-hoc line iteration and cached line numbers in a long method; consider extracting helpers for cursor position and diagnostics summary to reduce the ~200-line render method complexity.
• src/model/cursor.rs:220 — primary()/primary_mut() use expect("Primary cursor should always exist"). If a bug ever removes the primary cursor (e.g., during multi-cursor deletion), the editor will panic. Prefer returning an error or recreating a primary cursor to keep the UI alive.
• src/model/marker_tree.rs:494/509/512/517/etc. — AVL rotations and queries use unchecked unwrap() on child pointers. Any tree corruption (e.g., from earlier logic bugs) will panic. Consider defensive checks or debug assertions gated for release builds to avoid crashing the editor.
• src/view/split.rs:162 — ensure_layout returns self.layout.as_ref().unwrap(). If callers forget to call ensure_layout before get_layout usage, rendering will panic. Consider returning Option<&Layout> or asserting via a Result to make misuse harder in production.
• src/view/ui/split_rendering.rs:1642/1838 — rendering paths unwrap() optional highlight colors/positions when overlays and syntax spans overlap. If the lookups ever return None (e.g., missing semantic span), the renderer will panic; handle None with defaults to keep the UI resilient.
• src/view/prompt.rs:974 — selected_text().unwrap() in selection paths; production prompt operations should avoid panics when no selection exists (e.g., racey UI updates or plugin-driven prompt changes).
• src/input/buffer_mode.rs: ModeRegistry and BufferMode rely on string names without validation; missing mode references during inheritance or resolve silently stop traversal, which can mask misconfigurations. Consider explicit errors/logging for unknown parent/mode to aid debugging.
• src/view/viewport.rs:895 — ensure_cursors_visible unwraps min/max cursor positions; if called with an empty cursor list (due to upstream logic bugs), it will panic. Guard against empty input to keep scrolling resilient.
• src/view/ui/suggestions.rs:80/87 — width calculations use .max().unwrap_or(0) on suggestion lists; if any suggestion text/keybinding is extremely wide, width computation and rendering alignment are packed into the main render function. Consider extracting layout calculation and handling unreasonably wide strings to avoid overflow/panic in rendering math.
• src/services/signal_handler.rs:23/104 — global backtrace storage uses Mutex::lock().unwrap(); any poisoning (e.g., panic during signal handling) will panic again. Signal handlers should avoid unwraps and degrade gracefully to prevent double panics during shutdown.
• src/services/async_bridge.rs:300/324/329/356/... — multiple unwrap()s on channel send/recv in the bridge; if the channel is closed (e.g., runtime shutdown), the bridge will panic instead of allowing a clean exit. Return errors and let the main loop terminate gracefully.
• src/services/process_limits.rs:405/406/459/476/496/503 — heavy use of unwrap() around serialization and system resource reads; a failure to read system memory or serialize limits will panic instead of bubbling a configuration error. Prefer error propagation.
• src/services/plugins/api.rs & event_hooks/process: widespread RwLock::unwrap() and channel unwrap()s. A poisoned lock or closed channel will panic plugins and potentially the editor. Replace with error paths that surface to the plugin caller or log-and-drop to keep the host stable.
• src/input/command_registry.rs — plugin command registration/history uses RwLock::unwrap() in production paths; a poisoned lock (plugin panic) will crash the editor. Handle lock errors and surface failures to plugins.
• src/input/actions.rs:68 — block selection reads cursor.block_anchor.unwrap() when in block mode; if state enters block mode without an anchor, movement will panic. Guard against None to keep editing stable.
• src/services/plugins/process.rs:132/166/195/226 — plugin process bridge uses recv().unwrap(); closed channels during shutdown will panic the host. Treat channel closure as cancellation to allow clean teardown.
• src/services/plugins/process.rs — blocking recv() loops have no timeout/backoff; a misbehaving plugin can hang the thread. Consider async or timeout-aware handling to avoid host hangs.
• src/services/async_bridge.rs — uses unbounded std::sync::mpsc channels with no backpressure or eviction; a burst of async messages (e.g., LSP floods) can grow memory unbounded. Consider bounded channels or dropping strategies plus explicit error propagation on lock/channel failure.
• src/services/plugins/event_hooks.rs — apply_event_with_hooks appears to be legacy/test-only and is not part of the current editor edit path (which applies edits via Editor::apply_event_to_active_buffer and queues plugin hooks asynchronously). If this module becomes production-critical again, it should isolate hook execution (catch_unwind/log) and avoid panics on poisoned locks.
• src/app/plugin_commands.rs — some plugin text-editing commands (InsertText, DeleteRange, InsertAtCursor) mutate buffers without going through Editor::apply_event_to_active_buffer, which can bypass cross-cutting concerns (LSP change notifications, search highlight clearing, cursor/view sync, plugin after-insert/delete hooks). Consider routing “real edit” plugin commands through the centralized apply path, or document them as “raw/unsafe” edits.
• src/view/ui/tabs.rs — render_for_split packs tab width calculation, scroll computation, and rendering into one long function; consider splitting into layout calculation and render steps to improve readability and reduce bugs in scroll math.
• src/services/process_limits.rs — applies limits via cgroups/setrlimit with many unwraps noted; also lacks clear fallback/reporting when limits can't be applied (e.g., non-Linux or missing cgroup perms). Return actionable errors so callers can decide to continue without limits.

Test Code

• src/view/ui/menu.rs:655/776/811/830 — menu parsing/rendering tests use unwrap(); production parsing should handle malformed plugin menu contributions gracefully.
• src/services/fs/slow.rs — tests use TempDir::new().unwrap()/read_dir().await unwraps; ensure production slow backend surfaces IO errors cleanly.
• src/view/popup.rs — selection tests unwrap() the selected item; production popup handling should guard empty lists.