Fuzzy finder and global file pinning system#25
Merged
JackDerksen merged 12 commits intomainfrom Apr 24, 2026
Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdds a fuzzy-matching API to redox-core, rewrites crate READMEs, and implements a full TUI finder + pinboard feature across state, input, rendering, styling, and tests, integrating fuzzy scoring, file discovery, pin persistence, previewing, and popup UI into the editor loop. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Input
participant EditorState as EditorState
participant FS as FileSystem
participant Fuzzy as FuzzyMatcher
participant Render as Renderer
User->>Input: Space+Space (open finder)
Input->>EditorState: OpenFinder
EditorState->>FS: list files under launch_dir (respect .gitignore)
FS-->>EditorState: entries
EditorState->>Fuzzy: fuzzy_match_ranges(entries, empty_query)
Fuzzy-->>EditorState: matches
EditorState->>Render: show FinderPopup(results)
Render->>User: display finder popup
User->>Input: type query chars
Input->>EditorState: FinderChar(c)...
EditorState->>Fuzzy: fuzzy_match_ranges(path_label, query)
Fuzzy-->>EditorState: FuzzyMatch + highlights
EditorState->>EditorState: score, sort, update selection
EditorState->>FS: read preview bytes for selection
EditorState->>Render: update FinderPopup (highlights + preview)
Render->>User: updated popup
User->>Input: Enter
Input->>EditorState: FinderEnter
EditorState->>EditorState: open selected entry (set transient origin if pinned)
EditorState->>Render: close popup, switch to editor view
sequenceDiagram
participant User
participant Input
participant EditorState as EditorState
participant Config as ConfigFile
participant FS as FileSystem
User->>Input: Ctrl+Shift+P (quick pin)
Input->>EditorState: QuickPinCurrentFile
EditorState->>EditorState: open PinSelectorPopup
EditorState->>Config: read pinned_files.json (or legacy)
Config-->>EditorState: pinned list
User->>Input: Shift+3 (select slot)
Input->>EditorState: AssignPinSlot{slot}
EditorState->>FS: confirm path exists
EditorState->>Config: write pinned_files.json (atomic)
Config-->>EditorState: write complete
EditorState->>EditorState: refresh finder merge with pinned list
EditorState->>EditorState: close PinSelectorPopup
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
crates/redox-tui/src/ui/widgets/toast.rs (1)
223-230: Avoid hardcoded prompt-string coupling for toast styling.The dimmed styling for the
press yconfirmation prompt is keyed off the exact trimmed message literal. Any future tweak to the prompt wording (e.g. capitalisation, punctuation, or adding context) will silently drop the dim styling with no test/compile signal. Consider carrying the "dim" intent on the toast message itself (e.g. a lightweightStatusMsg { text, style: StatusStyle::Dim }or a separatehintfield onEditorState) and letting the producer decide, rather than the renderer pattern-matching on content.Not a blocker — flagging since more confirmation prompts are likely to show up as the finder/pinboard surfaces mature.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/redox-tui/src/ui/widgets/toast.rs` around lines 223 - 230, The renderer currently keys dim styling on the literal string "press y" which is fragile; instead add an explicit style flag on the toast message type (e.g., extend Toast/ToastMessage or introduce StatusMsg { text, style: StatusStyle } with StatusStyle::Dim/Normal or add a hint field on EditorState), update producers to set StatusStyle::Dim for confirmation prompts, and change the renderer code that calls window.write_str_colored (and related functions like write_str_colored) to branch on the new message.style/hint rather than matching the trimmed text; update any affected constructors/usages to supply the style so the dim behavior is driven by data, not content.crates/redox-tui/src/lib.rs (1)
59-65: Consider extracting the popup-overlay mode check to avoid drift.The same
Command | Search | Finder | PinSelectmatcher is now used for bothpopup_overlay_active(line 59) and for suppressing the perf popup (line 308). If another overlay mode is added later, it's easy to update one site and forget the other. A small helper onEditorMode(e.g.has_popup_overlay()) would keep them in sync.♻️ Optional refactor sketch
// In crates/redox-tui/src/app/state.rs impl EditorMode { pub fn has_popup_overlay(self) -> bool { matches!( self, EditorMode::Command | EditorMode::Search | EditorMode::Finder | EditorMode::PinSelect ) } }Then both sites become
state.mode.has_popup_overlay().Also applies to: 307-320
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/redox-tui/src/lib.rs` around lines 59 - 65, Add a helper method on EditorMode (e.g. pub fn has_popup_overlay(self) -> bool) that returns matches!(self, EditorMode::Command | EditorMode::Search | EditorMode::Finder | EditorMode::PinSelect), then replace the inline match in the popup_overlay_active calculation (the expression that also checks state.explorer_popup().is_some()) with state.mode.has_popup_overlay(), and likewise replace the duplicate matcher used when suppressing the perf popup with state.mode.has_popup_overlay() so both sites remain in sync.crates/redox-tui/src/app/state/finder.rs (1)
25-25:query_erroris carried through the state/popup but never populated.
FuzzyQuery::newis infallible, soself.query_error = Noneis the only assignment inrefresh_results, and the field is propagated intoFinderPopupand presumably rendered. Either wire up a real error condition (e.g., over-long query, or reserve it for a future stricter parser) or drop the field until it's needed — the current state is dead code that the UI has to branch on.Also applies to: 64-64, 161-161, 199-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/redox-tui/src/app/state/finder.rs` at line 25, The query_error field is dead code because FuzzyQuery::new is infallible; either remove the field and all propagation to FinderPopup (delete query_error from the FinderState struct and remove uses in refresh_results and FinderPopup rendering), or implement a real validation in refresh_results (e.g., check max query length or forbidden chars before calling FuzzyQuery::new and set self.query_error = Some(...) on validation failure) and keep FinderPopup as-is; target the symbols FinderState::query_error, refresh_results, FuzzyQuery::new, and FinderPopup when making the change.crates/redox-core/src/fuzzy.rs (1)
105-123: Path separators are hardcoded to'/'; breaks filename-priority scoring and depth ordering on Windows.
path_match_scorecorrectly usesPath::file_name(), butfile_name_byte_offset(line 279) callsrsplit_once('/')and depth is computed vialabel.matches('/').count()(line 120). Sincedisplay_path_for_popupusesPath::display(), which emits\separators on Windows, the parsing logic fails for Windows paths:
- The filename-offset contiguous-match preference becomes ineffective—the function falls through to full-label matching
PathMatchScore::depthcollapses to0for all entries, neutralizing the "prefer shallower paths" tiebreakerFor cross-platform robustness, normalize paths via
Path::components()or ensuredisplay_path_for_popupemits a canonical separator (e.g.,/) so the finder maintains a single parsing convention. The Rust standard library'sPath::components()is the idiomatic, cross-platform API for this.Also applies to: 279–284
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/redox-core/src/fuzzy.rs` around lines 105 - 123, path_match_score and related parsing assume '/' separators; fix by using Path APIs instead of string splits: in path_match_score replace depth: label.matches('/').count() with depth computed from Path::new(label).components().count().saturating_sub(1) (or appropriate component count semantics), and change file_name_byte_offset (and any rsplit_once('/') uses) to get the file name via Path::new(label).file_name().and_then(|n| n.to_str()) and locate its byte offset with label.rfind(filename) (or compute offset by summing component lengths) so matching and filename-priority logic work on Windows; also update any code that parses paths for display (e.g., display_path_for_popup) to rely on Path::components() or Path::display() canonicalization rather than hardcoded '/'.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/redox-tui/src/app/state/finder.rs`:
- Around line 397-412: The current save() writes a newline-delimited file
non-atomically and cannot represent paths containing newlines; change
persistence to write a JSON array of Option<String> (length MAX_PINNED_FILES,
None for empty slots) and perform an atomic write by writing to a sibling temp
file (use storage_path.with_extension("tmp") or similar) and then fs::rename to
storage_path; update load to first try reading the JSON array into
Vec<Option<String>> (honoring MAX_PINNED_FILES) and fall back to the existing
line-based parser for backward compatibility, and remove raw_line.trim() so
legal trailing whitespace is preserved when parsing legacy format.
- Around line 449-462: open_finder currently calls collect_files synchronously
on the UI thread (via open_finder -> collect_files -> FinderState::new), causing
freezes for large repos; move the filesystem walk off the UI thread by starting
a background worker that performs the WalkBuilder walk and sends results back
(e.g., via a channel) so you can create FinderState immediately with an
empty/partial list and incrementally feed candidates into FinderState as they
arrive, or alternatively implement a per-launch_dir cache invalidated by a
refresh key and use a worker to populate it; also apply the same
off-thread/read-limit strategy to load_preview (avoid blocking reads on
selection changes), and ensure you cap or timeout the walk (stop after N entries
or M ms) and surface a “truncated” status when results are incomplete.
In `@crates/redox-tui/src/app/state/tests.rs`:
- Around line 77-112: The helper with_isolated_launch_env still mutates
process-global state (set_current_dir and XDG_CONFIG_HOME) which can leak into
parallel tests; either stop mutating globals or make the tests run serially.
Easiest fix: add serial_test (or similar) and annotate the tests that call
with_isolated_launch_env / EditorSession::open_initial_unnamed with #[serial]
(or annotate the tests module to run serially) so they never run in parallel, or
refactor with_isolated_launch_env to avoid set_current_dir()/std::env::set_var
by using APIs that accept explicit paths/config (so
EditorSession::open_initial_unnamed receives a temp dir/config without changing
process env). Ensure you update all tests that call with_isolated_launch_env or
EditorSession::open_initial_unnamed accordingly.
In `@crates/redox-tui/src/input/mod.rs`:
- Around line 1641-1650: pin_slot_from_key currently maps shifted symbols for
slots 0–2 but omits '$' and '%' so Ctrl+Shift+4/5 don't open slots 3/4; update
the pin_slot_from_key function to also return Some(3) for KeyKind::Char('$') and
Some(4) for KeyKind::Char('%'), and add a unit/integration test mirroring the
existing normal_mode_ctrl_shift_symbol_opens_pinned_slot case for slot 3 (using
KeyKind::Char('$') with ctrl+shift) and slot 4 (using KeyKind::Char('%') with
ctrl+shift) to lock the behavior; do not change the Event::Character arm
exclusions that intentionally preserve '$'/'%' in normal mode.
In `@crates/redox-tui/src/ui/widgets/finder.rs`:
- Around line 153-162: The footer width is being subtracted even when
draw_query_row suppresses the footer; ensure you only reserve footer space when
it will actually be rendered by checking the same condition used by
draw_query_row: compute right_w = right_footer.width() but set right_w to 0 if
right_w >= layout.query.inner_w (i.e., footer won't fit), then use that adjusted
right_w when calculating input_w and passing input_w to
finder_input_cursor_offset (also update the analogous computations in the other
block at lines ~548-585). Reference symbols: finder_right_footer,
draw_query_row, right_footer.width(), layout.query.inner_w, input_col, input_w,
finder_input_cursor_offset, popup.query.
---
Nitpick comments:
In `@crates/redox-core/src/fuzzy.rs`:
- Around line 105-123: path_match_score and related parsing assume '/'
separators; fix by using Path APIs instead of string splits: in path_match_score
replace depth: label.matches('/').count() with depth computed from
Path::new(label).components().count().saturating_sub(1) (or appropriate
component count semantics), and change file_name_byte_offset (and any
rsplit_once('/') uses) to get the file name via
Path::new(label).file_name().and_then(|n| n.to_str()) and locate its byte offset
with label.rfind(filename) (or compute offset by summing component lengths) so
matching and filename-priority logic work on Windows; also update any code that
parses paths for display (e.g., display_path_for_popup) to rely on
Path::components() or Path::display() canonicalization rather than hardcoded
'/'.
In `@crates/redox-tui/src/app/state/finder.rs`:
- Line 25: The query_error field is dead code because FuzzyQuery::new is
infallible; either remove the field and all propagation to FinderPopup (delete
query_error from the FinderState struct and remove uses in refresh_results and
FinderPopup rendering), or implement a real validation in refresh_results (e.g.,
check max query length or forbidden chars before calling FuzzyQuery::new and set
self.query_error = Some(...) on validation failure) and keep FinderPopup as-is;
target the symbols FinderState::query_error, refresh_results, FuzzyQuery::new,
and FinderPopup when making the change.
In `@crates/redox-tui/src/lib.rs`:
- Around line 59-65: Add a helper method on EditorMode (e.g. pub fn
has_popup_overlay(self) -> bool) that returns matches!(self, EditorMode::Command
| EditorMode::Search | EditorMode::Finder | EditorMode::PinSelect), then replace
the inline match in the popup_overlay_active calculation (the expression that
also checks state.explorer_popup().is_some()) with
state.mode.has_popup_overlay(), and likewise replace the duplicate matcher used
when suppressing the perf popup with state.mode.has_popup_overlay() so both
sites remain in sync.
In `@crates/redox-tui/src/ui/widgets/toast.rs`:
- Around line 223-230: The renderer currently keys dim styling on the literal
string "press y" which is fragile; instead add an explicit style flag on the
toast message type (e.g., extend Toast/ToastMessage or introduce StatusMsg {
text, style: StatusStyle } with StatusStyle::Dim/Normal or add a hint field on
EditorState), update producers to set StatusStyle::Dim for confirmation prompts,
and change the renderer code that calls window.write_str_colored (and related
functions like write_str_colored) to branch on the new message.style/hint rather
than matching the trimmed text; update any affected constructors/usages to
supply the style so the dim behavior is driven by data, not content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 12d9c7ed-0e94-47d8-af9c-3e31f1f1d878
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
README.mdcrates/redox-core/README.mdcrates/redox-core/src/buffer/tests.rscrates/redox-core/src/fuzzy.rscrates/redox-core/src/lib.rscrates/redox-core/src/session/mod.rscrates/redox-tui/Cargo.tomlcrates/redox-tui/README.mdcrates/redox-tui/src/app/mod.rscrates/redox-tui/src/app/state.rscrates/redox-tui/src/app/state/actions.rscrates/redox-tui/src/app/state/editing.rscrates/redox-tui/src/app/state/explorer.rscrates/redox-tui/src/app/state/finder.rscrates/redox-tui/src/app/state/tests.rscrates/redox-tui/src/input/mod.rscrates/redox-tui/src/lib.rscrates/redox-tui/src/ui/mod.rscrates/redox-tui/src/ui/style.rscrates/redox-tui/src/ui/widgets/finder.rscrates/redox-tui/src/ui/widgets/mod.rscrates/redox-tui/src/ui/widgets/status_bar.rscrates/redox-tui/src/ui/widgets/toast.rs
indented the nested list items)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Here we go, two of the biggest Redox features are now in place: a fuzzy finder (inspired by Telescope.nvim) and a global file pinning system (loosely inspired by Harpoon.nvim). I've been planning and working on these for over a week now, and they're finally here.
The fuzzy finder (
<space><space>) allows you to quickly and easily search for files in the project directory, and navigate to them in an instant. On sufficiently large terminal windows (at least 130 cols wide), it will also show you a preview of the selected file. These previews are currently just black-and-white with no syntax highlighting, but I might add that in the future.The file pinning system allows you to easily "pin" up to five files that you want fast and easy access to, from any location. For example, you can pin a
go_reference.mdfile from a separate notes directory to the first slot in the pinboard, and then quickly open it withctrl+1. You can view the pinned file, edit it, etc. The key thing, however, is that it doesn't actually change your working directory, so when you open the explorer it's automatically back at the main project codebase. This allows for a quick flow of "peeking" reference files (or other important files), and hopping between those and the main project with ease.Ctrl+shift+popens the pinboard popup so you can manage your pins by rearranging their order, overwriting them with new files, or removing pinned entries entirely. These pinned files have their paths stored in a new.config/redox/directory, so they're persistent across all of your sessions. I'm currently planning out the UX for preserved project-specific pins, which are more in line with the behaviour of Harpoon. I think both systems have their place, so I plan to implement both; the only struggle right now is figuring out how to get them to play nice with each other :)Summary by CodeRabbit
New Features
Documentation
Tests