* Shrink binary: trim image decoders and swap color-eyre for eyre
Two dependency changes that cut the default `sk` binary from 13.6 MiB to
8.55 MiB (-5.06 MiB, -37%) with no loss of core functionality:
- image: build the `image` crate with only the common decoders (png,
jpeg, gif, webp) instead of its full default format set, and drop
ratatui-image's `image-defaults`. This removes AVIF encoding (ravif,
avif-serialize), OpenEXR (exr), TIFF, QOI and other decoders that are
irrelevant to terminal image previews. Previewing those formats now
falls back to the normal command preview.
- error handling: replace color-eyre with plain eyre. color-eyre only
provided colored panic/error backtraces; skim used none of its
Section/Help extension APIs. This drops the backtrace/gimli/addr2line/
color-spantrace stack. `color_eyre::install()` is no longer needed.
Tests, benches and examples are migrated from color_eyre to eyre so the
crate is fully removed from the dependency graph.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BQtxeCS4gM7dumghqmNgST
* chore: fmt
* docs: ARCHITECTURE.md
* chore(flake): add cargo-bloat
---------
Co-authored-by: Claude <noreply@anthropic.com>
* feat: add cargo-fuzz targets for hand-rolled text parsers
Skim's most panic-prone code is the hand-written byte/char-index
bookkeeping over untrusted input: ANSI stripping, --nth/--with-nth field
extraction, the fuzzy matching algorithms, the query->engine->match
pipeline, and the --bind key-map parser. Add five cargo-fuzz (libFuzzer)
targets covering each, asserting real invariants (char-boundary safety,
monotonic index mappings, match indices in bounds) rather than just
catching panics, plus a CI workflow that runs them on every push/PR
touching src/ or fuzz/ and a longer nightly session via cron.
* ci: wire fuzz targets into the existing test matrix
Replace the standalone fuzz.yml workflow with a `fuzz` job in the main
test.yml matrix, running each of the 5 fuzz targets for 60s (5 minutes
total per CI run) alongside nextest/clippy/msrv.
* ci: remove standalone fuzz workflow
Superseded by the fuzz job now in test.yml.
* ci: run fuzz job as a single job on the platform matrix
Reuse the existing linux/macos/windows matrix instead of a separate
per-target matrix; run all 5 fuzz targets sequentially in one step
(60s each, 5 minutes total). cargo-fuzz doesn't support Windows, so
the fuzzing step is skipped there while still installing the
toolchain for consistency with the rest of the matrix.
* ci: reuse existing yaml anchors in the fuzz job
Use *toolchain instead of a bespoke nightly-install step, matching
the coverage job's pattern of letting `cargo +nightly` auto-provision
the toolchain on demand.
* nix: add cargo-fuzz to the tests devShell
Makes cargo-fuzz available via `nix develop` alongside the other test
tooling, matching what CI installs for the fuzz job.
* ci: install cargo-fuzz via taiki-e/install-action
Matches how the other CI-only cargo subcommands (nextest, cargo-msrv,
cargo-llvm-cov) are installed, and is faster than compiling it from
source with cargo install.
* ci: force the native host target for cargo-fuzz
cargo-fuzz was picking a statically-linked musl target on the runner,
which fails since ASan can't link against a static libc. Pass the
actual host triple (from `rustc -vV`) explicitly so the sanitizer
build always targets the dynamically-linked gnu/darwin toolchain.
* ci: skip the whole fuzz job on windows
Rather than skipping just the fuzzing step, exclude the job entirely
for the windows-latest matrix entry via a job-level `if`, since
cargo-fuzz/libFuzzer has no Windows support.
* ci: gate the fuzz job with runner.os instead of matrix.os
Matches the runner.os-based conditionals already used elsewhere in
this workflow (the linux/macos dependency install steps) rather than
comparing matrix.os directly.
* ci: enable the fuzz job on windows without ASan
cargo-fuzz does support Windows, but AddressSanitizer on the MSVC
target needs the separate "C++ AddressSanitizer" VS component plus a
PATH tweak for its DLL, which this runner doesn't have configured.
Rather than skip the job, disable the sanitizer on Windows only
(--sanitizer none) and keep coverage-guided fuzzing there; our
targets assert via plain Rust panics so they don't depend on ASan.
* test: assert exact char_idx correctness in ansi_strip fuzz target
Replace the bounds-only char_idx check with an exact-equality check
against the char position of byte_pos in the original string. This
subsumes (and is stronger than) the monotonicity CodeRabbit flagged,
since strictly-increasing byte positions on char boundaries always
imply strictly-increasing char positions.
* ci: skip windows in fuzz job, scope job permissions
CI showed the Windows fuzz build fails with a real MSVC linker error
(LNK2001: unresolved __start/__stop___sancov_pcs) even with
--sanitizer none: MSVC's linker doesn't synthesize the section
boundary symbols that libFuzzer's coverage instrumentation requires,
so this is unrelated to the earlier ASan/PATH discussion and isn't
fixable by a sanitizer flag. Skip Windows via step-level `if`
(job-level `if` can't reference runner/matrix contexts). Also add an
explicit contents:read permissions block to the job.
* fix(fzy): fix unicode case-folding inconsistency causing overflow panic
The new fuzzy_match fuzz target found a real crash: FzyMatcher panicked
with "attempt to multiply with overflow" on choice="ű\0\0\0\u{1e}ű",
pattern="Űű".
Root cause: fzy_score's case-insensitive comparison used
char::to_ascii_lowercase (a no-op on non-ASCII letters like Ű/ű), while
the shared cheap_matches() prefilter (and the other matchers) use the
Unicode-aware char_equal(). This let cheap_matches accept a pattern
that fzy_score's own DP could then never actually align, since needle
char 'Ű' never matched any haystack position under ASCII-only folding.
The DP's SCORE_MIN sentinel ("impossible") isn't an absorbing element
under plain integer addition, so the broken alignment accumulated to a
value close to, but not exactly, SCORE_MIN, which then overflowed on
the final *SCORE_TO_SKIM conversion since only the exact sentinel was
special-cased.
Fix is_match to use the shared char_equal() so fzy.rs's case folding
matches cheap_matches and the other two matchers (skim.rs, clangd.rs
already do this). Also switch internal_to_skim_score to saturating_mul
as defense in depth, since fzy_score structurally always returns
Some(..) and has no other way to signal "no valid alignment" to the
caller.
* ci: try lld-link to get windows fuzzing working (no ASan)
MSVC ASan is documented broken on GitHub-hosted Windows runners
(actions/runner-images#8891 — ASan binaries crash with
STATUS_DLL_INIT_FAILED even with the runtime DLL on PATH, unresolved
upstream), so it's not viable here regardless of our config. Separately,
the sancov coverage instrumentation cargo-fuzz needs doesn't link with
MSVC's link.exe at all (missing __start/__stop section symbols).
Try switching the Windows leg to rustc's bundled LLD linker
(-C linker-flavor=lld-link -C link-self-contained=+linker) with
--sanitizer none, to at least get coverage-guided fuzzing (no ASan)
working there. Validating live against this PR's CI.
* ci: revert windows fuzzing attempt, exclude it again
The lld-link experiment ruled out the remaining option: LLD's COFF
driver hit the exact same missing __start/__stop___sancov_* symbols as
MSVC's link.exe. This confirms the section-boundary-symbol synthesis
libFuzzer's coverage instrumentation needs simply isn't implemented
for the COFF/Windows target in current LLVM/rustc — an upstream gap,
not a linker choice or CI config problem. Combined with MSVC ASan
being separately documented broken on GH-hosted Windows runners
(actions/runner-images#8891), there's no remaining avenue to try from
the workflow side. Back to excluding Windows from the fuzz job.
* ci: try windows fuzzing with default sanitizer + msvc dev env
Previous Windows attempts both used --sanitizer none, which removes
the ASan runtime that (on Windows) supplies the __start/__stop section
symbol shims libFuzzer's coverage instrumentation needs -- neither
linker synthesizes those on COFF. That's very likely why they failed
to link. Revert to the default sanitizer (address) and add
ilammy/msvc-dev-cmd to put the MSVC ASan DLL directory on PATH, per
the cargo-fuzz Windows setup guide and actions/runner-images#8891.
Testing live whether this builds, and whether the previously-reported
STATUS_DLL_INIT_FAILED runtime crash still reproduces on this runner
image.
* ci: point cargo at the real MSVC linker on windows
msvc-dev-cmd correctly set up Path, but Git Bash prepends its own
usr/bin ahead of it, so cargo picked up Git's coreutils `link`
(hardlink tool) instead of MSVC's link.exe. Set
CARGO_TARGET_X86_64_PC_WINDOWS_MSVC_LINKER explicitly using
VCToolsInstallDir (set by msvc-dev-cmd) to sidestep PATH ordering
entirely.
* fix(event): parse_action returns None instead of panicking on missing args
The keymap_parse fuzz target found a real crash: KeyMap::from("/:if-")
panicked ("no arg specified for event if-") since parse_action's
documented behavior was to panic on if-* actions missing their
argument, even though the function already returns Option<Action> and
every other malformed/unrecognized action already resolves to None
via the surrounding parse_action_chain/KeyMap plumbing.
Fixed that case, and while checking for the same pattern elsewhere in
the function found four more reachable panics of the same kind
(add-char, execute, execute-silent, set-preview-cmd, set-query parsed
without their required argument), confirmed each panics via a small
repro before fixing. All now return None like every other malformed
action, consistent with the function's existing contract, instead of
panicking on user-supplied --bind strings.
* ci: add a single aggregate status check for branch rulesets
Add a ci-success job that depends on every other job in the workflow
and fails if any of them failed or were cancelled (tolerating
deploy-coverage-page's expected skip off master). This gives branch
protection / repository rulesets one stable check name to require,
instead of enumerating every matrix leg (nextest (linux), fuzz
(windows), etc.) individually.
---------
Co-authored-by: Claude <noreply@anthropic.com>
* refactor: move filter mode into run_with via should_enter
Remove the standalone `filter` function from main.rs and integrate
filter mode into the main `run_with` pipeline. When `options.filter`
is set, `should_enter` now waits for all items to be processed and
returns false (skipping TUI), and `App::results` returns all matched
items. This unifies filter mode with the rest of the codebase so it
benefits from all other flags (sorting, tiebreaks, etc.).
https://claude.ai/code/session_01T7pa2RRX85MvBtnH7PWtmw
* perf: optimize filter mode to match single-pass performance
Three changes that eliminate the performance regression from routing
filter mode through run_with:
1. should_enter(): Wait for reader to finish BEFORE starting matcher,
then run matcher exactly once. The old polling loop called
restart_matcher() repeatedly, each time resetting the ItemPool taken
counter and re-processing all items from scratch. With 1M items
arriving in batches, items were matched multiple times.
2. matcher.run(): Remove unnecessary .enumerate() (index was discarded)
and remove item.clone() — into_par_iter() yields owned values so the
Arc can be moved directly into MatchedItem.
3. App::results(): In filter mode, drain items instead of cloning to
avoid 271K MatchedItem clones + Arc allocations.
Benchmark (1M file paths, query "test", 10 runs):
Old standalone filter: 5.306s ± 0.085s
New unified filter: 4.932s ± 0.099s (1.08x faster)
https://claude.ai/code/session_01T7pa2RRX85MvBtnH7PWtmw
* perf: make restart_matcher incremental when force=false
When force=false, skip the item pool reset so take() only returns new
(untaken) items. The callback merges new sorted matches into the
existing sorted result list using an O(n+m) merge, instead of
replacing it. This fixes the root cause: previously every
restart_matcher call re-processed ALL items from scratch because
reset() set the taken counter to 0.
This benefits all polling callers (filter, select-1, exit-0, sync),
not just filter mode. Items arriving in batches are now each matched
exactly once, and matching overlaps with I/O since batches are
processed as they arrive.
When force=true (query changed), behavior is unchanged: full reset
and re-match.
Reverts the filter-specific workaround from the previous commit in
favor of this general fix; the original polling loop in should_enter
is now efficient.
Benchmark (1M file paths, query "test", 10 runs):
Old standalone filter (master): 4.566s ± 0.055s
Incremental restart_matcher: 3.654s ± 0.035s (1.25x faster)
https://claude.ai/code/session_01T7pa2RRX85MvBtnH7PWtmw
* refactor: extract sorted_merge into MatchedItem method
Move the two-sorted-list merge from the restart_matcher closure into
MatchedItem::sorted_merge() for clarity and reusability. The method
merges two Vec<MatchedItem> lists that are already sorted by rank
into a single sorted Vec in O(n+m) time.
https://claude.ai/code/session_01T7pa2RRX85MvBtnH7PWtmw
* fix: preserve incremental matches across render cycles
When restart_matcher runs with force=false, the callback marks results
with a MergeStrategy so the render loop knows how to combine them with
existing items. Previously, render's .take() would drain processed_items
to None, and the next incremental callback would create a fresh batch —
causing the render to replace all items with just the latest batch,
losing previously matched items.
Now:
- Replace: full re-match (force=true), replaces item list entirely
- SortedMerge: incremental sorted results, merged into item_list.items
- Append: incremental unsorted results (--no-sort), appended
This fixes match count consistency in interactive mode. With 1M items
and query "test", match count is now 290,083 on every run (matching
fzf's consistency), vs wildly varying counts before (min 13, max 56,950).
https://claude.ai/code/session_01T7pa2RRX85MvBtnH7PWtmw
* chore: fmt & clippy
---------
Co-authored-by: Claude <noreply@anthropic.com>
This PR has grown beyond its initial scope due to me over-optimizing everything, but it leads to:
Paving the way for future actually interactive previews
Consistently better performance than fzf in our bench thanks to thread and concurrency optimizations as well as the use of kanal for the items channels
Given the scope, I'm marking this as breaking because:
setting wrap in the preview window layout disables the pty since we don't want to manipulate the raw buffer to word-wrap it manually
kanal channels work slightly differently and might break library usage, even though switching to them did not require any modifications of the examples so it's unlikely that users will see anything break
* fix: force cwd for preview
* fix: correctly set cwd & kill pty child in the right order
* fix: use std threads & reopen new pty for each preview
* feat: use tui-term for displaying
* feat: scroll in pty
* fix: make nested skim previews work
* fix: clippy mistake
* feat: reactive preview triggering
* chore: generate completions & manpage
* chore: optimizations & thread cleanup
* chore: use kanal for faster channels
* fix: tests
* fix: only send items if the matcher hasn't been killed in the meantime (#947)
* tests: add coverage
* tests: fix bin path with coverage
* tests: upload tests to codecov
* chore: make pty opt-in through preview-window
* chore: generate completions & manpage
---------
Co-authored-by: Skim bot <skim-bot@skim-rs.github.io>
