From 57b60252a7ad0497b1ec3c01b4b4ba72a088a4c9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 14:02:11 +0000 Subject: [PATCH] Add performance optimizations to the UI hot paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The whole UI is rebuilt inside `update()`, which egui runs on every repaint — including every mouse move — so per-frame work scaled with the size of the snippet library instead of with what is on screen. Measured on a 1000-snippet library (release build, 2 KB bodies), one grid frame took ~65 ms; it now takes ~2.4 ms, and the search filter went from ~2 ms/frame to ~5 us/frame. - Virtualize the card grid. `card_grid()` lays out only the rows intersecting the viewport (plus a row of overscan) and reserves the height of the skipped rows, so the scrollbar and every card position are unchanged. Card rects for drag-and-drop now come from `grid_card_rect()`, computed from the grid origin, so a drop onto a gap that was never rendered still lands in the right slot. Row widget ids are keyed on the row index (`push_id`) so they no longer shift as rows above the viewport are skipped. - Cache per-snippet derived text (`Derived`): the lowercase title/body/category the search matches against and the collapsed card preview. These were recomputed from scratch on every frame, lowercasing every body in the library and re-collapsing every visible preview. - Memoize the visible-card index list (`FilterCache`), keyed on the query, the category filter, and a library generation counter, reusing its allocation across frames. All snippet mutations now run through `snippets_changed()`, which is what keeps the caches consistent with `snippets`. - Stop rebuilding `egui::Visuals` every frame; apply the theme at startup and when the selection changes. Add `Theme::name() -> &'static str` (`Display` defers to it) so naming a theme doesn't allocate. - Drop per-frame clones: the category list handed to the editor, the save-error banner text, and the combo-box labels. Move the editor's body into the snippet on save instead of copying it. - Stop collapsing a whole snippet body just to preview its first 220 chars, and sort the canonical category list once per batch instead of once per snippet at startup (`Config::add_categories`); skip the allocations in `same_category` for the ASCII names it is called with. Tests cover the visible-row range, computed-vs-rendered card rects, the paint work of a ten-times-larger library, and the memoized filter against a fresh scan; docs record the new invariants. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Fbmt6KmCj5NsuPVQLvnt8J --- AGENTS.md | 17 +- CLAUDE.md | 9 +- src/app.rs | 941 ++++++++++++++++++++++++++++++++++++++++++------- src/storage.rs | 67 +++- src/theme.rs | 97 ++--- 5 files changed, 945 insertions(+), 186 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a10e09d..cd2b6c4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,10 +36,11 @@ CopyIt is a small Windows desktop app for storing scripts and AI prompts as copy - `src/main.rs` — Sets up the native window (`1000x700` default, `560x400` minimum) and runs the egui event loop. - `src/app.rs` — Contains the `CopyIt` app state and the entire UI: - Top bar with search, category filter, theme selector, and a New-snippet button. - - Responsive card grid of snippets. + - Responsive card grid of snippets, rendered by `CopyIt::card_grid` (virtualized — see below). - Copy-to-clipboard action and transient "Copied" feedback. - Modal editor for adding, editing, and deleting snippets. - Drag-and-drop reordering of snippet cards (pointer drag on a card body, not on its buttons). + - Two caches keep per-frame work off the hot path: `Derived` holds each snippet's lowercase title/body/category plus its collapsed card preview, and `FilterCache` memoizes the list of visible card indices for the current query, category filter, and library `generation`. - `src/model.rs` — Defines `Snippet { id, title, category, body }`. - `src/storage.rs` — `data_dir()` resolves the stable `%APPDATA%\CopyIt` directory (falling back to next-to-the-exe if `APPDATA` isn't set, e.g. non-Windows dev/test). `data_path()`, `load()`, and `save()` handle `snippets.json`; `config_path()`, `load_config()`, and `save_config()` handle `config.json` (canonical categories and theme). `legacy_candidate_dirs()` lists old next-to-exe locations used for one-time migration. Also contains the category helpers: `normalize_category()` (title-cases), `same_category()` (case-insensitive comparison), `is_reserved_category()` (rejects blank and the reserved `All`), and `canonical_category()` (maps unusable names to `UNCATEGORIZED`). - Both loaders return `Load` — `Loaded` / `Missing` / `Corrupt` — rather than an `Option`. Keep those three cases distinct: collapsing `Corrupt` into `Missing` makes the app seed defaults over a file it merely failed to parse and destroy the user's library on the next save. @@ -88,7 +89,7 @@ cargo clippy cargo test ``` -Unit tests live in the relevant `src/*.rs` file under `#[cfg(test)] mod tests` (`mod layout_tests` in `app.rs`). Coverage today: grid/gap geometry and the scroll-area coordinate space, drag-and-drop reordering (including filtered views and a snippet that vanishes mid-drag), save-error reporting, atomic writes, corrupt-file recovery, category normalization, and theme name round-trips. +Unit tests live in the relevant `src/*.rs` file under `#[cfg(test)] mod tests` (`mod layout_tests` in `app.rs`). Coverage today: grid/gap geometry and the scroll-area coordinate space, grid virtualization (visible-row range, computed-vs-rendered card rects, and that a ten-times-larger library emits roughly the same paint work), the memoized filter (matching a fresh scan, and invalidating on query/category/library changes), preview collapsing and truncation, drag-and-drop reordering (including filtered views and a snippet that vanishes mid-drag), save-error reporting, atomic writes, corrupt-file recovery, category normalization, and theme name round-trips. Tests that touch the save paths must point `path`/`config_path` at a throwaway temp directory — use the `test_app()` helper in `app.rs`, which does this. A test that leaves them as bare relative filenames writes `snippets.json` into the repository root. @@ -136,6 +137,18 @@ Tests that touch the save paths must point `path`/`config_path` at a throwaway t `src/main.rs` sets `#![cfg_attr(not(debug_assertions), windows_subsystem = "windows")]`, so `--release` builds launch without a console window. Debug builds retain the console for troubleshooting. +## Performance model + +The UI is repainted on every mouse move, so anything done inside `update()` runs dozens of times a second. The costly work is therefore cached or skipped: + +- **Derived snippet text is cached.** `Derived` stores the lowercase title/body/category the search matches against, plus the collapsed one-line card preview. Rebuilding it walks the whole library, so it happens only when the library changes. +- **All snippet mutations go through `CopyIt::snippets_changed()`.** It calls `rebuild_derived()` (which also bumps `generation`, invalidating the filter cache) and then `save_snippets()`. Never mutate `self.snippets` and call `save_snippets()` directly: `derived` is a parallel `Vec` indexed the same way as `snippets`, and letting it drift shows the wrong preview on a card and makes search match text that is no longer there. `card()` falls back to computing a preview if the two ever disagree, and a `debug_assert` catches it in debug builds. +- **The visible-card list is memoized.** `take_filtered()` recomputes the filtered index list only when the search text, the category filter, or `generation` changed; otherwise it hands back the same buffer. It *takes* the buffer (leaving the cache marked invalid) so the render loop can still borrow `self` mutably, and `restore_filtered()` puts it back at the end of the frame, reusing the allocation. If you add a second `take_filtered()` in one frame, restore it — the cache is deliberately treated as invalid while checked out, so the second call recomputes rather than reporting "no snippets match". +- **The card grid is virtualized.** `card_grid()` lays out only the rows intersecting `ui.clip_rect()`, plus one row of overscan, and reserves the height of the skipped rows above and below with `ui.add_space`, so the scrollbar and every card position are exactly what they would be if all rows were built. `visible_rows()` computes that range (and falls back to "all rows" on non-finite geometry). +- **Card rects are computed, not harvested.** Because rows can be skipped, `grid_card_rect()` derives each card's rect from the grid origin and the `CARD_*` / `CARD_SPACING` constants, and `card_grid` returns rects for *every* filtered card. Drag-and-drop can therefore still drop onto a gap that was never rendered. `layout_tests::computed_grid_rects_match_rendered_cards` pins the computed rects to what a real card gets, and a `debug_assert` in `card_grid` re-checks it per card. Changing card size, spacing, or grid margins means changing those constants — the renderer and the hit-testing read the same ones. +- **Each grid row gets an explicit widget id** via `ui.push_id(row_start, …)`. egui otherwise derives ids from a per-parent counter, which would make the ids inside a card depend on how many rows above the viewport were skipped, so a card's buttons would change identity as the user scrolls. +- **Visuals are applied only when the theme changes**, at startup and from the theme selector (which also requests one extra repaint so the already-painted top bar is redrawn with the new colors). `Theme::visuals()` builds a whole `egui::Visuals`; it is not something to do per frame. Use `Theme::name()` (`&'static str`) instead of `to_string()` in UI code, and avoid cloning app state (categories, the save-error message) just to satisfy the borrow checker inside a closure — borrow it, or record the decision in a local and apply it after the closure. + ## Common gotchas - `CopyIt` implements `eframe::App`, which already defines a `save(&mut self, _storage: &mut dyn Storage)` method. Do not add an inherent method named `save` on `CopyIt`; it will shadow the trait method and break compilation. Use descriptive names such as `save_snippets` and `save_config` for application-level persistence, as the current code does. diff --git a/CLAUDE.md b/CLAUDE.md index f5fffbf..2ac1273 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -23,7 +23,8 @@ cargo test # unit tests in src/*.rs (layout, drag/reorder, storage, Six modules in `src/`, with a strict separation of concerns: - `main.rs` — eframe setup (1000x700 window, 560x400 min). Sets `windows_subsystem = "windows"` for release only, so release builds have no console. -- `app.rs` — the `CopyIt` struct (all app state) and the entire egui UI: top bar (search, category filter, theme selector, New button), responsive card grid, clipboard copy with transient "Copied" feedback, modal add/edit/delete editor, and drag-and-drop card reordering. UI helpers (`truncate_chars`, `preview_text`, `category_color`) live at the bottom. +- `app.rs` — the `CopyIt` struct (all app state) and the entire egui UI: top bar (search, category filter, theme selector, New button), responsive card grid, clipboard copy with transient "Copied" feedback, modal add/edit/delete editor, and drag-and-drop card reordering. UI helpers (`truncate_chars`, `preview_text`, `category_color`, `grid_card_rect`, `visible_rows`) live at the bottom. + - Frame-rate hot paths are cached, not recomputed per repaint: `Derived` (per-snippet lowercase text + card preview) and `FilterCache` (the visible-card index list). The card grid also only lays out the rows in view. - `model.rs` — `Snippet { id, title, category, body }`. - `storage.rs` — JSON persistence. `data_dir()` resolves `%APPDATA%\CopyIt` (falls back to next-to-exe when `APPDATA` is unset, e.g. non-Windows dev). Handles `snippets.json` (library) and `config.json` (canonical categories + selected theme), one-time migration from legacy next-to-exe locations, and `normalize_category()` (title-cases and dedupes). - `seed.rs` — default snippet library seeded on first launch. @@ -31,6 +32,12 @@ Six modules in `src/`, with a strict separation of concerns: Persistence rule: keep UI in `app.rs`, data types in `model.rs`, persistence in `storage.rs`, themes in `theme.rs`. Saves happen automatically after every add/edit/delete/reorder. +## Performance rules + +- **Every mutation of `snippets` must go through `CopyIt::snippets_changed()`** (not a bare `save_snippets()`). It rebuilds the `derived` cache so it stays index-aligned with `snippets`, bumps `generation` to invalidate the filter cache, and then saves. Skipping it leaves cards showing another snippet's preview and search matching stale text. +- **The card grid is virtualized.** Only the rows intersecting `ui.clip_rect()` (plus one row of overscan) are laid out; the rest are replaced with `add_space` of the same height. Card rects come from `grid_card_rect` (computed from the grid origin), so drag-and-drop still sees the whole grid. If you change card size or spacing, change the `CARD_*` / `ROW_PITCH` / `GRID_*` constants — the renderer and the hit-testing both read them. +- **Don't call `ctx.set_visuals()` every frame.** It rebuilds a whole `egui::Visuals`; apply it at startup and when the theme selection changes. Use `Theme::name()` (`&'static str`) rather than `to_string()` in UI code. + ## Critical gotchas - **Do not add an inherent `save` method to `CopyIt`.** `eframe::App` already defines `save(&mut self, &mut dyn Storage)`; an inherent method would shadow it and break the build. Use `save_snippets` / `save_config` (as the code does). diff --git a/src/app.rs b/src/app.rs index 97eb829..efcc1ad 100644 --- a/src/app.rs +++ b/src/app.rs @@ -11,6 +11,88 @@ const SNIPPETS_SAVE_ERROR: &str = "Couldn't save snippets"; const CONFIG_SAVE_ERROR: &str = "Couldn't save settings"; /// Red used for the warning banner and the delete-confirmation button. const WARNING_COLOR: egui::Color32 = egui::Color32::from_rgb(0xef, 0x44, 0x44); +/// Characters of a snippet body shown in a card's preview line. +const PREVIEW_CHARS: usize = 220; + +// ---- Card grid geometry ---- +// The grid is uniform, and both the renderer and the drag-and-drop hit-testing derive +// card positions from these numbers, so they live in one place. +/// Width of a card's inner content area. +const CARD_INNER_W: f32 = 300.0; +/// Height of a card's inner content area. +const CARD_INNER_H: f32 = 168.0; +/// The group frame around each card adds 10 px of inner margin on each side. +const CARD_FRAME_MARGIN: f32 = 20.0; +/// Full visible width of a card, frame included. +const CARD_W: f32 = CARD_INNER_W + CARD_FRAME_MARGIN; +/// Full visible height of a card, frame included. +const CARD_H: f32 = CARD_INNER_H + CARD_FRAME_MARGIN; +/// Gap between neighbouring cards, horizontally and vertically. +const CARD_SPACING: f32 = 12.0; +/// Vertical distance from the top of one row of cards to the top of the next. +const ROW_PITCH: f32 = CARD_H + CARD_SPACING; +/// Blank space above the first row inside the scroll area. +const GRID_TOP_SPACE: f32 = 4.0; +/// Horizontal padding on both sides of the grid. +const GRID_MARGIN_X: f32 = 18.0; + +/// Per-snippet data derived from the snippet's own text: the lowercase forms the +/// search filter matches against, and the collapsed one-line card preview. +/// +/// These used to be recomputed inside the render loop, which meant lowercasing +/// every snippet body and re-collapsing every visible preview on *every* frame — +/// egui repaints on each mouse move, so a large library re-walked all of its text +/// dozens of times a second. They only change when a snippet changes, so they are +/// cached here and rebuilt by [`CopyIt::rebuild_derived`]. +struct Derived { + title_lower: String, + category_lower: String, + body_lower: String, + preview: String, +} + +impl Derived { + fn new(s: &Snippet) -> Self { + Derived { + title_lower: s.title.to_lowercase(), + category_lower: s.category.to_lowercase(), + body_lower: s.body.to_lowercase(), + preview: preview_text(&s.body, PREVIEW_CHARS), + } + } + + /// True when the snippet matches an already-lowercased, already-trimmed query. + fn matches(&self, query_lower: &str) -> bool { + self.title_lower.contains(query_lower) + || self.body_lower.contains(query_lower) + || self.category_lower.contains(query_lower) + } +} + +/// Memoized result of the search/category filter: the indices into `snippets` of +/// the cards that are currently visible, plus the inputs they were computed from. +/// +/// The filter only has to run when the query, the category selection, or the +/// library itself changes. Without this the grid rebuilt the whole index list — +/// scanning every snippet's title, body and category — on every repaint. +#[derive(Default)] +struct FilterCache { + indices: Vec, + search: String, + category: String, + generation: u64, + valid: bool, +} + +impl FilterCache { + /// True when the cached indices still describe the given inputs. + fn is_current(&self, search: &str, category: &str, generation: u64) -> bool { + self.valid + && self.generation == generation + && self.search == search + && self.category == category + } +} /// Main application state and UI coordinator. /// Maintains the full snippet library, handles search/filter/category logic, @@ -18,6 +100,9 @@ const WARNING_COLOR: egui::Color32 = egui::Color32::from_rgb(0xef, 0x44, 0x44); /// and persists all changes to disk automatically after mutations. pub struct CopyIt { snippets: Vec, // Full snippet library; order is preserved and user-draggable + derived: Vec, // Cached lowercase text + card preview, one entry per snippet (same order) + generation: u64, // Bumped whenever `snippets`/`derived` change; invalidates the filter cache + filter: FilterCache, // Memoized indices of the snippets visible under the current search/category next_id: u64, // Next ID to assign to a new snippet; incremented on creation path: PathBuf, // Path to snippets.json in the stable data directory config_path: PathBuf, // Path to config.json (categories + theme selection) @@ -215,9 +300,7 @@ impl CopyIt { Config::from_snippets(&snippets) } }; - for s in &snippets { - config.add_category(&s.category); - } + config.add_categories(snippets.iter().map(|s| s.category.as_str())); if let Err(e) = storage::save_config(&config_path, &config) { save_error = Some(format!("{CONFIG_SAVE_ERROR}: {e}")); } @@ -232,8 +315,13 @@ impl CopyIt { } } + let derived = snippets.iter().map(Derived::new).collect(); + Self { snippets, + derived, + generation: 0, + filter: FilterCache::default(), next_id, path, config_path, @@ -251,6 +339,78 @@ impl CopyIt { } } + /// Recomputes the per-snippet derived text caches and invalidates the filter cache. + /// Called from [`Self::snippets_changed`] after every mutation of `snippets`, which is + /// what keeps `derived` index-aligned with `snippets`. + fn rebuild_derived(&mut self) { + self.derived.clear(); + self.derived.reserve(self.snippets.len()); + self.derived.extend(self.snippets.iter().map(Derived::new)); + self.generation = self.generation.wrapping_add(1); + self.filter.valid = false; + } + + /// The single entry point for "the library changed": refreshes the derived caches + /// and persists the new library. Every add/edit/delete/reorder goes through here, so + /// `derived` can never drift out of sync with `snippets`. + fn snippets_changed(&mut self) { + self.rebuild_derived(); + self.save_snippets(); + } + + /// Returns the indices of the snippets visible under the current search and + /// category filter, recomputing them only when one of the inputs (or the library) + /// has changed. Ownership of the buffer is handed to the caller for the rest of the + /// frame so the render loop can borrow `self` mutably; [`Self::restore_filtered`] + /// puts it back, keeping its allocation for the next frame. + fn take_filtered(&mut self) -> Vec { + if !self + .filter + .is_current(&self.search, &self.category_filter, self.generation) + { + // Trim so a query of only spaces behaves like an empty one. + let query = self.search.trim().to_lowercase(); + let all_categories = self.category_filter == "All"; + let category = self.category_filter.as_str(); + let snippets = &self.snippets; + + let mut indices = std::mem::take(&mut self.filter.indices); + indices.clear(); + indices.extend( + self.derived + .iter() + .enumerate() + .filter(|(i, d)| { + (all_categories || snippets[*i].category == category) + && (query.is_empty() || d.matches(&query)) + }) + .map(|(i, _)| i), + ); + + self.filter.indices = indices; + self.filter.search.clear(); + self.filter.search.push_str(&self.search); + self.filter.category.clear(); + self.filter.category.push_str(&self.category_filter); + self.filter.generation = self.generation; + } + // The cache is only trusted while it actually holds the buffer: marking it + // invalid on the way out means a second `take_filtered` before the matching + // restore recomputes, instead of handing back an empty (i.e. "no cards") list. + self.filter.valid = false; + std::mem::take(&mut self.filter.indices) + } + + /// Returns the buffer handed out by [`Self::take_filtered`] so its allocation is + /// reused next frame. A mutation later in the same frame bumps `generation`, which + /// makes the restored indices stale-checked (not trusted) on the following frame. + fn restore_filtered(&mut self, filtered: Vec) { + self.filter.indices = filtered; + // Trustworthy again — `is_current` still re-checks the query, the category and + // the generation, so a mutation made later in this frame invalidates it anyway. + self.filter.valid = true; + } + /// Persists the full snippet library to snippets.json in the stable data directory. /// Updates save_error if an I/O error occurs; the error is shown in the top bar. fn save_snippets(&mut self) { @@ -357,7 +517,7 @@ impl CopyIt { }; let target_abs = target_abs.min(self.snippets.len()); self.snippets.insert(target_abs, snippet); - self.save_snippets(); + self.snippets_changed(); } /// Renders a single snippet card with all visual elements: title (strong, truncated), category badge @@ -377,7 +537,20 @@ impl CopyIt { is_dragged: bool, ) -> CardWidgets { let s = &self.snippets[idx]; - let card_h = 168.0; + let card_h = CARD_INNER_H; + + // The preview is cached per snippet; fall back to computing it only if the + // caches somehow got out of step, so a stale index can never panic or blank + // a card in a release build. + debug_assert_eq!(self.derived.len(), self.snippets.len()); + let fallback_preview; + let preview: &str = match self.derived.get(idx) { + Some(d) => &d.preview, + None => { + fallback_preview = preview_text(&s.body, PREVIEW_CHARS); + &fallback_preview + } + }; if is_dragged { ui.set_opacity(0.4); @@ -450,7 +623,7 @@ impl CopyIt { } ui.add_space(4.0); ui.with_layout(egui::Layout::top_down(egui::Align::LEFT), |ui| { - ui.label(egui::RichText::new(preview_text(&s.body, 220)).weak()); + ui.label(egui::RichText::new(preview).weak()); }); resp }) @@ -469,6 +642,123 @@ impl CopyIt { let (copy, edit) = frame.inner; CardWidgets { frame_rect, copy, edit } } + + /// Lays out the responsive card grid inside the scroll area and returns the column + /// count together with the screen-space rect of *every* filtered card. + /// + /// Only the rows that intersect the viewport (plus one row of overscan) are actually + /// built; the rest are replaced by blank space of exactly the same height. A library + /// of hundreds of snippets used to construct every card — text layout, galleys, + /// interaction ids — on every repaint, including the ones scrolled far out of sight. + /// + /// The returned rects cover the skipped cards too: the grid is uniform, so their + /// geometry is computed from the grid origin rather than harvested from the layout. + /// Drag-and-drop therefore still sees the whole grid and can drop onto an off-screen + /// gap exactly as before. Card rects are already in absolute screen space (see the + /// note in `update`), so they need no further translation. + /// + /// Collected interactions are appended to `actions` / `drag_start` / `hover_cursor` + /// instead of being applied here, so the caller can act on them once the grid's + /// borrow of `self` has ended. + fn card_grid( + &self, + ui: &mut egui::Ui, + filtered: &[usize], + now: f64, + actions: &mut Vec, + drag_start: &mut Option<(u64, egui::Pos2)>, + hover_cursor: &mut Option, + ) -> (usize, Vec) { + egui::Frame::none() + .inner_margin(egui::Margin::symmetric(GRID_MARGIN_X, 0.0)) + .show(ui, |ui| { + let avail = ui.available_width(); + let cols = ((avail / (CARD_W + CARD_SPACING)).floor() as usize).max(1); + + let origin = ui.cursor().min; + let rows = filtered.len().div_ceil(cols); + let card_rects: Vec = (0..filtered.len()) + .map(|i| grid_card_rect(i, cols, origin, CARD_W, CARD_H, CARD_SPACING)) + .collect(); + + let (first_row, last_row) = + visible_rows(ui.clip_rect(), origin.y, ROW_PITCH, rows); + // Reserve the height of the rows above the viewport. + if first_row > 0 { + ui.add_space(first_row as f32 * ROW_PITCH); + } + + for row_start in (first_row..=last_row).map(|r| r * cols) { + let row_end = (row_start + cols).min(filtered.len()); + let row = &filtered[row_start..row_end]; + // Key each row's widget ids on the row itself. egui otherwise derives + // them from a per-parent counter, which would make every id inside the + // grid depend on how many rows above the viewport were skipped — so a + // card's buttons would change identity as the user scrolled. + ui.push_id(row_start, |ui| { + ui.horizontal(|ui| { + ui.spacing_mut().item_spacing = egui::vec2(0.0, 0.0); + for (row_offset, &idx) in row.iter().enumerate() { + let is_dragged = self.drag.as_ref().is_some_and(|d| { + d.dragging && d.snippet_id == self.snippets[idx].id + }); + + let drag_id = ui.id().with("card_drag").with(idx); + let expected_rect = egui::Rect::from_min_size( + ui.cursor().min, + egui::vec2(CARD_W, CARD_H), + ); + let drag_resp = + ui.interact(expected_rect, drag_id, egui::Sense::drag()); + + let widgets = + self.card(ui, idx, CARD_INNER_W, now, actions, is_dragged); + + debug_assert!( + card_rects + .get(row_start + row_offset) + .is_some_and(|r| r.min.distance(widgets.frame_rect.min) + < 0.5), + "computed card rect must match the rendered one" + ); + + let pointer_over_buttons = + widgets.copy.hovered() || widgets.edit.hovered(); + + // Initiate drag only if: pointer is not over buttons, no active drag, and drag sensor triggered + if drag_resp.drag_started() + && !pointer_over_buttons + && self.drag.is_none() + { + if let Some(pos) = drag_resp.interact_pointer_pos() { + *drag_start = Some((self.snippets[idx].id, pos)); + } + } + + if drag_resp.hovered() + && self.drag.is_none() + && !pointer_over_buttons + { + *hover_cursor = Some(egui::CursorIcon::Grab); + } + + ui.add_space(CARD_SPACING); + } + }); + }); + ui.add_space(CARD_SPACING); + } + + // Reserve the height of the rows below the viewport, so the scrollbar + // still spans the whole library. + if last_row + 1 < rows { + ui.add_space((rows - 1 - last_row) as f32 * ROW_PITCH); + } + + (cols, card_rects) + }) + .inner + } } impl eframe::App for CopyIt { @@ -480,7 +770,10 @@ impl eframe::App for CopyIt { fn update(&mut self, ctx: &egui::Context, _frame: &mut eframe::Frame) { let now = ctx.input(|i| i.time); let previous_theme = self.theme; - ctx.set_visuals(self.theme.visuals()); + // The visuals are *not* rebuilt here every frame: `Theme::visuals()` constructs a + // whole `egui::Visuals` (and `set_visuals` clones the context style) for a value + // that only changes when the user picks a different theme. `CopyIt::new` applies + // the loaded theme once, and the theme selector below re-applies it on change. // ---- Top bar: search, category filter, theme selector, new ---- egui::TopBottomPanel::top("top").show(ctx, |ui| { @@ -503,7 +796,7 @@ impl eframe::App for CopyIt { let mut filter_selected = self.category_filter.clone(); let mut start_adding_category = false; egui::ComboBox::from_id_source("cat_filter") - .selected_text(self.category_filter.clone()) + .selected_text(self.category_filter.as_str()) .show_ui(ui, |ui| { ui.selectable_value(&mut filter_selected, "All".to_string(), "All"); for c in &self.categories { @@ -571,13 +864,17 @@ impl eframe::App for CopyIt { ui.add_space(12.0); egui::ComboBox::from_id_source("theme_select") - .selected_text(self.theme.to_string()) + .selected_text(self.theme.name()) .show_ui(ui, |ui| { for t in Theme::all() { - ui.selectable_value(&mut self.theme, *t, t.to_string()); + ui.selectable_value(&mut self.theme, *t, t.name()); } }); if self.theme != previous_theme { + ctx.set_visuals(self.theme.visuals()); + // The top bar above has already been painted with the old visuals, so + // ask for one more frame to redraw everything with the new ones. + ctx.request_repaint(); self.save_config(); } @@ -587,8 +884,11 @@ impl eframe::App for CopyIt { } }); }); - if let Some(err) = self.save_error.clone() { + // Borrow the message instead of cloning it every frame; the dismiss click is + // recorded in a local and applied after the closure releases the borrow. + if let Some(err) = self.save_error.as_deref() { ui.add_space(4.0); + let mut dismissed = false; ui.horizontal(|ui| { ui.add_space(16.0); ui.colored_label(WARNING_COLOR, format!("\u{26A0} {err}")); @@ -599,32 +899,23 @@ impl eframe::App for CopyIt { .on_hover_text("Dismiss") .clicked() { - self.save_error = None; + dismissed = true; } }); + if dismissed { + self.save_error = None; + } } ui.add_space(6.0); }); // ---- Main grid ---- - egui::CentralPanel::default().show(ctx, |ui| { - // Trim so a query of only spaces behaves like an empty one. - let q = self.search.trim().to_lowercase(); - // Filter snippets by category (if not "All") and search query (case-insensitive across title/body/category) - let filtered: Vec = self - .snippets - .iter() - .enumerate() - .filter(|(_, s)| { - (self.category_filter == "All" || s.category == self.category_filter) - && (q.is_empty() - || s.title.to_lowercase().contains(&q) - || s.body.to_lowercase().contains(&q) - || s.category.to_lowercase().contains(&q)) - }) - .map(|(i, _)| i) - .collect(); + // Visible-card indices come from the memoized filter: the scan over every + // snippet's title/body/category only reruns when the query, the category + // selection, or the library itself changes — not on every repaint. + let filtered = self.take_filtered(); + egui::CentralPanel::default().show(ctx, |ui| { if filtered.is_empty() { // No cards are on screen, so there is nothing to drop onto and the // drag-handling code below is skipped entirely. Abandon any drag now; @@ -642,88 +933,19 @@ impl eframe::App for CopyIt { let mut drag_start: Option<(u64, egui::Pos2)> = None; let mut hover_cursor: Option = None; - // Card layout: inner content (300x168) + frame padding (20px total) = visible card size - let card_inner_w = 300.0_f32; - let card_inner_h = 168.0_f32; - // The group frame around each card has 10 px inner margin on each side. - let card_frame_margin = 20.0_f32; - let card_w = card_inner_w + card_frame_margin; - let card_h = card_inner_h + card_frame_margin; - // Spacing between cards and rows; margin for horizontal scroll area padding - let spacing = 12.0_f32; - let top_space = 4.0_f32; - let margin_x = 18.0_f32; - let scroll_output = egui::ScrollArea::vertical() .auto_shrink([false; 2]) .show(ui, |ui| { ui.spacing_mut().item_spacing = egui::vec2(0.0, 0.0); - ui.add_space(top_space); - - egui::Frame::none() - .inner_margin(egui::Margin::symmetric(margin_x, 0.0)) - .show(ui, |ui| { - let avail = ui.available_width(); - let cols = - ((avail / (card_w + spacing)).floor() as usize).max(1); - - let mut card_content_rects: Vec = - Vec::with_capacity(filtered.len()); - - for row in filtered.chunks(cols) { - ui.horizontal(|ui| { - ui.spacing_mut().item_spacing = egui::vec2(0.0, 0.0); - for &idx in row.iter() { - let is_dragged = self.drag.as_ref().is_some_and(|d| { - d.dragging && d.snippet_id == self.snippets[idx].id - }); - - let drag_id = ui.id().with("card_drag").with(idx); - let expected_rect = egui::Rect::from_min_size( - ui.cursor().min, - egui::vec2(card_w, card_h), - ); - let drag_resp = ui.interact( - expected_rect, - drag_id, - egui::Sense::drag(), - ); - - let widgets = self.card( - ui, idx, card_inner_w, now, &mut actions, is_dragged, - ); - - card_content_rects.push(widgets.frame_rect); - - let pointer_over_buttons = widgets.copy.hovered() - || widgets.edit.hovered(); - - // Initiate drag only if: pointer is not over buttons, no active drag, and drag sensor triggered - if drag_resp.drag_started() - && !pointer_over_buttons - && self.drag.is_none() - { - if let Some(pos) = drag_resp.interact_pointer_pos() { - drag_start = Some((self.snippets[idx].id, pos)); - } - } - - if drag_resp.hovered() - && self.drag.is_none() - && !pointer_over_buttons - { - hover_cursor = Some(egui::CursorIcon::Grab); - } - - ui.add_space(spacing); - } - }); - ui.add_space(spacing); - } - - (cols, card_content_rects) - }) - .inner + ui.add_space(GRID_TOP_SPACE); + self.card_grid( + ui, + &filtered, + now, + &mut actions, + &mut drag_start, + &mut hover_cursor, + ) }); // Process normal click actions. @@ -784,21 +1006,21 @@ impl eframe::App for CopyIt { hover_cursor = Some(egui::CursorIcon::Grabbing); if let Some(pointer) = pointer_pos { - let gap = nearest_gap(pointer, card_screen_rects, cols, spacing, card_w); + let gap = nearest_gap(pointer, card_screen_rects, cols, CARD_SPACING, CARD_W); draw_insertion_line( ctx, gap, card_screen_rects, cols, - spacing, - card_w, - card_h, + CARD_SPACING, + CARD_W, + CARD_H, ); // Hollow ghost box following the cursor. let ghost_rect = egui::Rect::from_min_size( pointer + egui::vec2(8.0, 8.0), - egui::vec2(card_w, card_h), + egui::vec2(CARD_W, CARD_H), ); let painter = ctx.layer_painter(egui::LayerId::new( egui::Order::Tooltip, @@ -818,7 +1040,7 @@ impl eframe::App for CopyIt { if let Some(pointer) = pointer_pos { if grid_area.contains(pointer) { let gap = - nearest_gap(pointer, card_screen_rects, cols, spacing, card_w); + nearest_gap(pointer, card_screen_rects, cols, CARD_SPACING, CARD_W); self.reorder(snippet_id, gap, &filtered); } } @@ -837,11 +1059,16 @@ impl eframe::App for CopyIt { } }); + // Hand the index buffer back so its allocation is reused next frame. + self.restore_filtered(filtered); + // ---- Editor window (new / edit / delete) ---- // Modal editor for creating or modifying snippets; supports inline category creation via the dropdown if self.editor.is_some() { let mut ed = self.editor.take().unwrap(); - let categories = self.categories.clone(); + // Borrowed, not cloned: the whole category list used to be duplicated on + // every frame the editor was open. + let categories = &self.categories; let mut window_open = true; let mut result = EditorResult::None; let title = if ed.id.is_some() { @@ -891,7 +1118,7 @@ impl eframe::App for CopyIt { .width(180.0) .selected_text(display) .show_ui(ui, |ui| { - for c in &categories { + for c in categories { ui.selectable_value(&mut selected, c.clone(), c); } ui.selectable_value( @@ -990,7 +1217,9 @@ impl eframe::App for CopyIt { if let Some(s) = self.snippets.iter_mut().find(|s| s.id == id) { s.title = title; s.category = category; - s.body = ed.body.clone(); + // The editor is closing, so its buffer can be moved + // instead of copied — snippet bodies can be large. + s.body = std::mem::take(&mut ed.body); } } else { let id = self.next_id; @@ -999,15 +1228,15 @@ impl eframe::App for CopyIt { id, title, category, - body: ed.body.clone(), + body: std::mem::take(&mut ed.body), }); } - self.save_snippets(); + self.snippets_changed(); } EditorResult::Delete => { if let Some(id) = ed.id { self.snippets.retain(|s| s.id != id); - self.save_snippets(); + self.snippets_changed(); } } EditorResult::Cancel => {} @@ -1035,8 +1264,9 @@ impl eframe::App for CopyIt { /// Counts Unicode characters, not bytes, to correctly handle multi-byte characters. fn truncate_chars(s: &str, max: usize) -> String { if s.chars().count() > max { - let t: String = s.chars().take(max.saturating_sub(1)).collect(); - format!("{t}\u{2026}") + let mut t: String = s.chars().take(max.saturating_sub(1)).collect(); + t.push('\u{2026}'); + t } else { s.to_string() } @@ -1044,11 +1274,78 @@ fn truncate_chars(s: &str, max: usize) -> String { /// Collapses a snippet body into a single-line preview: splits on whitespace, joins with single spaces, /// and truncates to max characters. Used to display a short preview in each card. +/// +/// Collapsing stops as soon as enough characters have been gathered to fill the preview, +/// so a megabyte-long body costs the same as a one-line one instead of being copied whole. fn preview_text(body: &str, max: usize) -> String { - let collapsed: String = body.split_whitespace().collect::>().join(" "); + let mut collapsed = String::new(); + let mut chars = 0usize; + for word in body.split_whitespace() { + if !collapsed.is_empty() { + collapsed.push(' '); + chars += 1; + } + collapsed.push_str(word); + chars += word.chars().count(); + // One char past the limit is all `truncate_chars` needs to know it must trim. + if chars > max { + break; + } + } truncate_chars(&collapsed, max) } +/// Position of the card at index `i` of the filtered grid, computed from the grid's +/// origin rather than from layout. The grid is uniform — `cols` cards of `card_w` x +/// `card_h` per row, separated by `spacing` — so off-screen cards still have exact +/// rects for drag-and-drop hit-testing without being laid out or painted. +fn grid_card_rect( + i: usize, + cols: usize, + origin: egui::Pos2, + card_w: f32, + card_h: f32, + spacing: f32, +) -> egui::Rect { + let cols = cols.max(1); + let col = i % cols; + let row = i / cols; + egui::Rect::from_min_size( + origin + + egui::vec2( + col as f32 * (card_w + spacing), + row as f32 * (card_h + spacing), + ), + egui::vec2(card_w, card_h), + ) +} + +/// Inclusive range of grid rows that intersect `clip` (the visible part of the scroll +/// area), with one row of overscan on each side so a row entering the viewport is +/// already laid out and edge rounding can't reveal a gap. Rows outside the range are +/// replaced by blank space of the same height, so scrolling and card positions are +/// unaffected. Falls back to "every row" if the geometry isn't finite. +fn visible_rows(clip: egui::Rect, origin_y: f32, row_pitch: f32, rows: usize) -> (usize, usize) { + let max_row = rows.saturating_sub(1); + if rows == 0 { + return (0, 0); + } + if !row_pitch.is_finite() + || row_pitch <= 0.0 + || !origin_y.is_finite() + || !clip.top().is_finite() + || !clip.bottom().is_finite() + { + return (0, max_row); + } + let first = (((clip.top() - origin_y) / row_pitch).floor() - 1.0).max(0.0); + let last = (((clip.bottom() - origin_y) / row_pitch).ceil() + 1.0).max(0.0); + // `as usize` saturates, so an absurd clip rect clamps instead of wrapping. + let first = (first as usize).min(max_row); + let last = (last as usize).clamp(first, max_row); + (first, last) +} + /// Deterministically maps a category name to a color from a 6-color palette via hashing. /// Same category name always maps to the same color. Used to visually distinguish categories in badges. fn category_color(cat: &str) -> egui::Color32 { @@ -1307,8 +1604,11 @@ mod layout_tests { let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).expect("create temp data dir"); let next_id = snippets.iter().map(|s| s.id).max().unwrap_or(0) + 1; - CopyIt { + let mut app = CopyIt { snippets, + derived: Vec::new(), + generation: 0, + filter: FilterCache::default(), next_id, path: dir.join("snippets.json"), config_path: dir.join("config.json"), @@ -1323,7 +1623,9 @@ mod layout_tests { new_header_category: String::new(), category_error: None, save_error: None, - } + }; + app.rebuild_derived(); + app } fn snippet(id: u64, category: &str) -> Snippet { @@ -1683,6 +1985,397 @@ mod layout_tests { } } + /// Runs a single frame of the real card grid in a 1000x700 window at the given + /// scroll offset. Returns the number of paint shapes the frame emitted (a proxy for + /// how many cards were actually built), the rects the grid reported for every card, + /// the scroll area's content size, and the column count. + fn run_grid( + app: &CopyIt, + scroll_offset: f32, + ) -> (usize, Vec, egui::Vec2, usize) { + let ctx = egui::Context::default(); + let input = egui::RawInput { + screen_rect: Some(egui::Rect::from_min_size( + egui::pos2(0.0, 0.0), + egui::vec2(1000.0, 700.0), + )), + ..Default::default() + }; + let filtered: Vec = (0..app.snippets.len()).collect(); + let mut rects = Vec::new(); + let mut content_size = egui::Vec2::ZERO; + let mut cols = 0; + + let output = ctx.run(input, |ctx| { + egui::CentralPanel::default().show(ctx, |ui| { + let scroll = egui::ScrollArea::vertical() + .auto_shrink([false; 2]) + .vertical_scroll_offset(scroll_offset) + .show(ui, |ui| { + ui.spacing_mut().item_spacing = egui::vec2(0.0, 0.0); + ui.add_space(GRID_TOP_SPACE); + let mut actions = Vec::new(); + let mut drag_start = None; + let mut hover_cursor = None; + app.card_grid( + ui, + &filtered, + 0.0, + &mut actions, + &mut drag_start, + &mut hover_cursor, + ) + }); + cols = scroll.inner.0; + rects = scroll.inner.1.clone(); + content_size = scroll.content_size; + }); + }); + + (output.shapes.len(), rects, content_size, cols) + } + + /// The grid must only build the cards near the viewport, while still reporting the + /// geometry of the whole library and reserving its full scroll height. + #[test] + fn card_grid_only_builds_the_rows_in_view() { + let small = test_app( + "grid-small", + (1..=40).map(|i| snippet(i, "Git")).collect(), + vec!["Git".into()], + ); + let large = test_app( + "grid-large", + (1..=400).map(|i| snippet(i, "Git")).collect(), + vec!["Git".into()], + ); + + let (small_shapes, small_rects, small_content, cols) = run_grid(&small, 0.0); + let (large_shapes, large_rects, large_content, large_cols) = run_grid(&large, 0.0); + assert_eq!(cols, 2); + assert_eq!(large_cols, 2); + + // Every card is accounted for, on-screen or not. + assert_eq!(small_rects.len(), 40); + assert_eq!(large_rects.len(), 400); + + // The scroll range still spans the whole library. + let expected_height = |n: usize| GRID_TOP_SPACE + (n as f32 / 2.0).ceil() * ROW_PITCH; + assert!( + (small_content.y - expected_height(40)).abs() < 1.0, + "content height {} != {}", + small_content.y, + expected_height(40) + ); + assert!( + (large_content.y - expected_height(400)).abs() < 1.0, + "content height {} != {}", + large_content.y, + expected_height(400) + ); + + // Ten times the library, but the same viewport: the frame's paint work must stay + // roughly constant. Without virtualization the 400-snippet grid emitted ten times + // the shapes of the 40-snippet one. + assert!( + large_shapes <= small_shapes * 3 / 2, + "large grid emitted {large_shapes} shapes vs {small_shapes} for a tenth of the library" + ); + assert!(small_shapes > 20, "the visible cards must actually be painted"); + + // Scrolled deep into the library, cards are still painted (i.e. the visible band + // follows the viewport instead of staying at the top)... + let deep_offset = 60.0 * ROW_PITCH; + let (deep_shapes, deep_rects, _, _) = run_grid(&large, deep_offset); + assert!( + deep_shapes >= small_shapes / 2, + "scrolled grid emitted only {deep_shapes} shapes" + ); + + // ...and the reported geometry is a uniform grid whose rows are ROW_PITCH apart, + // shifted by the scroll offset. + for (i, r) in deep_rects.iter().enumerate() { + assert!((r.width() - CARD_W).abs() < 0.1); + assert!((r.height() - CARD_H).abs() < 0.1); + let expected = grid_card_rect(i, 2, deep_rects[0].min, CARD_W, CARD_H, CARD_SPACING); + assert!(r.min.distance(expected.min) < 0.1, "card {i} at {:?}", r.min); + } + assert!( + (deep_rects[0].min.y - (large_rects[0].min.y - deep_offset)).abs() < 1.0, + "scrolling must shift the grid geometry by the scroll offset" + ); + + // Drag-and-drop can still target a gap on a row that was never laid out: the + // pointer sits in the row-boundary gap after card 150. + let gap_index = 150; + let above = deep_rects[gap_index - 2]; + let below = deep_rects[gap_index]; + let pointer = egui::pos2( + above.center().x, + (above.bottom() + below.top()) * 0.5, + ); + assert_eq!( + nearest_gap(pointer, &deep_rects, 2, CARD_SPACING, CARD_W), + gap_index, + "an off-screen gap must still be a valid drop target" + ); + } + + /// The grid only lays out the rows in view, so the rects it hands to the + /// drag-and-drop code are computed from the grid origin instead of harvested from + /// the layout. Those computed rects must match what an actually-rendered card gets, + /// or every drop target would be off. + #[test] + fn computed_grid_rects_match_rendered_cards() { + let ctx = egui::Context::default(); + let input = egui::RawInput { + screen_rect: Some(egui::Rect::from_min_size( + egui::pos2(0.0, 0.0), + egui::vec2(1000.0, 700.0), + )), + ..Default::default() + }; + let app = test_app( + "computed-rects", + (1..=7).map(|i| snippet(i, "Git")).collect(), + vec!["Git".into()], + ); + let filtered: Vec = (0..app.snippets.len()).collect(); + + let _ = ctx.run(input, |ctx| { + egui::CentralPanel::default().show(ctx, |ui| { + egui::ScrollArea::vertical() + .auto_shrink([false; 2]) + .show(ui, |ui| { + ui.spacing_mut().item_spacing = egui::vec2(0.0, 0.0); + ui.add_space(4.0); + + let card_inner_w = 300.0_f32; + let card_inner_h = 168.0_f32; + let card_frame_margin = 20.0_f32; + let card_w = card_inner_w + card_frame_margin; + let card_h = card_inner_h + card_frame_margin; + let spacing = 12.0_f32; + + egui::Frame::none() + .inner_margin(egui::Margin::symmetric(18.0, 0.0)) + .show(ui, |ui| { + let avail = ui.available_width(); + let cols = ((avail / (card_w + spacing)).floor() as usize).max(1); + let origin = ui.cursor().min; + assert_eq!(cols, 2); + + for (i, chunk_start) in (0..filtered.len()).step_by(cols).enumerate() + { + let row = &filtered[chunk_start + ..(chunk_start + cols).min(filtered.len())]; + ui.horizontal(|ui| { + ui.spacing_mut().item_spacing = egui::vec2(0.0, 0.0); + for (offset, &idx) in row.iter().enumerate() { + let mut actions = Vec::new(); + let rendered = app + .card(ui, idx, card_inner_w, 0.0, &mut actions, false) + .frame_rect; + let computed = grid_card_rect( + i * cols + offset, + cols, + origin, + card_w, + card_h, + spacing, + ); + assert!( + rendered.min.distance(computed.min) < 0.1 + && rendered.max.distance(computed.max) < 0.1, + "card {} rendered at {:?}, computed {:?}", + i * cols + offset, + rendered, + computed, + ); + ui.add_space(spacing); + } + }); + ui.add_space(spacing); + } + }); + }); + }); + }); + } + + /// Row virtualization must cover everything the viewport can show (plus a row of + /// overscan) and never hand back a range outside the grid. + #[test] + fn visible_rows_covers_the_viewport_and_stays_in_bounds() { + let row_pitch = 200.0_f32; + let origin_y = 100.0_f32; + let rows = 50; + + // Scrolled to the top: starts at row 0, reaches past the bottom of the viewport. + let clip = egui::Rect::from_min_max(egui::pos2(0.0, 100.0), egui::pos2(1000.0, 700.0)); + let (first, last) = visible_rows(clip, origin_y, row_pitch, rows); + assert_eq!(first, 0); + assert!(last >= 3, "viewport spans 3 rows, got last = {last}"); + assert!(last < rows); + + // Scrolled into the middle: the visible band is covered with overscan on both + // sides, and rows far above/below are skipped. + let clip = egui::Rect::from_min_max(egui::pos2(0.0, 2100.0), egui::pos2(1000.0, 2700.0)); + let (first, last) = visible_rows(clip, origin_y, row_pitch, rows); + assert!((8..=9).contains(&first), "first = {first}"); + assert!(last >= 13, "last = {last}"); + assert!(first > 0, "rows above the viewport must be skipped"); + assert!(last < rows - 1, "rows below the viewport must be skipped"); + + // Every row of the grid is reachable by some scroll position. + for row in 0..rows { + let y = origin_y + row as f32 * row_pitch; + let clip = egui::Rect::from_min_max(egui::pos2(0.0, y), egui::pos2(1000.0, y + 10.0)); + let (first, last) = visible_rows(clip, origin_y, row_pitch, rows); + assert!( + first <= row && row <= last, + "row {row} not rendered for its own scroll position ({first}..={last})" + ); + assert!(last < rows); + } + + // Degenerate inputs fall back to rendering everything rather than nothing. + assert_eq!(visible_rows(clip, origin_y, 0.0, rows), (0, rows - 1)); + assert_eq!(visible_rows(clip, f32::NAN, row_pitch, rows), (0, rows - 1)); + assert_eq!(visible_rows(clip, origin_y, row_pitch, 1), (0, 0)); + assert_eq!(visible_rows(clip, origin_y, row_pitch, 0), (0, 0)); + } + + /// The filter is memoized: it must return the same indices the old + /// scan-every-frame code did, and it must be recomputed when the query, the + /// category, or the library changes. + #[test] + fn filter_cache_matches_a_fresh_scan_and_invalidates_on_change() { + let mut app = test_app( + "filter-cache", + vec![ + Snippet { + id: 1, + title: "Rebase onto main".into(), + category: "Git".into(), + body: "git rebase origin/MAIN".into(), + }, + Snippet { + id: 2, + title: "Summarize".into(), + category: "Prompt".into(), + body: "Summarize the following text".into(), + }, + Snippet { + id: 3, + title: "Stash".into(), + category: "Git".into(), + body: "git stash pop".into(), + }, + ], + vec!["Git".into(), "Prompt".into()], + ); + + // Reference implementation: the un-cached filter this replaced. + let expected = |app: &CopyIt| -> Vec { + let q = app.search.trim().to_lowercase(); + app.snippets + .iter() + .enumerate() + .filter(|(_, s)| { + (app.category_filter == "All" || s.category == app.category_filter) + && (q.is_empty() + || s.title.to_lowercase().contains(&q) + || s.body.to_lowercase().contains(&q) + || s.category.to_lowercase().contains(&q)) + }) + .map(|(i, _)| i) + .collect() + }; + let check = |app: &mut CopyIt| { + let want = expected(app); + let got = app.take_filtered(); + assert_eq!(got, want, "search {:?} / cat {:?}", app.search, app.category_filter); + app.restore_filtered(got); + }; + + check(&mut app); // everything + + // Taking the buffer twice without restoring it must not report "no matches". + let first = app.take_filtered(); + let second = app.take_filtered(); + assert_eq!(first, second, "a checked-out cache must be recomputed, not reused"); + app.restore_filtered(second); + + // A cached result must not survive a changed query... + app.search = "GIT".into(); // case-insensitive, matches bodies and the category + check(&mut app); + app.search = " summarize ".into(); // trimmed + check(&mut app); + app.search = " ".into(); // whitespace-only behaves like empty + check(&mut app); + assert_eq!(app.take_filtered().len(), 3); + let restored = vec![0, 1, 2]; + app.restore_filtered(restored); + + // ...a changed category filter... + app.search.clear(); + app.category_filter = "Git".into(); + check(&mut app); + assert_eq!(app.take_filtered(), vec![0, 2]); + app.restore_filtered(vec![0, 2]); + + // ...or a changed library. Reordering keeps ids and derived data aligned. + app.category_filter = "All".into(); + app.reorder(1, 3, &[0, 1, 2]); + assert_eq!(ids(&app), vec![2, 3, 1]); + check(&mut app); + app.search = "rebase".into(); + assert_eq!(app.take_filtered(), vec![2], "the moved card is still findable"); + app.restore_filtered(vec![2]); + + // Deriving must track edits to a snippet's text, not just its position. + app.snippets[2].body = "git rebase --abort".into(); + app.snippets[2].title = "Abort".into(); + app.snippets_changed(); + app.search = "abort".into(); + check(&mut app); + assert_eq!(app.take_filtered(), vec![2]); + } + + /// Card previews are cached; they must still collapse whitespace, truncate with an + /// ellipsis, and count characters rather than bytes. + #[test] + fn preview_text_collapses_and_truncates() { + assert_eq!(preview_text(" git stash \n pop ", 220), "git stash pop"); + assert_eq!(preview_text("", 220), ""); + + let long = "word ".repeat(400); + let preview = preview_text(&long, 10); + assert_eq!(preview.chars().count(), 10); + assert!(preview.ends_with('\u{2026}')); + assert!(preview.starts_with("word word")); + + // Multi-byte characters are counted as characters. + let unicode = "\u{00e9}".repeat(50); + assert_eq!(preview_text(&unicode, 10).chars().count(), 10); + assert_eq!(preview_text(&unicode, 100), unicode); + + // The cached preview is what the card renders. + let app = test_app( + "preview-cache", + vec![Snippet { + id: 1, + title: "T".into(), + category: "Git".into(), + body: " first line\nsecond line ".into(), + }], + vec!["Git".into()], + ); + assert_eq!(app.derived[0].preview, "first line second line"); + assert_eq!(app.derived[0].body_lower, " first line\nsecond line "); + } + #[test] fn add_category_normalizes_and_dedups() { let mut app = test_app( diff --git a/src/storage.rs b/src/storage.rs index 9d208b4..ddb118d 100644 --- a/src/storage.rs +++ b/src/storage.rs @@ -192,7 +192,15 @@ pub fn normalize_category(s: &str) -> String { /// True when two category names denote the same category, ignoring case. /// Uses full Unicode lowercasing (not `eq_ignore_ascii_case`) so accented /// categories don't sneak in as near-duplicates. +/// +/// Category names are almost always plain ASCII, and this runs once per known +/// category on every lookup, so that case is answered without allocating; anything +/// non-ASCII still goes through full Unicode lowercasing, which for ASCII input +/// produces exactly the same result. pub fn same_category(a: &str, b: &str) -> bool { + if a.is_ascii() && b.is_ascii() { + return a.eq_ignore_ascii_case(b); + } a.to_lowercase() == b.to_lowercase() } @@ -243,18 +251,25 @@ impl Config { } } - /// Adds a category to the canonical list if it isn't already present (case-insensitive). - /// Normalizes the input, rejects empty strings and "All" (reserved), and keeps the list sorted. - pub fn add_category(&mut self, raw: &str) { - let cat = normalize_category(raw); - if is_reserved_category(&cat) { - return; + /// Adds categories to the canonical list, skipping any that are already present + /// (case-insensitive) or reserved, and normalizing the rest. The list is sorted a + /// single time at the end: startup registers every snippet's category, which used + /// to re-sort the whole list once per snippet. + pub fn add_categories<'a>(&mut self, raws: impl IntoIterator) { + let before = self.categories.len(); + for raw in raws { + let cat = normalize_category(raw); + if is_reserved_category(&cat) { + continue; + } + if self.categories.iter().any(|c| same_category(c, &cat)) { + continue; + } + self.categories.push(cat); } - if self.categories.iter().any(|c| same_category(c, &cat)) { - return; + if self.categories.len() != before { + self.categories.sort(); } - self.categories.push(cat); - self.categories.sort(); } } @@ -308,14 +323,34 @@ mod tests { } #[test] - fn config_add_category_dedupes_case_insensitively() { + fn config_add_categories_dedupes_case_insensitively() { let mut config = Config::default(); - config.add_category("git"); - config.add_category("GIT"); - config.add_category(" Git "); - config.add_category("all"); - config.add_category(""); + config.add_categories(["git", "GIT", " Git ", "all", ""]); assert_eq!(config.categories, vec!["Git".to_string()]); + + // Adding in batches must behave like adding one at a time, and keep the + // canonical list sorted. + config.add_categories(["prompt"]); + config.add_categories(["Docker", "prompt", "ansible"]); + assert_eq!( + config.categories, + vec![ + "Ansible".to_string(), + "Docker".to_string(), + "Git".to_string(), + "Prompt".to_string(), + ] + ); + } + + #[test] + fn same_category_ignores_case_for_ascii_and_unicode() { + assert!(same_category("git", "GIT")); + assert!(same_category("Git Hub", "git hub")); + assert!(!same_category("git", "gitt")); + // Non-ASCII names still go through full Unicode lowercasing. + assert!(same_category("Café", "CAFÉ")); + assert!(!same_category("Café", "Cafe")); } #[test] diff --git a/src/theme.rs b/src/theme.rs index 0a7a962..b8c466d 100644 --- a/src/theme.rs +++ b/src/theme.rs @@ -1,8 +1,9 @@ //! Color themes and visual styling for CopyIt. //! //! Provides 37 selectable color themes, each with custom egui::Visuals for background, -//! text, panels, buttons, and accent colors. Themes are persisted in config.json -//! and applied every frame via update(). +//! text, panels, buttons, and accent colors. Themes are persisted in config.json and +//! applied to the egui context once at startup and again whenever the user picks a +//! different one — not rebuilt every frame. use eframe::egui; use std::fmt; @@ -96,7 +97,8 @@ impl Theme { ] } - /// Generates the complete egui::Visuals color scheme for this theme. Applied to egui context each frame. + /// Generates the complete egui::Visuals color scheme for this theme. Call it only when + /// the selected theme changes; the context keeps the visuals until they are replaced. pub fn visuals(self) -> egui::Visuals { match self { Theme::Dark => dark_theme(), @@ -138,50 +140,59 @@ impl Theme { Theme::HorizonDark => horizon_dark_theme(), } } + + /// The theme's human-readable name as a static string (e.g. "Solarized Dark"). + /// Used by the theme selector and by `Display`: returning `&'static str` means + /// naming a theme (once per frame for the selected one, once per entry when the + /// dropdown is open) no longer allocates a fresh `String`. + pub fn name(self) -> &'static str { + match self { + Theme::Dark => "Dark", + Theme::Light => "Light", + Theme::Nord => "Nord", + Theme::Dracula => "Dracula", + Theme::SolarizedDark => "Solarized Dark", + Theme::SolarizedLight => "Solarized Light", + Theme::GruvboxDark => "Gruvbox Dark", + Theme::GruvboxLight => "Gruvbox Light", + Theme::CatppuccinMocha => "Catppuccin Mocha", + Theme::CatppuccinLatte => "Catppuccin Latte", + Theme::CatppuccinFrappe => "Catppuccin Frappe", + Theme::CatppuccinMacchiato => "Catppuccin Macchiato", + Theme::TokyoNight => "Tokyo Night", + Theme::TokyoNightStorm => "Tokyo Night Storm", + Theme::TokyoNightLight => "Tokyo Night Light", + Theme::OneDark => "One Dark", + Theme::OneLight => "One Light", + Theme::Monokai => "Monokai", + Theme::MonokaiPro => "Monokai Pro", + Theme::GithubDark => "GitHub Dark", + Theme::GithubLight => "GitHub Light", + Theme::AyuDark => "Ayu Dark", + Theme::AyuLight => "Ayu Light", + Theme::AyuMirage => "Ayu Mirage", + Theme::RosePine => "Rose Pine", + Theme::RosePineMoon => "Rose Pine Moon", + Theme::RosePineDawn => "Rose Pine Dawn", + Theme::EverforestDark => "Everforest Dark", + Theme::EverforestLight => "Everforest Light", + Theme::MaterialOcean => "Material Ocean", + Theme::MaterialPalenight => "Material Palenight", + Theme::Kanagawa => "Kanagawa", + Theme::NightOwl => "Night Owl", + Theme::Zenburn => "Zenburn", + Theme::SynthwaveEighties => "Synthwave '84", + Theme::Cobalt2 => "Cobalt2", + Theme::HorizonDark => "Horizon Dark", + } + } } -/// Converts a theme to its human-readable display string (e.g., "Solarized Dark"). Used in UI dropdowns and config serialization. +/// Converts a theme to its human-readable display string (e.g., "Solarized Dark"). +/// Delegates to [`Theme::name`] so the two can never disagree. impl fmt::Display for Theme { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - match self { - Theme::Dark => write!(f, "Dark"), - Theme::Light => write!(f, "Light"), - Theme::Nord => write!(f, "Nord"), - Theme::Dracula => write!(f, "Dracula"), - Theme::SolarizedDark => write!(f, "Solarized Dark"), - Theme::SolarizedLight => write!(f, "Solarized Light"), - Theme::GruvboxDark => write!(f, "Gruvbox Dark"), - Theme::GruvboxLight => write!(f, "Gruvbox Light"), - Theme::CatppuccinMocha => write!(f, "Catppuccin Mocha"), - Theme::CatppuccinLatte => write!(f, "Catppuccin Latte"), - Theme::CatppuccinFrappe => write!(f, "Catppuccin Frappe"), - Theme::CatppuccinMacchiato => write!(f, "Catppuccin Macchiato"), - Theme::TokyoNight => write!(f, "Tokyo Night"), - Theme::TokyoNightStorm => write!(f, "Tokyo Night Storm"), - Theme::TokyoNightLight => write!(f, "Tokyo Night Light"), - Theme::OneDark => write!(f, "One Dark"), - Theme::OneLight => write!(f, "One Light"), - Theme::Monokai => write!(f, "Monokai"), - Theme::MonokaiPro => write!(f, "Monokai Pro"), - Theme::GithubDark => write!(f, "GitHub Dark"), - Theme::GithubLight => write!(f, "GitHub Light"), - Theme::AyuDark => write!(f, "Ayu Dark"), - Theme::AyuLight => write!(f, "Ayu Light"), - Theme::AyuMirage => write!(f, "Ayu Mirage"), - Theme::RosePine => write!(f, "Rose Pine"), - Theme::RosePineMoon => write!(f, "Rose Pine Moon"), - Theme::RosePineDawn => write!(f, "Rose Pine Dawn"), - Theme::EverforestDark => write!(f, "Everforest Dark"), - Theme::EverforestLight => write!(f, "Everforest Light"), - Theme::MaterialOcean => write!(f, "Material Ocean"), - Theme::MaterialPalenight => write!(f, "Material Palenight"), - Theme::Kanagawa => write!(f, "Kanagawa"), - Theme::NightOwl => write!(f, "Night Owl"), - Theme::Zenburn => write!(f, "Zenburn"), - Theme::SynthwaveEighties => write!(f, "Synthwave '84"), - Theme::Cobalt2 => write!(f, "Cobalt2"), - Theme::HorizonDark => write!(f, "Horizon Dark"), - } + f.write_str(self.name()) } }