mirror of
https://github.com/lotabout/skim.git
synced 2026-09-10 07:16:23 -04:00
d7e294eb7d
* fix(tui): stop input reader and give execute() children their own tty Interactive/ncurses programs run via an `execute()` action (e.g. `ncdu`) would freeze after a few keystrokes. Two independent problems caused it: 1. skim's background input reader (the `EventStream` task started in `Tui::start`) kept reading the terminal while the child ran, so skim and the child raced for keystrokes on the same tty — roughly half the keys were stolen from the child. 2. The child inherited skim's stdin (fd 0), which is a pipe whenever items are piped in (`find | sk`). An interactive child then had no keyboard source at all. Fix both: - Add `Tui::stop_and_join`, which cancels the event-pump task and blocks until it has dropped its `EventStream`, guaranteeing skim has released the terminal before the child starts. `run_foreground` calls it before running the child and `Tui::start` after. - Give the child its own stdin opened from the controlling terminal (`/dev/tty`, or `CONIN$` on Windows), falling back to inheriting skim's stdin if that fails. Because running a foreground process needs the `Tui` (which `handle_action` does not have), `Execute` now only expands the command and returns a new `Event::RunExecute`, which `handle_event` runs via `run_foreground`. This mirrors the existing `RunPreview` pattern. `execute-silent` is unchanged. Update ARCHITECTURE.md (event dispatch table, terminal lifecycle, and cross-reference line numbers) and add tests. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBQ1chjHyN3JNTWqMMED44 * test(execute): add tmux e2e test; fix reader restart and post-execute repaint Add a tmux-based integration test (tests/execute.rs, unix-gated so it runs on the Linux and macOS CI legs) that drives an interactive child through an `execute` action and asserts it keeps receiving keystrokes, then that skim is interactive again once the child exits. It covers both the fullscreen and inline (`--height`) layouts. Writing the test surfaced two bugs in the execute reader-suspend work that unit tests could not catch: 1. Reader never resumed. `Tui::stop_and_join` cancels the shared `CancellationToken`, but `Tui::start` reused that same token — and a cancelled token stays cancelled — so the respawned reader observed the cancellation immediately and exited without reading input. `start` now installs a fresh token on every call (also fixing the latent restart-while-running path). 2. Post-execute repaint hung when stdout was redirected. The repaint went through `Event::Redraw` → `tui.clear()`, and ratatui's `Terminal::clear` queries the cursor position, which crossterm writes to stdout via `ESC [ 6 n`. skim renders to stderr and its stdout is routinely redirected (`sk > file`), so the query reached no terminal, got no reply, and stalled the UI for seconds before erroring out. Replace it with `Tui::force_full_redraw`, which resets ratatui's diff buffers for a full repaint with no cursor query and works for both fullscreen and inline viewports. Update ARCHITECTURE.md and the cross-reference table accordingly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QBQ1chjHyN3JNTWqMMED44 * ci: fix matrix * fix: pop kitty keyboard flag before entering execute * chore: remove duplication in backend.rs * chore: refactor * fix: kitty maintains a different set of flags in alt screen --------- Co-authored-by: Claude <noreply@anthropic.com> |
||
|---|---|---|
| .. | ||
| fuzz_targets | ||
| .gitignore | ||
| Cargo.lock | ||
| Cargo.toml | ||
| README.md | ||
Fuzzing
This directory contains cargo-fuzz
(libFuzzer) targets for skim's hand-written, untrusted-input-facing parsers:
text that flows in from stdin, --ansi sequences, --nth/--with-nth field
specs, the search query syntax, and --bind key maps. These are exactly the
places where skim does manual byte/char-index bookkeeping on attacker- or
data-controlled strings, which is the most panic-prone code in the project.
Targets
| Target | Exercises |
|---|---|
ansi_strip |
helper::item::strip_ansi — ANSI escape stripping & byte/char index map |
field_extract |
field::{FieldRange, get_string_by_field, parse_matching_fields, parse_transform_fields} — --nth/--with-nth |
fuzzy_match |
fuzzy_matcher::{skim, fzy, clangd} — the fuzzy matching algorithms |
query_match |
Matcher::create_engine_factory + DefaultSkimItem — the full query → engine → match pipeline (exact/regex/AND-OR/fuzzy, with ANSI) |
keymap_parse |
binds::KeyMap — the --bind key-map parser |
Each target asserts more than "doesn't panic" where a cheap invariant is available (e.g. reported match indices must be valid char indices into the matched text, index mappings must stay monotonic and land on char boundaries).
Running
Install cargo-fuzz (requires a nightly toolchain):
cargo install cargo-fuzz
Run a target:
cargo +nightly fuzz run ansi_strip
Run for a bounded time (useful in CI or for a quick check):
cargo +nightly fuzz run query_match -- -max_total_time=60
Reproducing a crash
cargo fuzz run writes failing inputs to fuzz/artifacts/<target>/. Replay one with:
cargo +nightly fuzz run <target> fuzz/artifacts/<target>/crash-<hash>
Adding a target
Add a new fuzz_targets/<name>.rs, register it in fuzz/Cargo.toml's
[[bin]] list, and prefer asserting a real invariant of the function under
test (bounds, monotonicity, round-tripping) rather than only catching panics.