From c75de34a012e04c93012c55256e05924914864b6 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 13 Sep 2026 12:54:30 -0700 Subject: [PATCH 01/16] docs: drop Linear from task creation --- CLAUDE.md | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 5cfbba66..5844eec9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -108,11 +108,8 @@ Notion is where a task is born. Never create a Notion task from a GitHub issue 1. Notion page in the Tasks Tracker. 2. `gh issue create --title "ASNT-: "`, body ending `**Notion:** `. Match the structure existing issues use (Overview / Scope / Task Details / Definition of Done / Estimate / Type). 3. Write the issue URL back into the Notion `GitHub Link` property. -4. Linear issue in the `qrate` project, via the Linear MCP (`save_issue`). It supports parent and - blocked-by relations natively, so mirror Notion's sub-task nesting and `Blocked by`/`Blocking` - rather than describing them in prose. Write the issue URL back into the Notion `Linear Link` - property — nothing propagates a link in the other direction. Check `list_issues` before creating - so a re-run does not duplicate. + +Linear is no longer used — don't create Linear issues or fill the `Linear Link` property. ## Code Style — no bloat From 54e58c698c449e46addd255e6485fe6194535795 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 13 Sep 2026 12:54:34 -0700 Subject: [PATCH 02/16] feat(history): log every data and note change in the project file Every edit, structural change, undo, redo and note change is recorded in __history/__history_changes, keyed by row id and column name, with its origin and author. Entries are written in the same transaction as the data they describe. Column moves are now undoable instead of clearing undo, rename undo re-keys notes and settings, and restore_to/restore_value put the grid back by identity as one undoable step. Refs #132 --- crates/clustering/src/lib.rs | 10 +- crates/diagnostics/src/fixes.rs | 38 +- crates/diagnostics/src/lib.rs | 121 ++++-- crates/diagnostics/src/panel.rs | 8 +- crates/project-wizard/src/steps/review.rs | 1 + crates/settings/src/history.rs | 397 ++++++++++++++++++++ crates/settings/src/lib.rs | 1 + crates/settings/src/project.rs | 32 +- crates/table/src/delegate.rs | 437 +++++++++++++++++++--- crates/table/src/editing.rs | 5 +- crates/table/src/history.rs | 16 + crates/table/src/lib.rs | 215 +++++++++-- crates/table/src/note.rs | 26 +- crates/table/src/panel.rs | 50 ++- crates/workspace/src/panels/details.rs | 2 +- 15 files changed, 1172 insertions(+), 187 deletions(-) create mode 100644 crates/settings/src/history.rs diff --git a/crates/clustering/src/lib.rs b/crates/clustering/src/lib.rs index 0141b8bf..7d60cca4 100644 --- a/crates/clustering/src/lib.rs +++ b/crates/clustering/src/lib.rs @@ -181,12 +181,14 @@ pub fn variant_group_fixes(members: &[GroupMember], cx: &App) -> Vec { .map(|target| { let (target, members, delimiter) = (target.clone(), members.to_vec(), delimiter.clone()); - GroupFix::action(format!("Change all to “{target}”"), move |cx| { + let label = format!("Change all to “{target}”"); + let origin = settings::history::Origin::Fix(label.clone()); + GroupFix::action(label, move |cx| { let Some(replacements) = replacements_to(&members, &target, &delimiter) else { return; }; if let Some(hooks) = cx.try_global::().copied() { - (hooks.set_texts)(replacements, cx); + (hooks.set_texts)(replacements, origin.clone(), cx); } }) }) @@ -455,8 +457,8 @@ mod tests { cx.set_global(diagnostics::DiagnosticHooks { reveal: |_, _| {}, text_at: |_, _| None, - set_text: |_, _, _| {}, - set_texts: |replacements, cx| { + set_text: |_, _, _, _| {}, + set_texts: |replacements, _, cx| { use gpui::BorrowAppContext as _; cx.update_global::(|applied, _| applied.0 = replacements); }, diff --git a/crates/diagnostics/src/fixes.rs b/crates/diagnostics/src/fixes.rs index cb2e8ef1..64591c4b 100644 --- a/crates/diagnostics/src/fixes.rs +++ b/crates/diagnostics/src/fixes.rs @@ -17,6 +17,7 @@ use gpui_component::WindowExt as _; use gpui_component::button::Button; use gpui_component::menu::{PopupMenu, PopupMenuItem}; use gpui_component::notification::Notification; +use settings::history::Origin; use settings::project::RowId; use crate::{Diagnostic, DiagnosticHooks, Diagnostics, Location, SpellActions}; @@ -52,11 +53,13 @@ impl GroupFix { label: impl Into, replacements: Vec<(Location, SharedString)>, ) -> Self { + let label = label.into(); + let origin = Origin::Fix(label.to_string()); Self { - label: label.into(), + label, apply: Rc::new(move |cx| { if let Some(hooks) = cx.try_global::().copied() { - (hooks.set_texts)(replacements.clone(), cx); + (hooks.set_texts)(replacements.clone(), origin.clone(), cx); } }), } @@ -216,19 +219,20 @@ pub fn finding_menu( text: Option<&SharedString>, menu: PopupMenu, cx: &mut Context, - apply: impl Fn(SharedString, &mut App) + Clone + 'static, + apply: impl Fn(SharedString, Origin, &mut App) + Clone + 'static, ) -> PopupMenu { let source = finding.source.key(); let subject = finding.group.as_ref().and_then(|g| g.subject.clone()); let mut menu = menu; if let Some(text) = text { - let fix = |menu: PopupMenu, label: SharedString, replacement: SharedString| { - let apply = apply.clone(); - menu.item( - PopupMenuItem::new(menu_label(&label)) - .on_click(move |_, _, cx| apply(replacement.clone(), cx)), - ) - }; + let fix = + |menu: PopupMenu, label: SharedString, replacement: SharedString, origin: Origin| { + let apply = apply.clone(); + menu.item( + PopupMenuItem::new(menu_label(&label)) + .on_click(move |_, _, cx| apply(replacement.clone(), origin.clone(), cx)), + ) + }; if source == "spell" && let Some(actions) = cx.try_global::().copied() { @@ -237,7 +241,12 @@ pub fn finding_menu( for (word, suggestions) in &found { for suggestion in suggestions { let fixed = text.replace(word.as_ref(), suggestion.as_ref()); - menu = fix(menu, format!("{word} → {suggestion}").into(), fixed.into()); + menu = fix( + menu, + format!("{word} → {suggestion}").into(), + fixed.into(), + Origin::Spelling, + ); } } menu = menu.separator(); @@ -256,7 +265,8 @@ pub fn finding_menu( cx, ); for found in offered { - menu = fix(menu, found.label, found.replacement); + let origin = Origin::Fix(found.label.to_string()); + menu = fix(menu, found.label, found.replacement, origin); } } let menu = with_source_actions(&source, menu, cx); @@ -463,8 +473,8 @@ mod tests { cx.set_global(DiagnosticHooks { reveal: |_, _| {}, text_at: |_, _| None, - set_text: |_, _, _| {}, - set_texts: |replacements, cx| { + set_text: |_, _, _, _| {}, + set_texts: |replacements, _, cx| { use gpui::BorrowAppContext as _; cx.update_global::(|applied, _| applied.0 = replacements); }, diff --git a/crates/diagnostics/src/lib.rs b/crates/diagnostics/src/lib.rs index 9d7df010..d365a4cf 100644 --- a/crates/diagnostics/src/lib.rs +++ b/crates/diagnostics/src/lib.rs @@ -20,6 +20,7 @@ use std::path::PathBuf; use gpui::{App, Global, Hsla, SharedString}; use gpui_component::ActiveTheme as _; +use settings::history::{Change, Entry, Origin}; /// The one dataset a project can hold today. `__notes` keys by name so a second sheet is new /// rows rather than a new table. @@ -442,6 +443,7 @@ impl Diagnostics { return; } let filed = Self::filed_now(cx); + let logged = Self::note_entry(&location, None, Some(&message), Origin::Typed, cx); let this = cx.default_global::(); this.items.push(Diagnostic { location, @@ -452,7 +454,7 @@ impl Diagnostics { filed, }); this.reindex(); - this.persist(); + this.persist(&logged); } /// Stamp for a note being filed right now: today's date from the project file's own clock, and @@ -462,33 +464,53 @@ impl Diagnostics { .try_global::() .and_then(|p| settings::project::today(&p.file)) .map(SharedString::from); - // Guarded: a note can be filed before the settings globals exist (early startup, tests), - // and an unsigned note is a far better outcome than a panic mid-keystroke. - let author = match cx.has_global::() { - false => None, - true => match settings::effective_text(settings::NOTE_AUTHOR_KEY, cx) { - author if author.trim().is_empty() => None, - author => Some(author), - }, - }; + let author = settings::history::author(cx).map(SharedString::from); (date.is_some() || author.is_some()).then_some(Filed { date, author }) } + /// The log entry for one note going from `before` to `after`. Empty when nothing changed, or + /// when no project is open to log it in. + fn note_entry( + location: &Location, + before: Option<&SharedString>, + after: Option<&SharedString>, + origin: Origin, + cx: &App, + ) -> Vec { + if before == after || location.dataset != DATASET_MAIN { + return Vec::new(); + } + let change = Change::Note { + row: location.row_id, + column: location.column.as_ref().map(|c| c.to_string()), + before: before.map(|b| b.to_string()), + after: after.map(|a| a.to_string()), + }; + vec![Entry::new( + origin, + vec![change], + settings::history::author(cx), + )] + } + /// Attach a note here, replacing any note already at this location; an empty `message` deletes /// it. Writes straight through to `__notes` — this is a deliberate keystroke, not the hot path /// the debounced setting writer exists for. - pub fn set_note(location: Location, message: SharedString, cx: &mut App) { - // Keeps the original filing stamp: correcting a transcription is not re-observing the - // item, and re-dating it to today would erase when the observation was actually made. - let filed = Self::notes_at( + pub fn set_note(location: Location, message: SharedString, origin: Origin, cx: &mut App) { + let (before, filed) = Self::notes_at( &location.dataset, location.row, location.column.as_deref(), cx, ) .next() - .and_then(|d| d.filed.clone()) - .or_else(|| Self::filed_now(cx)); + .map(|d| (Some(d.message.clone()), d.filed.clone())) + .unwrap_or_default(); + let after = (!message.trim().is_empty()).then_some(&message); + let logged = Self::note_entry(&location, before.as_ref(), after, origin, cx); + // Keeps the original filing stamp: correcting a transcription is not re-observing the + // item, and re-dating it to today would erase when the observation was actually made. + let filed = filed.or_else(|| Self::filed_now(cx)); let this = cx.default_global::(); this.items .retain(|d| d.source != Source::Note || d.location != location); @@ -503,12 +525,18 @@ impl Diagnostics { }); } this.reindex(); - this.persist(); + this.persist(&logged); } /// Rebind authored notes to the current source positions after a structural edit or its undo. /// Their stable ids decide ownership; positions are only the live UI address. - pub fn align_note_rows(dataset: &str, row_ids: &[settings::project::RowId], cx: &mut App) { + pub fn align_note_rows( + dataset: &str, + row_ids: &[settings::project::RowId], + origin: Origin, + cx: &mut App, + ) { + let author = settings::history::author(cx); let positions: HashMap<_, _> = row_ids .iter() .copied() @@ -517,6 +545,7 @@ impl Diagnostics { .collect(); let this = cx.default_global::(); let mut changed = false; + let mut dropped = Vec::new(); this.items.retain_mut(|item| { if item.location.dataset != dataset || item.source != Source::Note { return true; @@ -526,6 +555,12 @@ impl Diagnostics { }; let Some(source) = positions.get(&row_id).copied() else { changed = true; + dropped.push(Change::Note { + row: Some(row_id), + column: item.location.column.as_ref().map(|c| c.to_string()), + before: Some(item.message.to_string()), + after: None, + }); return false; }; changed |= item.location.row != Some(source); @@ -533,8 +568,12 @@ impl Diagnostics { true }); if changed { + let logged = match dropped.is_empty() { + true => Vec::new(), + false => vec![Entry::new(origin, dropped, author)], + }; this.reindex(); - this.persist(); + this.persist(&logged); } } @@ -546,12 +585,12 @@ impl Diagnostics { item.location.column = Some(after.clone()); } } - this.persist(); + this.persist(&[]); } /// Write every authored note back to `__notes`, replacing what was there. Computed findings /// are never persisted — they are recomputed on open and a stored copy would go stale. - fn persist(&self) { + fn persist(&self, history: &[Entry]) { // No open project (tests, early startup) leaves the change in memory only. let Some(file) = self.loaded.as_ref() else { return; @@ -577,7 +616,7 @@ impl Diagnostics { .and_then(|f| f.author.as_ref().map(SharedString::to_string)), }) .collect(); - if let Err(err) = settings::project::write_notes(file, SOURCE_NOTE, ¬es) { + if let Err(err) = settings::project::write_notes(file, SOURCE_NOTE, ¬es, history) { log::error!("failed to save notes: {err}"); } } @@ -646,9 +685,9 @@ pub struct DiagnosticHooks { pub text_at: fn(&Location, &App) -> Option, /// Write text back to a location and revalidate. The other half of [`Self::text_at`], and the /// reason a panel row can offer the same corrections a cell does. - pub set_text: fn(&Location, SharedString, &mut App), + pub set_text: fn(&Location, SharedString, Origin, &mut App), /// Apply a group resolution as one undoable edit and one validation pass. - pub set_texts: fn(Vec<(Location, SharedString)>, &mut App), + pub set_texts: fn(Vec<(Location, SharedString)>, Origin, &mut App), /// Re-run validation after a resolution changes validator settings instead of cell text. pub revalidate: fn(&mut App), } @@ -662,6 +701,7 @@ mod tests { // emits, recursing until rustc's stack overflows. use crate::{DATASET_MAIN, Diagnostic, Diagnostics, Filed, Location, Severity, Source, init}; use gpui::{App, SharedString, TestAppContext}; + use settings::history::Origin; use settings::project::{CurrentProject, ProjectData, ProjectSpec, StoredNote}; /// The row index is a second copy of the truth in `items`, so every mutation has to rebuild @@ -691,11 +731,11 @@ mod tests { row_id: None, column: Some("Title".into()), }; - Diagnostics::set_note(location.clone(), "hand written".into(), cx); + Diagnostics::set_note(location.clone(), "hand written".into(), Origin::Typed, cx); assert_eq!(at_00(cx), 2, "the note joins the validator's finding"); // An empty message deletes the note, which shifts every later index in `items`. - Diagnostics::set_note(location, "".into(), cx); + Diagnostics::set_note(location, "".into(), Origin::Typed, cx); assert_eq!(at_00(cx), 1); // Republishing nothing is how a validator clears itself. @@ -787,7 +827,12 @@ mod tests { cx.default_global::().items[0].filed = original.clone(); // set_note collapses to one and keeps that stamp. - Diagnostics::set_note(cell, "verso inscription, in pencil".into(), cx); + Diagnostics::set_note( + cell, + "verso inscription, in pencil".into(), + Origin::Typed, + cx, + ); let notes = filed(cx); assert_eq!(notes.len(), 1, "a correction replaces rather than adds"); assert_eq!(notes[0].0, "verso inscription, in pencil"); @@ -858,6 +903,7 @@ mod tests { author: None, }, ], + &[], ) .unwrap(); @@ -940,12 +986,12 @@ mod tests { }); init(cx); - Diagnostics::set_note(cell.clone(), "first".into(), cx); - Diagnostics::set_note(whole_row.clone(), "row wide".into(), cx); + Diagnostics::set_note(cell.clone(), "first".into(), Origin::Typed, cx); + Diagnostics::set_note(whole_row.clone(), "row wide".into(), Origin::Typed, cx); assert_eq!(Diagnostics::all(cx).len(), 2); // Same location again replaces rather than stacking. - Diagnostics::set_note(cell.clone(), "second".into(), cx); + Diagnostics::set_note(cell.clone(), "second".into(), Origin::Typed, cx); assert_eq!(Diagnostics::all(cx).len(), 2); assert_eq!( Diagnostics::note_at(DATASET_MAIN, Some(3), Some("Title"), cx), @@ -973,7 +1019,7 @@ mod tests { ); // An empty message deletes the note and leaves the validator's entry standing. - Diagnostics::set_note(cell.clone(), " ".into(), cx); + Diagnostics::set_note(cell.clone(), " ".into(), Origin::Typed, cx); assert_eq!( Diagnostics::note_at(DATASET_MAIN, Some(3), Some("Title"), cx), None @@ -1000,10 +1046,15 @@ mod tests { row_id: Some(row_id), column: Some("Title".into()), }; - Diagnostics::set_note(note(1, 10), "ten".into(), cx); - Diagnostics::set_note(note(3, 20), "twenty".into(), cx); + Diagnostics::set_note(note(1, 10), "ten".into(), Origin::Typed, cx); + Diagnostics::set_note(note(3, 20), "twenty".into(), Origin::Typed, cx); - Diagnostics::align_note_rows(DATASET_MAIN, &[99, 10, 11, 12, 20], cx); + Diagnostics::align_note_rows( + DATASET_MAIN, + &[99, 10, 11, 12, 20], + Origin::Structure, + cx, + ); assert_eq!( Diagnostics::note_at(DATASET_MAIN, Some(1), Some("Title"), cx), Some("ten".into()) @@ -1013,7 +1064,7 @@ mod tests { Some("twenty".into()) ); - Diagnostics::align_note_rows(DATASET_MAIN, &[99, 11, 12, 20], cx); + Diagnostics::align_note_rows(DATASET_MAIN, &[99, 11, 12, 20], Origin::Structure, cx); assert_eq!( Diagnostics::all(cx).len(), 1, diff --git a/crates/diagnostics/src/panel.rs b/crates/diagnostics/src/panel.rs index a2ce6ba6..f7b06a83 100644 --- a/crates/diagnostics/src/panel.rs +++ b/crates/diagnostics/src/panel.rs @@ -829,8 +829,8 @@ impl Render for ProblemsPanel { text.as_ref(), menu, cx, - move |fixed, cx| { - (hooks.set_text)(&applied, fixed, cx) + move |fixed, origin, cx| { + (hooks.set_text)(&applied, fixed, origin, cx) }, ); } @@ -1058,8 +1058,8 @@ mod tests { cx.update_global::(|seen, _| seen.0.push(location.clone())); }, text_at: |_, _| None, - set_text: |_, _, _| panic!("navigation must not edit"), - set_texts: |_, _| panic!("navigation must not edit"), + set_text: |_, _, _, _| panic!("navigation must not edit"), + set_texts: |_, _, _| panic!("navigation must not edit"), revalidate: |_| panic!("navigation must not revalidate"), }); Diagnostics::set( diff --git a/crates/project-wizard/src/steps/review.rs b/crates/project-wizard/src/steps/review.rs index 040748c5..90e68d8b 100644 --- a/crates/project-wizard/src/steps/review.rs +++ b/crates/project-wizard/src/steps/review.rs @@ -248,6 +248,7 @@ impl ProjectWizard { std::path::Path::new(&file), diagnostics::SOURCE_NOTE, ¬es, + &[], ) { log::error!("couldn't save the sheet's notes — {e}"); } diff --git a/crates/settings/src/history.rs b/crates/settings/src/history.rs new file mode 100644 index 00000000..1eb72117 --- /dev/null +++ b/crates/settings/src/history.rs @@ -0,0 +1,397 @@ +//! The project's change log: every edit to the data and its notes, kept in the `.qrate` file for +//! good. Append-only — a restore is a new entry, never a rewind — so the log is an audit trail, +//! not an undo stack. +//! +//! Changes are keyed by stable [`RowId`] and column name rather than grid positions, which is what +//! lets an entry written a year ago still be read, and reversed, against today's grid. Positions +//! ride along only as the place to put a row or column back. +//! +//! Entries reach disk inside the same transaction as the data they describe (see +//! `project::save_dataset` and `project::write_notes`), so the log never claims a change the file +//! doesn't hold. + +use std::path::Path; +use std::time::{SystemTime, UNIX_EPOCH}; + +use anyhow::{Context as _, Result}; +use gpui::App; +use rusqlite::{Connection, params}; +use serde::{Deserialize, Serialize}; + +use crate::project::RowId; + +pub type EntryId = i64; + +/// Where a change came from. The difference between a value an archivist typed and one they +/// accepted from a fix is the point of keeping a log at all. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub enum Origin { + Typed, + Paste, + Clear, + ReplaceAll, + Details, + Spelling, + /// A fix from the Fixes menu or the Problems panel, named by what offered it. + Fix(String), + /// Rows or columns added, removed, renamed, or moved. + Structure, + Undo, + Redo, + /// Put back by restoring the project, or a single value, to this entry. + Restore(EntryId), +} + +/// One recorded change. Every variant is reversible from what it carries alone. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub enum Change { + Cell { + row: RowId, + column: String, + before: String, + after: String, + }, + RowAdded { + row: RowId, + position: usize, + cells: Vec<(String, String)>, + }, + RowRemoved { + row: RowId, + position: usize, + cells: Vec<(String, String)>, + }, + ColumnAdded { + column: String, + position: usize, + cells: Vec<(RowId, String)>, + }, + ColumnRemoved { + column: String, + position: usize, + cells: Vec<(RowId, String)>, + }, + ColumnRenamed { + before: String, + after: String, + }, + ColumnMoved { + column: String, + from: usize, + to: usize, + }, + /// `None` on either side is no note there. A row-less or column-less note widens to the whole + /// column or row, as it does in `__notes`. + Note { + row: Option, + column: Option, + before: Option, + after: Option, + }, +} + +impl Change { + /// The change that undoes this one. + pub fn inverse(&self) -> Change { + match self.clone() { + Change::Cell { + row, + column, + before, + after, + } => Change::Cell { + row, + column, + before: after, + after: before, + }, + Change::RowAdded { + row, + position, + cells, + } => Change::RowRemoved { + row, + position, + cells, + }, + Change::RowRemoved { + row, + position, + cells, + } => Change::RowAdded { + row, + position, + cells, + }, + Change::ColumnAdded { + column, + position, + cells, + } => Change::ColumnRemoved { + column, + position, + cells, + }, + Change::ColumnRemoved { + column, + position, + cells, + } => Change::ColumnAdded { + column, + position, + cells, + }, + Change::ColumnRenamed { before, after } => Change::ColumnRenamed { + before: after, + after: before, + }, + Change::ColumnMoved { column, from, to } => Change::ColumnMoved { + column, + from: to, + to: from, + }, + Change::Note { + row, + column, + before, + after, + } => Change::Note { + row, + column, + before: after, + after: before, + }, + } + } + + /// The row and column this change is looked up by. + fn key(&self) -> (Option, Option<&str>) { + match self { + Change::Cell { row, column, .. } => (Some(*row), Some(column)), + Change::RowAdded { row, .. } | Change::RowRemoved { row, .. } => (Some(*row), None), + Change::ColumnAdded { column, .. } + | Change::ColumnRemoved { column, .. } + | Change::ColumnMoved { column, .. } + | Change::ColumnRenamed { after: column, .. } => (None, Some(column)), + Change::Note { row, column, .. } => (*row, column.as_deref()), + } + } +} + +/// One user action: a paste, a restore, a single typed value — however many changes it made. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Entry { + /// `0` until the entry reaches disk. + pub id: EntryId, + /// Unix seconds, taken when the change was made rather than when it was saved. + pub at: i64, + pub author: Option, + pub origin: Origin, + pub changes: Vec, +} + +impl Entry { + pub fn new(origin: Origin, changes: Vec, author: Option) -> Self { + Self { + id: 0, + at: SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |d| d.as_secs() as i64), + author, + origin, + changes, + } + } +} + +/// Who the archivist has told qrate they are, shared with notes. `None` when unset — the log +/// stays unsigned rather than guessing from the OS account. +pub fn author(cx: &App) -> Option { + if !cx.has_global::() { + return None; + } + let author = crate::effective_text(crate::NOTE_AUTHOR_KEY, cx); + (!author.trim().is_empty()).then(|| author.to_string()) +} + +const HISTORY_DDL: &str = r#" + CREATE TABLE IF NOT EXISTS __history ( + id INTEGER PRIMARY KEY, + at INTEGER NOT NULL, + author TEXT, + origin TEXT NOT NULL, + label TEXT + ); + CREATE TABLE IF NOT EXISTS __history_changes ( + entry_id INTEGER NOT NULL REFERENCES __history(id), + seq INTEGER NOT NULL, + row_id INTEGER, + column_name TEXT, + change TEXT NOT NULL + ); + CREATE INDEX IF NOT EXISTS __history_changes_cell ON __history_changes(row_id, column_name); +"#; + +/// Write `entries` on `conn`, inside whatever transaction the caller holds. +pub(crate) fn append(conn: &Connection, entries: &[Entry]) -> Result<()> { + if entries.is_empty() { + return Ok(()); + } + conn.execute_batch(HISTORY_DDL) + .context("Create history tables")?; + let mut entry = + conn.prepare("INSERT INTO __history(at, author, origin) VALUES (?1, ?2, ?3)")?; + let mut change = conn.prepare( + "INSERT INTO __history_changes(entry_id, seq, row_id, column_name, change) + VALUES (?1, ?2, ?3, ?4, ?5)", + )?; + for e in entries.iter().filter(|e| !e.changes.is_empty()) { + entry + .execute(params![e.at, e.author, serde_json::to_string(&e.origin)?]) + .context("Insert history entry")?; + let id = conn.last_insert_rowid(); + for (seq, c) in e.changes.iter().enumerate() { + let (row, column) = c.key(); + change + .execute(params![id, seq, row, column, serde_json::to_string(c)?]) + .context("Insert history change")?; + } + } + Ok(()) +} + +/// Every entry newer than `after`, oldest first. A file that has never logged anything reads as +/// an empty history. +pub fn entries_after(path: &Path, after: EntryId) -> Result> { + let conn = crate::project::open_ro(path)?; + let exists: i64 = conn.query_row( + "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = '__history'", + [], + |r| r.get(0), + )?; + if exists == 0 { + return Ok(Vec::new()); + } + let mut entries = conn + .prepare("SELECT id, at, author, origin FROM __history WHERE id > ?1 ORDER BY id")? + .query_map([after], |r| { + Ok(( + r.get::<_, EntryId>(0)?, + r.get::<_, i64>(1)?, + r.get::<_, Option>(2)?, + r.get::<_, String>(3)?, + )) + })? + .map(|row| { + let (id, at, author, origin) = row?; + Ok(Entry { + id, + at, + author, + origin: serde_json::from_str(&origin).context("Read history origin")?, + changes: Vec::new(), + }) + }) + .collect::>>()?; + let mut changes = + conn.prepare("SELECT change FROM __history_changes WHERE entry_id = ?1 ORDER BY seq")?; + for entry in &mut entries { + entry.changes = changes + .query_map([entry.id], |r| r.get::<_, String>(0))? + .map(|c| serde_json::from_str(&c?).context("Read history change")) + .collect::>()?; + } + Ok(entries) +} + +#[cfg(test)] +mod tests { + use super::{Change, Entry, Origin, entries_after}; + use crate::project::{ProjectSpec, create_project_file, save_dataset, write_notes}; + + fn project(name: &str) -> std::path::PathBuf { + let dir = std::env::temp_dir().join("qrate-history-test"); + let _ = std::fs::create_dir_all(&dir); + let path = dir.join(name); + let _ = std::fs::remove_file(&path); + let headers = vec!["Title".to_string()]; + let rows = vec![vec!["one".to_string()]]; + create_project_file( + &path, + &ProjectSpec { + name: "H", + source: "CSV", + headers: &headers, + rows: &rows, + ..Default::default() + }, + ) + .unwrap(); + path + } + + fn edit(before: &str, after: &str) -> Change { + Change::Cell { + row: 1, + column: "Title".into(), + before: before.into(), + after: after.into(), + } + } + + /// Entries written with the data and with the notes read back in order, whole, and numbered, + /// and a file that has never logged anything reads as an empty history. + #[test] + fn entries_round_trip_through_both_writers() { + let path = project("round-trip.qrate"); + assert_eq!(entries_after(&path, 0).unwrap(), Vec::new()); + + let typed = Entry::new(Origin::Typed, vec![edit("one", "One")], Some("rk".into())); + let fixed = Entry::new( + Origin::Fix("Use “One.”".into()), + vec![edit("One", "One.")], + None, + ); + let headers = vec!["Title".to_string()]; + save_dataset( + &path, + &headers, + &[1], + &[vec!["One.".into()]], + &[typed.clone(), fixed.clone()], + ) + .unwrap(); + let note = Entry::new( + Origin::Clear, + vec![Change::Note { + row: Some(1), + column: None, + before: Some("faded".into()), + after: None, + }], + None, + ); + write_notes(&path, "note", &[], std::slice::from_ref(¬e)).unwrap(); + + let all = entries_after(&path, 0).unwrap(); + let ids: Vec<_> = all.iter().map(|e| e.id).collect(); + assert_eq!(ids, vec![1, 2, 3]); + let strip = |e: &Entry| Entry { id: 0, ..e.clone() }; + assert_eq!( + all.iter().map(strip).collect::>(), + vec![typed, fixed, note] + ); + assert_eq!(entries_after(&path, 2).unwrap().len(), 1); + } + + #[test] + fn an_inverse_undoes_and_its_own_inverse_is_the_original() { + let moved = Change::ColumnMoved { + column: "Title".into(), + from: 0, + to: 3, + }; + assert_eq!(edit("a", "b").inverse(), edit("b", "a")); + assert_eq!(moved.inverse().inverse(), moved); + } +} diff --git a/crates/settings/src/lib.rs b/crates/settings/src/lib.rs index eebcd099..fc51ebec 100644 --- a/crates/settings/src/lib.rs +++ b/crates/settings/src/lib.rs @@ -4,6 +4,7 @@ pub mod columns; pub mod dirty; pub mod filenames; +pub mod history; pub mod os_open; pub mod path_picker; pub mod plugins; diff --git a/crates/settings/src/project.rs b/crates/settings/src/project.rs index abbbd1af..b35e7da7 100644 --- a/crates/settings/src/project.rs +++ b/crates/settings/src/project.rs @@ -221,7 +221,7 @@ fn open_rw(path: &Path) -> Result { } /// Read-only open; no journal-mode change (that needs a write handle). -fn open_ro(path: &Path) -> Result { +pub(crate) fn open_ro(path: &Path) -> Result { let conn = Connection::open_with_flags(path, rusqlite::OpenFlags::SQLITE_OPEN_READ_ONLY) .with_context(|| format!("Open project at {path:?}"))?; conn.busy_timeout(Duration::from_secs(5)) @@ -639,7 +639,12 @@ pub fn read_notes(path: &Path) -> Result> { /// replace-by-source rule the in-memory store uses, so a re-import can't leave orphans behind. /// Only authored sources belong here (imported notes, user marks); computed diagnostics are /// recomputed on open and would go stale in storage. -pub fn write_notes(path: &Path, source: &str, notes: &[StoredNote]) -> Result<()> { +pub fn write_notes( + path: &Path, + source: &str, + notes: &[StoredNote], + history: &[crate::history::Entry], +) -> Result<()> { let mut conn = open_rw(path)?; let tx = conn.transaction()?; tx.execute_batch(NOTES_DDL).context("Create __notes")?; @@ -665,6 +670,7 @@ pub fn write_notes(path: &Path, source: &str, notes: &[StoredNote]) -> Result<() .context("Insert note")?; } } + crate::history::append(&tx, history)?; tx.pragma_update(None, "user_version", QRATE_SCHEMA_VERSION) .context("Set user_version")?; tx.commit().context("Commit notes")?; @@ -843,6 +849,7 @@ pub fn save_dataset( headers: &[String], row_ids: &[RowId], rows: &[Vec], + history: &[crate::history::Entry], ) -> Result<()> { if headers.is_empty() { return Ok(()); @@ -851,6 +858,7 @@ pub fn save_dataset( conn.execute_batch("BEGIN; DROP TABLE IF EXISTS dataset_main;") .context("Begin dataset rewrite")?; create_and_fill_dataset(&conn, headers, Some(row_ids), rows)?; + crate::history::append(&conn, history)?; conn.execute_batch("COMMIT") .context("Commit dataset rewrite")?; Ok(()) @@ -965,7 +973,7 @@ mod tests { let mut fresh = note(Some(2), Some("Title"), "identified by her daughter"); fresh.created_at = Some("2026-08-14".into()); fresh.author = Some("rk".into()); - write_notes(&path, "import", &[fresh]).unwrap(); + write_notes(&path, "import", &[fresh], &[]).unwrap(); let after = read_notes(&path).unwrap(); assert_eq!(after.len(), 1); @@ -985,12 +993,19 @@ mod tests { note(Some(0), Some("Title"), "cell note"), note(Some(3), None, "whole row"), ], + &[], ) .unwrap(); assert_eq!(read_notes(&path).unwrap().len(), 2); // A different source is additive — it must not disturb `import`'s entries. - write_notes(&path, "spell", &[note(None, Some("Title"), "whole column")]).unwrap(); + write_notes( + &path, + "spell", + &[note(None, Some("Title"), "whole column")], + &[], + ) + .unwrap(); assert_eq!(read_notes(&path).unwrap().len(), 3); // Optional coordinates survive the NULL round-trip in both directions. @@ -1004,7 +1019,7 @@ mod tests { ); // Republishing an empty set clears only that source — the invalidation rule. - write_notes(&path, "import", &[]).unwrap(); + write_notes(&path, "import", &[], &[]).unwrap(); let left = read_notes(&path).unwrap(); assert_eq!(left.len(), 1); assert_eq!(left[0].message, "whole column"); @@ -1024,7 +1039,7 @@ mod tests { // A v1 `.qrate` is missing the table entirely; that is empty, not an error. assert_eq!(read_notes(&path).unwrap(), Vec::new()); // ...and the first write creates it, which is the whole 1 -> 2 migration. - write_notes(&path, "import", &[note(Some(1), Some("A"), "hi")]).unwrap(); + write_notes(&path, "import", &[note(Some(1), Some("A"), "hi")], &[]).unwrap(); assert_eq!(read_notes(&path).unwrap().len(), 1); } @@ -1177,7 +1192,7 @@ mod tests { vec!["1".to_string(), "Edited".to_string()], vec!["2".to_string(), "Second".to_string()], ]; - save_dataset(&path, &headers, &[1, 2], &edited).unwrap(); + save_dataset(&path, &headers, &[1, 2], &edited, &[]).unwrap(); let data = load_project_file(&path).unwrap(); assert_eq!(data.headers, headers); @@ -1229,12 +1244,13 @@ mod tests { author: None, }, ], + &[], ) .unwrap(); let mut inserted = vec![vec!["new item".into()]]; inserted.extend(rows); - save_dataset(&path, &headers, &[7, 1, 2, 3, 4, 5, 6], &inserted).unwrap(); + save_dataset(&path, &headers, &[7, 1, 2, 3, 4, 5, 6], &inserted, &[]).unwrap(); let reopened = load_project_file(&path).unwrap(); assert_eq!(reopened.row_ids, vec![7, 1, 2, 3, 4, 5, 6]); diff --git a/crates/table/src/delegate.rs b/crates/table/src/delegate.rs index 6e7efc4d..f0e123e9 100644 --- a/crates/table/src/delegate.rs +++ b/crates/table/src/delegate.rs @@ -13,6 +13,7 @@ use gpui_component::{ use serde::{Deserialize, Serialize}; use diagnostics::{DATASET_MAIN, Location}; +use settings::history::{Change, Entry, EntryId, Origin}; use crate::history::{Cells, Col, History, Row, Step}; use crate::{cell, editing::EditState, filter, row_index}; @@ -91,6 +92,9 @@ pub struct QrateTableDelegate { /// Undo/redo stack for cell edits and shape changes alike, so a delete and the typing before /// it come back in the order they went in. history: History, + /// Log entries for changes not yet saved. They reach the `.qrate` file with the data they + /// describe, and are dropped with it when a project is closed without saving. + unsaved: Vec, /// How many leading data columns are frozen. A count in *display* order, not a set of keys: /// the library's fixed region is always the leading columns, and moving a column in or out of /// it is how a sheet re-freezes. Zero means only the pinned `#` column stays put. @@ -118,6 +122,7 @@ impl QrateTableDelegate { subdelimiter: SharedString::default(), values_generation: 0, history: History::default(), + unsaved: Vec::new(), frozen: 0, } } @@ -359,15 +364,7 @@ impl QrateTableDelegate { row_ids: &[settings::project::RowId], rows: &[Vec], ) { - self.columns = headers - .iter() - .map(|h| { - Column::new(h.clone(), h.clone()) - .width(px(120.)) - .resizable(true) - .movable(true) - }) - .collect(); + self.columns = headers.iter().map(|h| new_column(h.clone())).collect(); self.rows = rows .iter() .map(|r| r.iter().map(|c| SharedString::from(c.clone())).collect()) @@ -386,6 +383,7 @@ impl QrateTableDelegate { self.editing = EditState::Idle; // Recorded edits index into the outgoing dataset. self.history = History::default(); + self.unsaved.clear(); self.filters = vec![HashSet::new(); self.columns.len()]; self.filters_enabled = vec![false; self.columns.len()]; self.visible_rows = (0..self.rows.len()).collect(); @@ -519,7 +517,7 @@ impl QrateTableDelegate { /// through here — inline commit, diagnostic fix, paste, bulk fill — so none of them carries /// undo logic of its own. Cells whose text is unchanged are dropped, so a commit that typed /// nothing doesn't consume an undo. - pub(crate) fn apply_edit(&mut self, cells: Cells) -> bool { + pub(crate) fn apply_edit(&mut self, cells: Cells, origin: Origin) -> bool { let mut edit = Vec::with_capacity(cells.len()); for (row, col, after) in cells { let Some(before) = self.cell(row, col).cloned() else { @@ -532,30 +530,246 @@ impl QrateTableDelegate { edit.push((row, col, before, after)); } let changed = !edit.is_empty(); - self.history.push(Step::Cells(edit)); + self.record(Step::Cells(edit), origin); changed } - /// Reverse the last recorded step. The answer says whether it moved rows; `None` means there - /// was nothing to undo. - pub(crate) fn undo(&mut self) -> Option { + /// Push a step that has just been applied onto the undo stack, and log it. + fn record(&mut self, step: Step, origin: Origin) { + let changes = self.changes(&step); + self.log(origin, changes); + self.history.push(step); + } + + fn log(&mut self, origin: Origin, changes: Vec) { + if !changes.is_empty() { + self.unsaved.push(Entry::new(origin, changes, None)); + } + } + + /// What a step did, by row id and column name. Read against the grid as it stands just after + /// the step, which is the only state its indices are guaranteed to mean anything in. + fn changes(&self, step: &Step) -> Vec { + let row_id = |row: usize| self.row_ids.get(row).copied().unwrap_or_default(); + let named = |cells: &[SharedString]| { + self.columns + .iter() + .zip(cells) + .map(|(c, text)| (c.name.to_string(), text.to_string())) + .collect() + }; + let by_row = |col: &Col| { + self.row_ids + .iter() + .zip(&col.cells) + .map(|(id, text)| (*id, text.to_string())) + .collect() + }; + match step { + Step::Cells(cells) => cells + .iter() + .map(|(row, col, before, after)| Change::Cell { + row: row_id(*row), + column: self.column_name(*col).to_string(), + before: before.to_string(), + after: after.to_string(), + }) + .collect(), + Step::RowsAdded { at, rows } => rows + .iter() + .enumerate() + .map(|(offset, row)| Change::RowAdded { + row: row.id, + position: at + offset, + cells: named(&row.cells), + }) + .collect(), + Step::RowsRemoved(rows) => rows + .iter() + .map(|(at, row)| Change::RowRemoved { + row: row.id, + position: *at, + cells: named(&row.cells), + }) + .collect(), + Step::ColumnAdded { at, col } => vec![Change::ColumnAdded { + column: col.column.name.to_string(), + position: *at, + cells: by_row(col), + }], + Step::ColumnRemoved { at, col } => vec![Change::ColumnRemoved { + column: col.column.name.to_string(), + position: *at, + cells: by_row(col), + }], + Step::Renamed { before, after, .. } => vec![Change::ColumnRenamed { + before: before.to_string(), + after: after.to_string(), + }], + Step::ColumnMoved { from, to } => vec![Change::ColumnMoved { + column: self.column_name(*to).to_string(), + from: *from, + to: *to, + }], + // A batch's steps are read one by one, each in its own state, as they are replayed. + Step::Batch(_) => Vec::new(), + } + } + + /// Reverse the last recorded step, and log that as its own entry. `None` when there was + /// nothing to undo; otherwise what the undo changed. + pub(crate) fn undo(&mut self) -> Option> { let step = self.history.undo()?; - let rows_changed = matches!(step, Step::RowsAdded { .. } | Step::RowsRemoved(_)); - self.replay(&step, false); - Some(rows_changed) + Some(self.replay_logged(&step, false, Origin::Undo)) } /// [`undo`](Self::undo)'s mirror. - pub(crate) fn redo(&mut self) -> Option { + pub(crate) fn redo(&mut self) -> Option> { let step = self.history.redo()?; - let rows_changed = matches!(step, Step::RowsAdded { .. } | Step::RowsRemoved(_)); - self.replay(&step, true); - Some(rows_changed) + Some(self.replay_logged(&step, true, Origin::Redo)) + } + + fn replay_logged(&mut self, step: &Step, forward: bool, origin: Origin) -> Vec { + let mut changes = Vec::new(); + self.replay(step, forward, &mut changes); + self.log(origin, changes.clone()); + changes + } + + /// Put the grid back the way `changes` say, by identity rather than position: the log's + /// positions are only where to put a row or column back. Everything applied lands as one undo + /// step and one log entry. A change that no longer fits — its row already gone, its column + /// renamed out from under it — is skipped. Notes are not the grid's to apply. + pub(crate) fn restore(&mut self, changes: &[Change], to: EntryId) -> Vec { + let mut steps = Vec::new(); + let mut applied = Vec::new(); + for change in changes { + match self.apply_change(change) { + Some(step) => { + applied.extend(self.changes(&step)); + steps.push(step); + } + None => log::warn!("restore skipped a change that no longer fits: {change:?}"), + } + } + self.log(Origin::Restore(to), applied.clone()); + self.history.push(Step::Batch(steps)); + self.editing = EditState::Idle; + applied + } + + fn apply_change(&mut self, change: &Change) -> Option { + let find = |ids: &[settings::project::RowId], row| ids.iter().position(|id| *id == row); + match change { + Change::Cell { + row, column, after, .. + } => { + let (r, c) = (find(&self.row_ids, *row)?, self.data_col(column)?); + let before = self.cell(r, c)?.clone(); + let after = SharedString::from(after.clone()); + self.set_cell(r, c, after.clone()); + Some(Step::Cells(vec![(r, c, before, after)])) + } + Change::RowAdded { + row, + position, + cells, + } => { + if find(&self.row_ids, *row).is_some() { + return None; + } + let text = |name: &SharedString| { + cells + .iter() + .find(|(column, _)| column == name.as_ref()) + .map(|(_, text)| SharedString::from(text.clone())) + .unwrap_or_default() + }; + let rows = vec![Row { + id: *row, + cells: self.columns.iter().map(|c| text(&c.name)).collect(), + image: None, + }]; + let at = (*position).min(self.rows.len()); + self.splice_rows(at, &rows); + Some(Step::RowsAdded { at, rows }) + } + Change::RowRemoved { row, .. } => { + let at = find(&self.row_ids, *row)?; + Some(Step::RowsRemoved(self.cut_rows(&[at]))) + } + Change::ColumnAdded { + column, + position, + cells, + } => { + if self.data_col(column).is_some() { + return None; + } + let col = Col { + column: new_column(column.clone()), + cells: self + .row_ids + .iter() + .map(|id| { + cells + .iter() + .find(|(row, _)| row == id) + .map(|(_, text)| SharedString::from(text.clone())) + .unwrap_or_default() + }) + .collect(), + excluded: HashSet::new(), + filter_enabled: false, + }; + let at = (*position).min(self.columns.len()); + self.splice_column(at, &col); + Some(Step::ColumnAdded { at, col }) + } + Change::ColumnRemoved { column, .. } => { + let at = self.data_col(column)?; + let col = self.cut_column(at)?; + Some(Step::ColumnRemoved { at, col }) + } + Change::ColumnRenamed { before, after } => { + let col = self.data_col(before)?; + if self.data_col(after).is_some() { + return None; + } + self.set_column_name(col, after.clone().into()); + Some(Step::Renamed { + col, + before: before.clone().into(), + after: after.clone().into(), + }) + } + Change::ColumnMoved { column, to, .. } => { + let from = self.data_col(column)?; + let to = (*to).min(self.columns.len() - 1); + self.shift_column(from, to); + Some(Step::ColumnMoved { from, to }) + } + Change::Note { .. } => None, + } + } + + /// The log entries made since the last save, oldest first. + pub fn unsaved_history(&self) -> &[Entry] { + &self.unsaved + } + + /// Forget the first `count` unsaved entries — they have just been written. + pub(crate) fn history_saved(&mut self, count: usize) { + self.unsaved.drain(..count.min(self.unsaved.len())); } /// Apply one side of a recorded step without re-recording it — going through `apply_edit` here - /// would make undo its own undoable action and the stack would never drain. - fn replay(&mut self, step: &Step, forward: bool) { + /// would make undo its own undoable action and the stack would never drain. What it changed is + /// added to `changes`, each step read in the state it was written against. + fn replay(&mut self, step: &Step, forward: bool, changes: &mut Vec) { + if !forward { + changes.extend(self.changes(step).iter().rev().map(Change::inverse)); + } match step { Step::Cells(cells) => { for (row, col, before, after) in cells { @@ -596,6 +810,20 @@ impl QrateTableDelegate { Step::Renamed { col, before, after } => { self.set_column_name(*col, if forward { after } else { before }.clone()); } + Step::ColumnMoved { from, to } => match forward { + true => self.shift_column(*from, *to), + false => self.shift_column(*to, *from), + }, + Step::Batch(steps) => match forward { + true => steps.iter().for_each(|s| self.replay(s, true, changes)), + false => steps + .iter() + .rev() + .for_each(|s| self.replay(s, false, changes)), + }, + } + if forward { + changes.extend(self.changes(step)); } // An open edit would commit over what was just restored. self.editing = EditState::Idle; @@ -733,13 +961,13 @@ impl QrateTableDelegate { let id = self.fresh_row_id(); let rows = vec![Row { id, cells, image }]; self.splice_rows(at, &rows); - self.history.push(Step::RowsAdded { at, rows }); + self.record(Step::RowsAdded { at, rows }, Origin::Structure); } /// Delete the rows at `ats` as one undo step, carrying their cells and photos on it. pub(crate) fn remove_rows(&mut self, ats: &[usize]) { let removed = self.cut_rows(ats); - self.history.push(Step::RowsRemoved(removed)); + self.record(Step::RowsRemoved(removed), Origin::Structure); } pub(crate) fn row_id(&self, source: usize) -> Option { @@ -763,16 +991,13 @@ impl QrateTableDelegate { pub(crate) fn insert_column(&mut self, at: usize) -> SharedString { let name = self.fresh_column_name(); let col = Col { - column: Column::new(name.clone(), name.clone()) - .width(px(120.)) - .resizable(true) - .movable(true), + column: new_column(name.clone()), cells: vec![SharedString::default(); self.rows.len()], excluded: HashSet::new(), filter_enabled: false, }; self.splice_column(at, &col); - self.history.push(Step::ColumnAdded { at, col }); + self.record(Step::ColumnAdded { at, col }, Origin::Structure); name } @@ -781,7 +1006,7 @@ impl QrateTableDelegate { let Some(col) = self.cut_column(at) else { return; }; - self.history.push(Step::ColumnRemoved { at, col }); + self.record(Step::ColumnRemoved { at, col }, Origin::Structure); } /// Rename a column, which re-keys it — the caller moves its stored settings to match. `false` @@ -794,14 +1019,39 @@ impl QrateTableDelegate { return false; } self.set_column_name(col, name.clone()); - self.history.push(Step::Renamed { - col, - before, - after: name, - }); + self.record( + Step::Renamed { + col, + before, + after: name, + }, + Origin::Structure, + ); true } + /// Move the column at `from` to `to`, its cells and filter with it; row order is untouched. + fn shift_column(&mut self, from: usize, to: usize) { + if from >= self.columns.len() || to >= self.columns.len() { + return; + } + let col = self.columns.remove(from); + self.columns.insert(to, col); + move_col(&mut self.rows, from, to); + self.values_generation += 1; + if from < self.filters.len() && to < self.filters.len() { + let f = self.filters.remove(from); + self.filters.insert(to, f); + } + if from < self.filters_enabled.len() && to < self.filters_enabled.len() { + let e = self.filters_enabled.remove(from); + self.filters_enabled.insert(to, e); + } + // An in-flight edit and the shift-click range both index into the old column order. + self.editing = EditState::Idle; + self.range = None; + } + /// The name *and* the key, which are the same string — see [`Self::column_key`]. fn set_column_name(&mut self, col: usize, name: SharedString) { if let Some(column) = self.columns.get_mut(col) { @@ -1024,6 +1274,15 @@ fn move_col(rows: &mut [Vec], from: usize, to: usize) { } } +/// A fresh grid column under `name`, which is both its label and its key. +fn new_column(name: impl Into) -> Column { + let name = name.into(); + Column::new(name.clone(), name) + .width(px(120.)) + .resizable(true) + .movable(true) +} + /// A cell's filterable values: the whole cell, or its sub-delimited parts when a delimiter is set. /// Parts are trimmed, because `Film; Video` is written with a space that nobody means as part of /// the value. @@ -1240,23 +1499,8 @@ impl TableDelegate for QrateTableDelegate { if from >= self.columns.len() || to >= self.columns.len() { return; } - let col = self.columns.remove(from); - self.columns.insert(to, col); - move_col(&mut self.rows, from, to); - self.values_generation += 1; - // The column's filter (and whether it has one) moves with it; row order is untouched. - if from < self.filters.len() && to < self.filters.len() { - let f = self.filters.remove(from); - self.filters.insert(to, f); - } - if from < self.filters_enabled.len() && to < self.filters_enabled.len() { - let e = self.filters_enabled.remove(from); - self.filters_enabled.insert(to, e); - } - // An in-flight edit indexes into the old column order — drop it, and the recorded ones too. - self.editing = EditState::Idle; - self.range = None; - self.history = History::default(); + self.shift_column(from, to); + self.record(Step::ColumnMoved { from, to }, Origin::Structure); } } @@ -1495,9 +1739,11 @@ mod tests { #[cfg(test)] mod app_tests { // Never `use super::*` here — see the note on `note.rs`'s test module. + use crate::history::{Step, moves_rows}; use crate::{TablePanel, TableStateHandle}; use gpui::{Entity, TestAppContext}; use gpui_component::table::{ColumnFixed, TableDelegate as _, TableState}; + use settings::history::{Change, Origin}; fn project() -> settings::project::CurrentProject { settings::project::CurrentProject { @@ -1729,7 +1975,7 @@ mod app_tests { assert_eq!(delegate.row_image(1), None, "the new row has no photo"); assert_eq!(delegate.row_image(2), Some("1.jpg".as_ref())); - assert_eq!(delegate.undo(), Some(true)); + assert!(delegate.undo().is_some_and(|c| moves_rows(&c))); assert_eq!(delegate.row_ids(), &[1, 2, 3, 4]); assert_eq!(delegate.cell(1, 1).map(|c| c.as_ref()), Some("two")); assert_eq!(delegate.row_image(1), Some("1.jpg".as_ref())); @@ -1754,7 +2000,7 @@ mod app_tests { }; assert_eq!(names(delegate), vec!["two", "four"]); - assert_eq!(delegate.undo(), Some(true)); + assert!(delegate.undo().is_some_and(|c| moves_rows(&c))); assert_eq!(delegate.row_ids(), &[1, 2, 3, 4]); assert_eq!(names(delegate), vec!["one", "two", "three", "four"]); assert_eq!(delegate.undo(), None, "one step, not two"); @@ -1782,13 +2028,13 @@ mod app_tests { None, ); assert_eq!(edits.len(), 2); - delegate.apply_edit(edits); + delegate.apply_edit(edits, Origin::Typed); assert_eq!( media(delegate), vec!["Film", "Videotape", "Videotape", "Film"] ); - assert_eq!(delegate.undo(), Some(false)); + assert!(delegate.undo().is_some_and(|c| !moves_rows(&c))); assert_eq!(media(delegate), vec!["Film", "Video", "Video", "Film"]); assert_eq!(delegate.undo(), None, "one step, not two"); }); @@ -1803,13 +2049,13 @@ mod app_tests { cx.update(|cx| { state.update(cx, |state, _| { let delegate = state.delegate_mut(); - delegate.apply_edit(vec![(3, 1, "edited".into())]); + delegate.apply_edit(vec![(3, 1, "edited".into())], Origin::Typed); delegate.remove_rows(&[0]); // "four" is row 2 now, and its edit was recorded against row 3. assert_eq!(delegate.cell(2, 1).map(|c| c.as_ref()), Some("edited")); - assert_eq!(delegate.undo(), Some(true)); - assert_eq!(delegate.undo(), Some(false)); + assert!(delegate.undo().is_some_and(|c| moves_rows(&c))); + assert!(delegate.undo().is_some_and(|c| !moves_rows(&c))); assert_eq!(delegate.cell(3, 1).map(|c| c.as_ref()), Some("four")); }); }); @@ -1830,7 +2076,7 @@ mod app_tests { assert_eq!(delegate.column_name(0), "Title"); assert_eq!(delegate.cell(0, 0).map(|c| c.as_ref()), Some("one")); - assert_eq!(delegate.undo(), Some(false)); + assert!(delegate.undo().is_some_and(|c| !moves_rows(&c))); assert_eq!(delegate.column_name(0), "Medium"); assert_eq!(delegate.cell(3, 0).map(|c| c.as_ref()), Some("Film")); assert!(delegate.column_filter_enabled(0)); @@ -1855,7 +2101,7 @@ mod app_tests { assert!(delegate.rename_column(1, "Date".into())); assert_eq!(delegate.column_key(1), "Date"); - assert_eq!(delegate.undo(), Some(false)); + assert!(delegate.undo().is_some_and(|c| !moves_rows(&c))); assert_eq!(delegate.column_name(1), "Column 3"); }); }); @@ -1900,4 +2146,71 @@ mod app_tests { }); }); } + + /// The log is complete enough to walk back: undoing every change made after an entry, newest + /// first and by identity, lands on exactly the grid that entry left — through deletes, renames + /// and moves that shuffle every position the early changes were written against. The restore + /// is one more entry, and one undo takes it back. + #[gpui::test] + fn restoring_to_an_entry_reproduces_the_grid_it_left(cx: &mut TestAppContext) { + let state = table(cx); + cx.update(|cx| { + state.update(cx, |state, _| { + let delegate = state.delegate_mut(); + delegate.apply_edit(vec![(1, 1, "TWO".into())], Origin::Typed); + let checkpoint = delegate.dataset_snapshot(); + let kept = delegate.unsaved_history().len(); + + delegate.remove_rows(&[0]); + delegate.insert_column(1); + delegate.rename_column(0, "Format".into()); + delegate.shift_column(0, 2); + delegate.record(Step::ColumnMoved { from: 0, to: 2 }, Origin::Structure); + delegate.apply_edit(vec![(2, 2, "4".into())], Origin::Paste); + delegate.insert_rows(0, Some(1)); + let head = delegate.dataset_snapshot(); + + let undo: Vec = delegate.unsaved_history()[kept..] + .iter() + .rev() + .flat_map(|e| e.changes.iter().rev().map(Change::inverse)) + .collect(); + let applied = delegate.restore(&undo, 7); + assert_eq!(applied.len(), undo.len(), "every change still fits"); + assert_eq!(delegate.dataset_snapshot(), checkpoint); + assert_eq!( + delegate.unsaved_history().last().map(|e| e.origin.clone()), + Some(Origin::Restore(7)) + ); + + assert!(delegate.undo().is_some()); + assert_eq!(delegate.dataset_snapshot(), head); + }); + }); + } + + /// Undo and redo are entries in their own right, and say what they put back. + #[gpui::test] + fn undo_is_logged_as_the_inverse_of_what_it_reverses(cx: &mut TestAppContext) { + let state = table(cx); + cx.update(|cx| { + state.update(cx, |state, _| { + let delegate = state.delegate_mut(); + delegate.apply_edit(vec![(3, 1, "FOUR".into())], Origin::Typed); + delegate.undo(); + let log = delegate.unsaved_history(); + assert_eq!(log.len(), 2); + assert_eq!(log[1].origin, Origin::Undo); + assert_eq!( + log[1].changes, + vec![Change::Cell { + row: 4, + column: "Title".into(), + before: "FOUR".into(), + after: "four".into(), + }] + ); + }); + }); + } } diff --git a/crates/table/src/editing.rs b/crates/table/src/editing.rs index 4169e373..135e31f3 100644 --- a/crates/table/src/editing.rs +++ b/crates/table/src/editing.rs @@ -6,6 +6,7 @@ use std::path::Path; use gpui::{Context, PathPromptOptions, SharedString, Window}; use gpui_component::table::TableState; +use settings::history::Origin; use crate::delegate::QrateTableDelegate; @@ -74,7 +75,7 @@ pub(crate) fn start( cx, ); } - crate::write_cell(row, col, name, cx); + crate::write_cell(row, col, name, Origin::Typed, cx); }); } }) @@ -151,7 +152,7 @@ pub(crate) fn commit( } } EditState::Editing { row, col } => { - if !delegate.apply_edit(vec![(row, col, value)]) { + if !delegate.apply_edit(vec![(row, col, value)], Origin::Typed) { return Committed::Unchanged; } settings::dirty::mark(settings::dirty::PROJECT_DATA, cx); diff --git a/crates/table/src/history.rs b/crates/table/src/history.rs index 5920aeef..8d8d0d4b 100644 --- a/crates/table/src/history.rs +++ b/crates/table/src/history.rs @@ -13,6 +13,7 @@ use std::path::PathBuf; use gpui::SharedString; use gpui_component::table::Column; +use settings::history::Change; /// A batch of cell writes as `(row, col, text)` — what `apply_edit` takes and what undo hands back. pub(crate) type Cells = Vec<(usize, usize, SharedString)>; @@ -71,6 +72,12 @@ pub(crate) enum Step { before: SharedString, after: SharedString, }, + ColumnMoved { + from: usize, + to: usize, + }, + /// Several steps taken as one, in the order they were made — a restore. + Batch(Vec), } impl Step { @@ -81,11 +88,20 @@ impl Step { Step::RowsAdded { rows, .. } => rows.is_empty(), Step::RowsRemoved(rows) => rows.is_empty(), Step::Renamed { before, after, .. } => before == after, + Step::ColumnMoved { from, to } => from == to, + Step::Batch(steps) => steps.iter().all(Step::is_empty), _ => false, } } } +/// Whether any of `changes` added or removed a row — what notes have to be re-aligned after. +pub(crate) fn moves_rows(changes: &[Change]) -> bool { + changes + .iter() + .any(|c| matches!(c, Change::RowAdded { .. } | Change::RowRemoved { .. })) +} + #[derive(Default)] pub(crate) struct History { done: Vec, diff --git a/crates/table/src/lib.rs b/crates/table/src/lib.rs index da46727f..e3f02217 100644 --- a/crates/table/src/lib.rs +++ b/crates/table/src/lib.rs @@ -42,6 +42,7 @@ use gpui::{ use gpui_component::table::TableState; use plugin_api::CommandContext; use settings::columns::ColumnType; +use settings::history::{Change, EntryId, Origin}; /// Global handle to the live table state, so cross-crate status-bar items (the fake-data button /// and the selected-cell widget in the `app` crate) can reach the table. @@ -121,8 +122,8 @@ fn autosave(cx: &mut App) { /// Commit `text` into a cell, mark the project dirty, and re-run validation — everything a /// committed edit does except going through the inline editor. What a fix menu applies through. -pub fn write_cell(row: usize, col: usize, text: SharedString, cx: &mut App) { - write_cells(vec![(row, col, text)], cx); +pub fn write_cell(row: usize, col: usize, text: SharedString, origin: Origin, cx: &mut App) { + write_cells(vec![(row, col, text)], origin, cx); } /// The selection as tab-separated text on the clipboard — what Ctrl+C copies, reachable from @@ -169,7 +170,7 @@ pub fn clear_selection(cx: &mut App) { /// [`write_cell`]'s bulk form: one undo step for the whole batch, one validation pass at the end, /// one save. For menu items, which act on an explicitly clicked target; the keyboard and clipboard /// paths go through `TablePanel`'s own method of the same name, which debounces both instead. -pub fn write_cells(cells: Vec<(usize, usize, SharedString)>, cx: &mut App) { +pub fn write_cells(cells: Vec<(usize, usize, SharedString)>, origin: Origin, cx: &mut App) { if cells.is_empty() { return; } @@ -181,7 +182,7 @@ pub fn write_cells(cells: Vec<(usize, usize, SharedString)>, cx: &mut App) { }; state.update(cx, |state, cx| { let files = names_a_file(state.delegate(), &cells, cx); - state.delegate_mut().apply_edit(cells); + state.delegate_mut().apply_edit(cells, origin); if files { photos::refresh(state, cx); } @@ -216,28 +217,156 @@ pub fn history_step(redo: bool, cx: &mut App) { else { return; }; - let row_ids = state.update(cx, |state, cx| { - let rows_changed = match redo { - true => state.delegate_mut().redo(), - false => state.delegate_mut().undo(), - }; - if rows_changed.is_some() { - cx.emit(delegate::TableChanged); - cx.notify(); - } - rows_changed.map(|changed| changed.then(|| state.delegate().row_ids().to_vec())) + let changes = state.update(cx, |state, _| match redo { + true => state.delegate_mut().redo(), + false => state.delegate_mut().undo(), }); - let Some(row_ids) = row_ids else { + let Some(changes) = changes else { return; }; - if let Some(row_ids) = row_ids { - diagnostics::Diagnostics::align_note_rows(diagnostics::DATASET_MAIN, &row_ids, cx); + let origin = match redo { + true => Origin::Redo, + false => Origin::Undo, + }; + settle(&state, &changes, origin, cx); +} + +/// Everything that follows the grid changing under the user rather than one cell at a time — an +/// undo, a redo, a restore: notes follow their rows, stored column settings follow a rename, and +/// the layout, validation and save catch up. +fn settle( + state: &Entity>, + changes: &[Change], + origin: Origin, + cx: &mut App, +) { + state.update(cx, |state, cx| { + // A row put back by a restore carries no photo, and an undone filename names another. + photos::refresh(state, cx); + // The library caches a `Column` per index, so a changed column set needs this. + state.refresh(cx); + cx.emit(delegate::TableChanged); + cx.notify(); + }); + if history::moves_rows(changes) { + let row_ids = state.read(cx).delegate().row_ids().to_vec(); + diagnostics::Diagnostics::align_note_rows(diagnostics::DATASET_MAIN, &row_ids, origin, cx); + } + for change in changes { + if let Change::ColumnRenamed { before, after } = change { + follow_rename(before, &after.clone().into(), cx); + } } + TablePanel::persist_columns(state, cx); settings::dirty::mark(settings::dirty::PROJECT_DATA, cx); revalidate_now(cx); autosave(cx); } +/// Re-key what is stored under a column's name — its notes, its settings, its `__columns` row — +/// after the grid's column has been renamed. +fn follow_rename(before: &str, after: &SharedString, cx: &mut App) { + diagnostics::Diagnostics::column_renamed(diagnostics::DATASET_MAIN, before, after, cx); + settings::columns::rename(before, after, cx); + if let Some(file) = cx + .try_global::() + .map(|p| p.file.clone()) + && let Err(err) = settings::project::rename_column(&file, before, after) + { + log::error!("failed to rename column {before} to {after}: {err}"); + } +} + +/// Put the project back the way it was just after entry `to`, by applying the inverse of every +/// change made since, newest first. Nothing is rewound: the restore is itself logged, and undone +/// with one Ctrl+Z. +pub fn restore_to(to: EntryId, cx: &mut App) { + let Some(file) = cx + .try_global::() + .map(|p| p.file.clone()) + else { + return; + }; + let Some(state) = cx + .try_global::() + .and_then(|h| h.0.upgrade()) + else { + return; + }; + let saved = match settings::history::entries_after(&file, to) { + Ok(saved) => saved, + Err(err) => { + log::error!("couldn't read the project history to restore it: {err}"); + return; + } + }; + let unsaved = state.read(cx).delegate().unsaved_history().to_vec(); + let (notes, grid): (Vec, Vec) = saved + .iter() + .chain(&unsaved) + .rev() + .flat_map(|entry| entry.changes.iter().rev().map(Change::inverse)) + .partition(|change| matches!(change, Change::Note { .. })); + + let applied = state.update(cx, |state, _| state.delegate_mut().restore(&grid, to)); + settle(&state, &applied, Origin::Restore(to), cx); + + for note in notes { + let Change::Note { + row, column, after, .. + } = note + else { + continue; + }; + let position = row.and_then(|id| { + let delegate = state.read(cx).delegate(); + delegate.row_ids().iter().position(|r| *r == id) + }); + if row.is_some() && position.is_none() { + continue; + } + let location = diagnostics::Location { + dataset: diagnostics::DATASET_MAIN.into(), + row: position, + row_id: row, + column: column.map(Into::into), + }; + diagnostics::Diagnostics::set_note( + location, + after.unwrap_or_default().into(), + Origin::Restore(to), + cx, + ); + } +} + +/// Put one cell back to the value an entry recorded, wherever that row and column now sit. +pub fn restore_value( + row: settings::project::RowId, + column: &str, + text: SharedString, + from: EntryId, + cx: &mut App, +) { + let Some(state) = cx + .try_global::() + .and_then(|h| h.0.upgrade()) + else { + return; + }; + let target = { + let delegate = state.read(cx).delegate(); + delegate + .row_ids() + .iter() + .position(|id| *id == row) + .zip(delegate.data_col(column)) + }; + if let Some((row, col)) = target { + write_cell(row, col, text, Origin::Restore(from), cx); + } +} + /// A change to the grid's shape rather than its contents. One enum rather than five entry points /// because every one of them has the same tail: re-index the notes, re-save the column layout, /// revalidate, mark dirty, save. @@ -327,18 +456,15 @@ pub fn structural(op: Structural, cx: &mut App) { Structural::InsertRow { .. } | Structural::DuplicateRow { .. } | Structural::DeleteRows(_) ) { let row_ids = state.read(cx).delegate().row_ids().to_vec(); - diagnostics::Diagnostics::align_note_rows(diagnostics::DATASET_MAIN, &row_ids, cx); + diagnostics::Diagnostics::align_note_rows( + diagnostics::DATASET_MAIN, + &row_ids, + Origin::Structure, + cx, + ); } if let Some((before, after)) = renamed { - diagnostics::Diagnostics::column_renamed(diagnostics::DATASET_MAIN, &before, &after, cx); - settings::columns::rename(&before, &after, cx); - if let Some(file) = cx - .try_global::() - .map(|p| p.file.clone()) - && let Err(err) = settings::project::rename_column(&file, &before, &after) - { - log::error!("failed to rename column {before} to {after}: {err}"); - } + follow_rename(&before, &after, cx); } // The saved layout is only applied when its keys are a permutation of the live ones, so a @@ -397,7 +523,12 @@ pub fn cell_text(location: &diagnostics::Location, cx: &App) -> Option() .and_then(|h| h.0.upgrade()) @@ -413,12 +544,16 @@ pub fn set_cell_text(location: &diagnostics::Location, text: SharedString, cx: & .zip(location.row) }; if let Some((col, row)) = target { - write_cell(row, col, text, cx); + write_cell(row, col, text, origin, cx); } } /// Resolve several diagnostic locations through one table edit. -pub fn set_cell_texts(replacements: Vec<(diagnostics::Location, SharedString)>, cx: &mut App) { +pub fn set_cell_texts( + replacements: Vec<(diagnostics::Location, SharedString)>, + origin: Origin, + cx: &mut App, +) { let Some(state) = cx .try_global::() .and_then(|h| h.0.upgrade()) @@ -441,7 +576,7 @@ pub fn set_cell_texts(replacements: Vec<(diagnostics::Location, SharedString)>, .map(|((row, col), text)| (row, col, text)) .collect() }; - write_cells(cells, cx); + write_cells(cells, origin, cx); } /// What a command invoked from outside the table acts on: the selected column, or nothing at all. @@ -503,10 +638,24 @@ pub fn save_now(cx: &mut App) { }; let started = std::time::Instant::now(); let (headers, row_ids, rows) = state.read(cx).delegate().dataset_snapshot(); - match settings::project::save_dataset(&file, &headers, &row_ids, &rows) { + let author = settings::history::author(cx); + let history: Vec<_> = state + .read(cx) + .delegate() + .unsaved_history() + .iter() + .map(|entry| settings::history::Entry { + author: author.clone(), + ..entry.clone() + }) + .collect(); + match settings::project::save_dataset(&file, &headers, &row_ids, &rows, &history) { Ok(()) => { + state.update(cx, |state, _| { + state.delegate_mut().history_saved(history.len()) + }); log::debug!("saved {} rows in {:?}", rows.len(), started.elapsed()); - settings::dirty::clear(settings::dirty::PROJECT_DATA, cx) + settings::dirty::clear(settings::dirty::PROJECT_DATA, cx); } Err(err) => log::error!("failed to save project data: {err}"), } diff --git a/crates/table/src/note.rs b/crates/table/src/note.rs index 3fba52b2..b497d6c7 100644 --- a/crates/table/src/note.rs +++ b/crates/table/src/note.rs @@ -22,6 +22,7 @@ use diagnostics::{Diagnostics, Location, Severity, Source, severity_color}; use plugin_api::{ ColumnMapContributions, CommandContext, MenuContributions, MenuTarget, PluginHooks, }; +use settings::history::Origin; use crate::TableStateHandle; use crate::delegate::{QrateTableDelegate, Selection, TableChanged}; @@ -296,7 +297,7 @@ fn paste_onto(row: usize, col: usize, cx: &mut App) { let block = crate::panel::parse_tsv(&text); let reach = state.read(cx).delegate().rows_from(row, block.len()); let cells = crate::panel::paste_cells(&block, &[row], col..=col, &reach); - crate::write_cells(cells, cx); + crate::write_cells(cells, Origin::Paste, cx); } /// Build the right-click menu for `target`. Live state is read off the table global here rather @@ -381,7 +382,7 @@ pub fn menu( .action(Box::new(crate::Cut)) .on_click(move |_, _, cx| { cx.write_to_clipboard(ClipboardItem::new_string(cut.to_string())); - crate::write_cell(row, col, SharedString::default(), cx); + crate::write_cell(row, col, SharedString::default(), Origin::Clear, cx); }), ) .item( @@ -397,7 +398,7 @@ pub fn menu( PopupMenuItem::new("Clear contents") .action(Box::new(crate::Clear)) .on_click(move |_, _, cx| { - crate::write_cell(row, col, SharedString::default(), cx) + crate::write_cell(row, col, SharedString::default(), Origin::Clear, cx) }), ) .separator() @@ -425,11 +426,9 @@ pub fn menu( Target::Row(row) => { let rows = target_rows(table.read(cx).delegate(), row); structural_items(menu, Some(&rows), None) - .item( - PopupMenuItem::new("Clear row").on_click(move |_, _, cx| { - crate::write_cells(crate::blank_row(row, cx), cx) - }), - ) + .item(PopupMenuItem::new("Clear row").on_click(move |_, _, cx| { + crate::write_cells(crate::blank_row(row, cx), Origin::Clear, cx) + })) .separator() } // Built-in column controls come first; plugin entries are appended without running plugin @@ -594,7 +593,9 @@ pub fn menu( Some(&text), sub.max_w(px(360.)), cx, - move |fixed, cx| crate::write_cell(row, col, fixed, cx), + move |fixed, origin, cx| { + crate::write_cell(row, col, fixed, origin, cx) + }, ) }, ) @@ -626,10 +627,9 @@ pub fn menu( ); let loc = location.clone(); sub.when(note.is_some(), |sub| { - sub.item( - PopupMenuItem::new("Delete note") - .on_click(move |_, _, cx| Diagnostics::set_note(loc.clone(), "".into(), cx)), - ) + sub.item(PopupMenuItem::new("Delete note").on_click(move |_, _, cx| { + Diagnostics::set_note(loc.clone(), "".into(), Origin::Clear, cx) + })) }) }) } diff --git a/crates/table/src/panel.rs b/crates/table/src/panel.rs index fc13db34..39b807db 100644 --- a/crates/table/src/panel.rs +++ b/crates/table/src/panel.rs @@ -12,6 +12,7 @@ use gpui_component::{ }; use plugin_api::{CommandContext, PluginHooks, Suggestions}; +use settings::history::Origin; use crate::{ TableStateHandle, @@ -198,7 +199,12 @@ impl TablePanel { let Some(location) = state.delegate_mut().note_edit.take() else { return; }; - diagnostics::Diagnostics::set_note(location, message, cx); + diagnostics::Diagnostics::set_note( + location, + message, + settings::history::Origin::Typed, + cx, + ); cx.notify(); }); if by_enter { @@ -210,7 +216,7 @@ impl TablePanel { let _table_sub = cx.subscribe_in( &state, window, - |_this, state, event: &TableEvent, window, cx| { + |this, state, event: &TableEvent, window, cx| { match event { TableEvent::SelectCell(row, col) if *col == row_index::COL_IX => { let view = *row; @@ -300,7 +306,11 @@ impl TablePanel { state.update(cx, |s, _| s.delegate_mut().set_column_widths(widths)); Self::persist_columns(state, cx); } - TableEvent::MoveColumn(..) => Self::persist_columns(state, cx), + TableEvent::MoveColumn(..) => { + Self::persist_columns(state, cx); + settings::dirty::mark(settings::dirty::PROJECT_DATA, cx); + this.schedule_autosave(cx); + } _ => {} } state.update(cx, |_, cx| cx.emit(TableChanged)); @@ -499,7 +509,7 @@ impl TablePanel { if all { log::info!("replace all: rewrote {} cells", cells.len()); } - self.write_cells(cells, cx); + self.write_cells(cells, Origin::ReplaceAll, cx); // The rewritten cell stops matching, so everything after it shifts down one — holding the // index still is what lands us on the next match (Zed's replace-and-advance). let at = self.search_ix; @@ -600,13 +610,13 @@ impl TablePanel { /// Commit a batch of cell writes as one undoable step, then do everything a committed edit /// does. The single path cut, paste and bulk fill all take. - fn write_cells(&mut self, cells: Cells, cx: &mut Context) { + fn write_cells(&mut self, cells: Cells, origin: Origin, cx: &mut Context) { if cells.is_empty() { return; } self.state.update(cx, |state, cx| { let files = crate::names_a_file(state.delegate(), &cells, cx); - state.delegate_mut().apply_edit(cells); + state.delegate_mut().apply_edit(cells, origin); if files { photos::refresh(state, cx); } @@ -635,7 +645,7 @@ impl TablePanel { for row in rows { blanked.extend(cols.clone().map(|col| (row, col, SharedString::default()))); } - self.write_cells(blanked, cx); + self.write_cells(blanked, Origin::Clear, cx); } /// Paste the clipboard over the selection. One clipboard value across a multi-cell selection @@ -662,7 +672,7 @@ impl TablePanel { ); } let cells = paste_cells(&block, &rows, cols, &reach); - self.write_cells(cells, cx); + self.write_cells(cells, Origin::Paste, cx); } /// Apply the project's saved column layout — order, widths, and how many columns are frozen — @@ -1230,6 +1240,7 @@ mod tests { // Never `use super::*` here — the parent's `use gpui::*` would shadow `#[test]`. use diagnostics::{DATASET_MAIN, Diagnostic, Diagnostics, Location, Severity, Source}; use gpui::{SharedString, TestAppContext}; + use settings::history::Origin; fn wrote(cells: &[(usize, usize, SharedString)]) -> Vec<(usize, usize, &str)> { cells.iter().map(|(r, c, v)| (*r, *c, v.as_ref())).collect() @@ -1293,7 +1304,13 @@ mod tests { assert_eq!(Diagnostics::all(cx).len(), 2); let location = Location::cell(DATASET_MAIN, 0, None, "Title"); let fix = clustering::variant_fixes(&location, "Agnès Varda", None, cx).remove(0); - crate::write_cell(0, 0, fix.replacement, cx); + crate::write_cell( + 0, + 0, + fix.replacement, + Origin::Fix(fix.label.to_string()), + cx, + ); Diagnostics::all(cx)[0].group.clone() }); cx.run_until_parked(); @@ -1304,7 +1321,12 @@ mod tests { "Varda, Agnès" ); panel.state.update(cx, |state, _| { - assert_eq!(state.delegate_mut().undo(), Some(false)); + assert!( + state + .delegate_mut() + .undo() + .is_some_and(|c| !crate::history::moves_rows(&c)) + ); assert_eq!( state.delegate_mut().undo(), None, @@ -1349,12 +1371,18 @@ mod tests { "fixed".into(), ), ], + Origin::Fix("Use fixed".into()), cx, ); assert_eq!(panel.state.read(cx).delegate().cell(0, 0).unwrap(), "fixed"); assert_eq!(panel.state.read(cx).delegate().cell(1, 0).unwrap(), "fixed"); panel.state.update(cx, |state, _| { - assert_eq!(state.delegate_mut().undo(), Some(false)); + assert!( + state + .delegate_mut() + .undo() + .is_some_and(|c| !crate::history::moves_rows(&c)) + ); assert_eq!(state.delegate_mut().undo(), None); assert_eq!(state.delegate().cell(0, 0).unwrap(), "one"); assert_eq!(state.delegate().cell(1, 0).unwrap(), "two"); diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 6b2d1e12..b724fac0 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -431,7 +431,7 @@ impl DetailsPanel { // drops the rows whose text this didn't change, so committing an untouched shared field // costs nothing. let cells = rows.into_iter().map(|row| (row, col, value.clone())); - table::write_cells(cells.collect(), cx); + table::write_cells(cells.collect(), settings::history::Origin::Details, cx); cx.notify(); } From e3c707b993b965c987a33375e2d9e192d5a58b96 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 13 Sep 2026 13:49:43 -0700 Subject: [PATCH 03/16] feat(history): add the History panel A right-dock panel lists every change newest first: unsaved changes on top, saved ones under day headers, with bursts of edits by one person collapsed. Right-clicking a change restores the project to it, restores a single value, or names the version; the header filters by kind or to named versions and can clear the history. Saved layouts that predate a panel now adopt it into its default dock instead of losing it. Refs #132 --- crates/settings/src/history.rs | 167 ++++- crates/settings/src/project.rs | 2 +- crates/table/src/delegate.rs | 2 +- crates/workspace/src/lib.rs | 69 +- crates/workspace/src/panel_registry.rs | 4 +- crates/workspace/src/panels/history.rs | 860 +++++++++++++++++++++++++ crates/workspace/src/panels/mod.rs | 2 + 7 files changed, 1058 insertions(+), 48 deletions(-) create mode 100644 crates/workspace/src/panels/history.rs diff --git a/crates/settings/src/history.rs b/crates/settings/src/history.rs index 1eb72117..d7452586 100644 --- a/crates/settings/src/history.rs +++ b/crates/settings/src/history.rs @@ -42,6 +42,25 @@ pub enum Origin { Restore(EntryId), } +impl Origin { + /// How the History panel names where a change came from. + pub fn label(&self) -> String { + match self { + Origin::Typed => "Typed".into(), + Origin::Paste => "Pasted".into(), + Origin::Clear => "Cleared".into(), + Origin::ReplaceAll => "Replace".into(), + Origin::Details => "Details panel".into(), + Origin::Spelling => "Spelling fix".into(), + Origin::Fix(fix) => format!("Fix: {fix}"), + Origin::Structure => "Rows and columns".into(), + Origin::Undo => "Undo".into(), + Origin::Redo => "Redo".into(), + Origin::Restore(id) => format!("Restored to #{id}"), + } + } +} + /// One recorded change. Every variant is reversible from what it carries alone. #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] pub enum Change { @@ -260,9 +279,35 @@ pub(crate) fn append(conn: &Connection, entries: &[Entry]) -> Result<()> { Ok(()) } +/// An entry as the History panel lists it: with its name, if it has been given one, and when it was +/// made in the archivist's own time zone. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Listed { + pub entry: Entry, + pub label: Option, + /// `YYYY-MM-DD`, local. + pub day: String, + /// `HH:MM`, local. + pub time: String, + /// Whole days between `day` and today, so "Today" and "Yesterday" need no calendar here. + pub days_ago: i64, +} + /// Every entry newer than `after`, oldest first. A file that has never logged anything reads as /// an empty history. pub fn entries_after(path: &Path, after: EntryId) -> Result> { + Ok(select(path, "id > ?1 ORDER BY id", after, -1)? + .into_iter() + .map(|listed| listed.entry) + .collect()) +} + +/// Up to `limit` entries older than `before`, newest first — one page of the History panel. +pub fn page(path: &Path, before: EntryId, limit: i64) -> Result> { + select(path, "id < ?1 ORDER BY id DESC", before, limit) +} + +fn select(path: &Path, filter: &str, bound: EntryId, limit: i64) -> Result> { let conn = crate::project::open_ro(path)?; let exists: i64 = conn.query_row( "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = '__history'", @@ -272,41 +317,82 @@ pub fn entries_after(path: &Path, after: EntryId) -> Result> { if exists == 0 { return Ok(Vec::new()); } - let mut entries = conn - .prepare("SELECT id, at, author, origin FROM __history WHERE id > ?1 ORDER BY id")? - .query_map([after], |r| { + let local = "date(at, 'unixepoch', 'localtime')"; + let mut listed = conn + .prepare(&format!( + "SELECT id, at, author, origin, label, {local}, strftime('%H:%M', at, 'unixepoch', 'localtime'), + CAST(julianday(date('now', 'localtime')) - julianday({local}) AS INTEGER) + FROM __history WHERE {filter} LIMIT ?2" + ))? + .query_map(params![bound, limit], |r| { Ok(( - r.get::<_, EntryId>(0)?, - r.get::<_, i64>(1)?, - r.get::<_, Option>(2)?, - r.get::<_, String>(3)?, + ( + r.get::<_, EntryId>(0)?, + r.get::<_, i64>(1)?, + r.get::<_, Option>(2)?, + r.get::<_, String>(3)?, + ), + ( + r.get::<_, Option>(4)?, + r.get::<_, String>(5)?, + r.get::<_, String>(6)?, + r.get::<_, i64>(7)?, + ), )) })? .map(|row| { - let (id, at, author, origin) = row?; - Ok(Entry { - id, - at, - author, - origin: serde_json::from_str(&origin).context("Read history origin")?, - changes: Vec::new(), + let ((id, at, author, origin), (label, day, time, days_ago)) = row?; + Ok(Listed { + entry: Entry { + id, + at, + author, + origin: serde_json::from_str(&origin).context("Read history origin")?, + changes: Vec::new(), + }, + label, + day, + time, + days_ago, }) }) .collect::>>()?; let mut changes = conn.prepare("SELECT change FROM __history_changes WHERE entry_id = ?1 ORDER BY seq")?; - for entry in &mut entries { - entry.changes = changes - .query_map([entry.id], |r| r.get::<_, String>(0))? + for listed in &mut listed { + listed.entry.changes = changes + .query_map([listed.entry.id], |r| r.get::<_, String>(0))? .map(|c| serde_json::from_str(&c?).context("Read history change")) .collect::>()?; } - Ok(entries) + Ok(listed) +} + +/// Name an entry — "Before Islandora ingest" — or, with `None`, take the name away again. +pub fn set_label(path: &Path, id: EntryId, label: Option<&str>) -> Result<()> { + crate::project::open_rw(path)? + .execute( + "UPDATE __history SET label = ?2 WHERE id = ?1", + params![id, label], + ) + .context("Name history entry")?; + Ok(()) +} + +/// Forget the whole log. The data it describes is untouched. +pub fn clear(path: &Path) -> Result<()> { + let mut conn = crate::project::open_rw(path)?; + let tx = conn.transaction()?; + tx.execute_batch(HISTORY_DDL)?; + tx.execute_batch("DELETE FROM __history_changes; DELETE FROM __history;") + .context("Clear history")?; + tx.commit().context("Commit cleared history")?; + Ok(()) } #[cfg(test)] mod tests { - use super::{Change, Entry, Origin, entries_after}; + use super::{Change, Entry, EntryId, Origin, clear, entries_after, page, set_label}; use crate::project::{ProjectSpec, create_project_file, save_dataset, write_notes}; fn project(name: &str) -> std::path::PathBuf { @@ -384,6 +470,49 @@ mod tests { assert_eq!(entries_after(&path, 2).unwrap().len(), 1); } + /// The panel pages newest first, a name sticks to its entry until it is taken away, and + /// clearing forgets the log without touching the data. + #[test] + fn pages_run_newest_first_and_names_and_clearing_stick() { + let path = project("paging.qrate"); + let headers = vec!["Title".to_string()]; + let entries: Vec = (0..5) + .map(|n| { + Entry::new( + Origin::Typed, + vec![edit(&n.to_string(), &(n + 1).to_string())], + None, + ) + }) + .collect(); + save_dataset(&path, &headers, &[1], &[vec!["5".into()]], &entries).unwrap(); + + let first = page(&path, EntryId::MAX, 2).unwrap(); + assert_eq!( + first.iter().map(|l| l.entry.id).collect::>(), + vec![5, 4] + ); + assert_eq!(first[0].days_ago, 0, "written just now"); + let older = page(&path, 4, 10).unwrap(); + assert_eq!( + older.iter().map(|l| l.entry.id).collect::>(), + vec![3, 2, 1] + ); + + set_label(&path, 2, Some("Before ingest")).unwrap(); + assert_eq!( + page(&path, 3, 1).unwrap()[0].label.as_deref(), + Some("Before ingest") + ); + set_label(&path, 2, None).unwrap(); + assert_eq!(page(&path, 3, 1).unwrap()[0].label, None); + + clear(&path).unwrap(); + assert!(page(&path, EntryId::MAX, 10).unwrap().is_empty()); + let data = crate::project::load_project_file(&path).unwrap(); + assert_eq!(data.rows, vec![vec!["5".to_string()]]); + } + #[test] fn an_inverse_undoes_and_its_own_inverse_is_the_original() { let moved = Change::ColumnMoved { diff --git a/crates/settings/src/project.rs b/crates/settings/src/project.rs index b35e7da7..bbceb6f8 100644 --- a/crates/settings/src/project.rs +++ b/crates/settings/src/project.rs @@ -209,7 +209,7 @@ impl CurrentProject { /// rollback journals; NORMAL is only equivalent under WAL — and a busy /// timeout so a transient AV/indexer file lock retries instead of surfacing /// as SQLITE_BUSY. -fn open_rw(path: &Path) -> Result { +pub(crate) fn open_rw(path: &Path) -> Result { let conn = Connection::open(path).with_context(|| format!("Open project at {path:?}"))?; conn.busy_timeout(Duration::from_secs(5)) .context("Set busy timeout")?; diff --git a/crates/table/src/delegate.rs b/crates/table/src/delegate.rs index f0e123e9..22abe09a 100644 --- a/crates/table/src/delegate.rs +++ b/crates/table/src/delegate.rs @@ -974,7 +974,7 @@ impl QrateTableDelegate { self.row_ids.get(source).copied() } - pub(crate) fn row_ids(&self) -> &[settings::project::RowId] { + pub fn row_ids(&self) -> &[settings::project::RowId] { &self.row_ids } diff --git a/crates/workspace/src/lib.rs b/crates/workspace/src/lib.rs index bd7c3af4..09c1cb63 100644 --- a/crates/workspace/src/lib.rs +++ b/crates/workspace/src/lib.rs @@ -29,8 +29,7 @@ use settings::AppSettings; use diagnostics::ProblemsPanel; -use crate::panel_registry::PROBLEMS_META; -use crate::panels::{AGENT_META, AgentPanel, DetailsPanel}; +use crate::panels::{AgentPanel, DetailsPanel, HistoryPanel}; use crate::views::ViewsPanel; /// Settings key under which the serialized [`DockAreaState`] is persisted — @@ -101,15 +100,11 @@ impl Workspace { let weak = ctx.dock_area(); panel_handle(cx.new(|cx| ViewsPanel::new(weak, window, cx))) }); - register_panel(cx, "DetailsPanel", |_ctx, window, cx| { - panel_handle(cx.new(|cx| DetailsPanel::new(window, cx))) - }); - register_panel(cx, "AgentPanel", |_ctx, window, cx| { - panel_handle(cx.new(|cx| AgentPanel::new(window, cx))) - }); - register_panel(cx, "ProblemsPanel", |_ctx, window, cx| { - panel_handle(cx.new(|cx| ProblemsPanel::new(window, cx))) - }); + for meta in PANELS { + register_panel(cx, meta.name, move |_ctx, window, cx| { + build_panel(meta.name, window, cx) + }); + } // The appearance is a separate object in 0.6, installed as the area's renderer. The // handle it hands back is how its settings are reached afterwards. @@ -122,20 +117,10 @@ impl Workspace { let centre = cx.new(|cx| ViewsPanel::new(weak.clone(), window, cx)); // Which dock each panel starts in is the panel's own declaration, so they're built // as a flat list and grouped by it rather than one `set_*_dock` call apiece. - let panels: [(&PanelMeta, Arc); 3] = [ - ( - &DETAILS_META, - panel_handle(cx.new(|cx| DetailsPanel::new(window, cx))), - ), - ( - &PROBLEMS_META, - panel_handle(cx.new(|cx| ProblemsPanel::new(window, cx))), - ), - ( - &AGENT_META, - panel_handle(cx.new(|cx| AgentPanel::new(window, cx))), - ), - ]; + let panels: Vec<(&PanelMeta, Arc)> = PANELS + .iter() + .map(|meta| (*meta, build_panel(meta.name, window, cx))) + .collect(); dock_area.update(cx, |area, cx| { // `tabs`, not a bare panel: a bare centre has no title bar at all — and the title @@ -176,6 +161,7 @@ impl Workspace { // toggle arrows. A skin setting in 0.6, so it survives `load` without being reapplied. skin.set_toggle_button_visible(false, cx); + Self::adopt_new_panels(&dock_area, window, cx); // Saved layouts from older builds may contain arrangements qrate no longer permits. PanelRegistry::enforce_edge_tabs(&dock_area, window, cx); // Whichever path ran above built the panels; this is what learns where they landed. @@ -260,12 +246,34 @@ impl Workspace { /// so nothing else re-triggers `restore_layout` for the newly opened project. pub fn reload_layout(&mut self, window: &mut Window, cx: &mut Context) { if Self::restore_layout(&self.dock_area, window, cx) { + Self::adopt_new_panels(&self.dock_area, window, cx); // `load` rebuilt every panel from its saved name, so the old entries are orphans. PanelRegistry::sync(&self.dock_area, cx); PanelRegistry::focus_frontmost(DockPlacement::Center, &self.dock_area, window, cx); } } + /// Give a panel that a saved layout predates its default dock, so a new panel reaches people + /// who already arranged theirs without throwing that arrangement away. + fn adopt_new_panels(dock_area: &Entity, window: &mut Window, cx: &mut App) { + PanelRegistry::sync(dock_area, cx); + let missing: Vec<&PanelMeta> = PANELS + .iter() + .filter(|meta| PanelRegistry::placement(meta.name, cx).is_none()) + .copied() + .collect(); + if missing.is_empty() { + return; + } + dock_area.update(cx, |area, cx| { + for meta in missing { + let view = build_panel(meta.name, window, cx); + area.add_panel_view(view, meta.default_placement, None, window, cx); + } + }); + PanelRegistry::sync(dock_area, cx); + } + /// Applies a previously saved dock arrangement, returning `true` if one was loaded. `false` /// (no saved layout / corrupt / stale version / load error) tells the caller to fall back to /// the default static layout. @@ -320,6 +328,17 @@ impl Workspace { } } +/// A fresh dockable panel by its registered name. The one list of constructors, shared by layout +/// restore, the default layout, and adopting a panel a saved layout predates. +fn build_panel(name: &str, window: &mut Window, cx: &mut App) -> Arc { + match name { + "DetailsPanel" => panel_handle(cx.new(|cx| DetailsPanel::new(window, cx))), + "ProblemsPanel" => panel_handle(cx.new(|cx| ProblemsPanel::new(window, cx))), + "HistoryPanel" => panel_handle(cx.new(|cx| HistoryPanel::new(window, cx))), + _ => panel_handle(cx.new(|cx| AgentPanel::new(window, cx))), + } +} + /// A panel name a saved layout is allowed to hold: the three dockable ones plus the centre, which /// has no `PanelMeta` because it never moves. Matches what [`Workspace::new`] registers, so adding /// a panel to [`PANELS`] is still the only edit. diff --git a/crates/workspace/src/panel_registry.rs b/crates/workspace/src/panel_registry.rs index 32823214..d1129299 100644 --- a/crates/workspace/src/panel_registry.rs +++ b/crates/workspace/src/panel_registry.rs @@ -20,7 +20,7 @@ use gpui_component::{ }; use crate::Workspace; -use crate::panels::{AGENT_META, DETAILS_META}; +use crate::panels::{AGENT_META, DETAILS_META, HISTORY_META}; /// Which status-bar group a panel's button sits in. #[derive(Copy, Clone, PartialEq)] @@ -63,7 +63,7 @@ pub static PROBLEMS_META: PanelMeta = PanelMeta { /// Every dockable panel, in status-bar order within its group. The centre table isn't one: it has /// no button and nowhere to move to. -pub static PANELS: [&PanelMeta; 3] = [&DETAILS_META, &PROBLEMS_META, &AGENT_META]; +pub static PANELS: [&PanelMeta; 4] = [&DETAILS_META, &PROBLEMS_META, &HISTORY_META, &AGENT_META]; pub struct PanelEntry { pub meta: &'static PanelMeta, diff --git a/crates/workspace/src/panels/history.rs b/crates/workspace/src/panels/history.rs new file mode 100644 index 00000000..de9ff23c --- /dev/null +++ b/crates/workspace/src/panels/history.rs @@ -0,0 +1,860 @@ +use std::collections::{HashMap, HashSet}; +use std::path::PathBuf; +use std::time::{SystemTime, UNIX_EPOCH}; + +use gpui::prelude::FluentBuilder as _; +use gpui::*; +use gpui_component::dock::{BasePanel, DockPlacement, Panel, PanelEvent}; +use gpui_component::input::{Input, InputEvent, InputState}; +use gpui_component::menu::{ContextMenuExt as _, DropdownMenu as _, PopupMenu, PopupMenuItem}; +use gpui_component::scroll::ScrollableElement as _; +use gpui_component::{ + ActiveTheme as _, IconName, Sizable as _, + button::{Button, ButtonVariants as _}, + h_flex, v_flex, +}; +use settings::history::{Change, Entry, EntryId, Listed, Origin}; +use settings::project::{CurrentProject, RowId}; +use table::{TableChanged, TableStateHandle}; + +use crate::BottomDockCrop; +use crate::panel_registry::PanelMeta; + +pub static HISTORY_META: PanelMeta = PanelMeta { + name: "HistoryPanel", + icon: IconName::Undo2, + label: "History", + default_placement: DockPlacement::Right, + badge: false, +}; + +/// How many saved entries one "Load older" brings in. +const PAGE: i64 = 200; + +/// Consecutive edits closer together than this, by the same person and the same means, read as +/// one burst of work and collapse under one row. +const BURST_SECS: i64 = 5 * 60; + +#[derive(Clone, Copy, PartialEq, Eq)] +enum Filter { + All, + Cells, + Structure, + Notes, + Fixes, +} + +impl Filter { + const ALL: [Filter; 5] = [ + Filter::All, + Filter::Cells, + Filter::Structure, + Filter::Notes, + Filter::Fixes, + ]; + + fn label(self) -> &'static str { + match self { + Filter::All => "All changes", + Filter::Cells => "Cell edits", + Filter::Structure => "Rows and columns", + Filter::Notes => "Notes", + Filter::Fixes => "Fixes", + } + } + + fn admits(self, entry: &Entry) -> bool { + let any = |f: fn(&Change) -> bool| entry.changes.iter().any(f); + match self { + Filter::All => true, + Filter::Cells => any(|c| matches!(c, Change::Cell { .. })), + Filter::Structure => any(|c| !matches!(c, Change::Cell { .. } | Change::Note { .. })), + Filter::Notes => any(|c| matches!(c, Change::Note { .. })), + Filter::Fixes => matches!(entry.origin, Origin::Fix(_) | Origin::Spelling), + } + } +} + +/// Every change to the project's data and notes, newest first, with a way back to any of them. +/// +/// Saved entries are read from the `.qrate` file a page at a time; entries not saved yet come +/// straight from the table and sit on top, where there is nothing to restore to yet. +pub struct HistoryPanel { + focus_handle: FocusHandle, + scroll: ScrollHandle, + saved: Vec, + /// Whether a "Load older" would find anything. + more: bool, + /// The file and modification time `saved` was read at. Everything that can add an entry + /// rewrites the file, so an unchanged stamp means there is nothing new to read. + read_at: Option<(PathBuf, SystemTime)>, + filter: Filter, + named_only: bool, + /// Bursts opened up, by the id of their newest entry. + expanded: HashSet, + /// The entry being named, and the box the name is typed in. + naming: Option<(EntryId, Entity, Subscription)>, + _handle_sub: Subscription, + _table_sub: Option, + _notes_sub: Subscription, + _crop_sub: Subscription, +} + +impl HistoryPanel { + pub fn new(_window: &mut Window, cx: &mut Context) -> Self { + let mut this = Self { + focus_handle: cx.focus_handle(), + scroll: ScrollHandle::new(), + saved: Vec::new(), + more: false, + read_at: None, + filter: Filter::All, + named_only: false, + expanded: HashSet::new(), + naming: None, + _handle_sub: cx.observe_global::(|this: &mut Self, cx| { + this.bind(cx); + cx.notify(); + }), + _table_sub: None, + // Notes are written the moment they change, without the table hearing about it. + _notes_sub: cx.observe_global::(|this: &mut Self, cx| { + this.reload(false, cx); + }), + _crop_sub: cx.observe_global::(|_this: &mut Self, cx| cx.notify()), + }; + this.bind(cx); + this + } + + fn bind(&mut self, cx: &mut Context) { + self._table_sub = cx + .try_global::() + .and_then(|h| h.0.upgrade()) + .map(|entity| { + cx.subscribe(&entity, |this, _state, _event: &TableChanged, cx| { + this.reload(false, cx); + cx.notify(); + }) + }); + self.reload(false, cx); + } + + /// Re-read the saved history if the project file has changed since it was last read. `force` + /// re-reads regardless, for this panel's own writes. + fn reload(&mut self, force: bool, cx: &mut Context) { + let Some(file) = cx.try_global::().map(|p| p.file.clone()) else { + self.saved.clear(); + self.read_at = None; + return; + }; + let stamp = std::fs::metadata(&file) + .and_then(|m| m.modified()) + .unwrap_or(UNIX_EPOCH); + let fresh = Some((file.clone(), stamp)); + if !force && self.read_at == fresh { + return; + } + let limit = PAGE.max(self.saved.len() as i64); + match settings::history::page(&file, EntryId::MAX, limit) { + Ok(page) => { + self.more = page.len() as i64 == limit; + self.saved = page; + } + Err(err) => log::error!("couldn't read the project history: {err}"), + } + self.read_at = fresh; + cx.notify(); + } + + fn load_older(&mut self, cx: &mut Context) { + let (Some(file), Some(last)) = ( + cx.try_global::().map(|p| p.file.clone()), + self.saved.last().map(|l| l.entry.id), + ) else { + return; + }; + match settings::history::page(&file, last, PAGE) { + Ok(page) => { + self.more = page.len() as i64 == PAGE; + self.saved.extend(page); + } + Err(err) => log::error!("couldn't read older project history: {err}"), + } + cx.notify(); + } + + fn start_naming(&mut self, id: EntryId, window: &mut Window, cx: &mut Context) { + let current = self + .saved + .iter() + .find(|l| l.entry.id == id) + .and_then(|l| l.label.clone()) + .unwrap_or_default(); + let input = cx.new(|cx| { + InputState::new(window, cx) + .placeholder("Name this version") + .default_value(current) + }); + input.update(cx, |input, cx| input.focus(window, cx)); + let sub = cx.subscribe_in( + &input, + window, + |this, input, event: &InputEvent, _window, cx| match event { + InputEvent::PressEnter { .. } => { + let name = input.read(cx).value().trim().to_string(); + if let Some((id, _, _)) = this.naming.take() { + this.name(id, (!name.is_empty()).then_some(name.as_str()), cx); + } + } + InputEvent::Blur => { + this.naming = None; + cx.notify(); + } + _ => {} + }, + ); + self.naming = Some((id, input, sub)); + cx.notify(); + } + + fn name(&mut self, id: EntryId, label: Option<&str>, cx: &mut Context) { + if let Some(file) = cx.try_global::().map(|p| p.file.clone()) + && let Err(err) = settings::history::set_label(&file, id, label) + { + log::error!("couldn't name history entry #{id}: {err}"); + } + self.reload(true, cx); + } + + /// Ask before putting the whole project back, saying how much that reverses. + fn confirm_restore(listed: &Listed, window: &mut Window, cx: &mut App) { + let Some(file) = cx.try_global::().map(|p| p.file.clone()) else { + return; + }; + let since = settings::history::entries_after(&file, listed.entry.id) + .map(|entries| entries.iter().map(|e| e.changes.len()).sum::()) + .unwrap_or_default() + + cx.try_global::() + .and_then(|h| h.0.upgrade()) + .map_or(0, |state| { + let delegate = state.read(cx).delegate(); + delegate + .unsaved_history() + .iter() + .map(|e| e.changes.len()) + .sum() + }); + let id = listed.entry.id; + let answer = window.prompt( + PromptLevel::Warning, + &format!( + "Restore the project to {} at {}?", + day_label(&listed.day, listed.days_ago), + listed.time + ), + Some(&format!( + "{since} change{} made since will be reversed. The restore is added to the history, and Undo takes it back.", + if since == 1 { "" } else { "s" } + )), + &["Restore", "Cancel"], + cx, + ); + cx.spawn(async move |cx| { + if answer.await == Ok(0) { + cx.update(|cx| table::restore_to(id, cx)); + } + }) + .detach(); + } + + fn confirm_clear(panel: WeakEntity, window: &mut Window, cx: &mut App) { + let answer = window.prompt( + PromptLevel::Critical, + "Clear this project's history?", + Some("Every recorded change is forgotten and nothing before now can be restored. The data itself is not changed."), + &["Clear History", "Cancel"], + cx, + ); + cx.spawn(async move |cx| { + if answer.await != Ok(0) { + return; + } + cx.update(|cx| { + if let Some(file) = cx.try_global::().map(|p| p.file.clone()) + && let Err(err) = settings::history::clear(&file) + { + log::error!("couldn't clear the project history: {err}"); + } + panel.update(cx, |this, cx| this.reload(true, cx)).ok(); + }); + }) + .detach(); + } +} + +/// `Today`, `Yesterday`, or `3 Sep 2026`. +fn day_label(day: &str, days_ago: i64) -> String { + const MONTHS: [&str; 12] = [ + "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec", + ]; + match days_ago { + 0 => "Today".into(), + 1 => "Yesterday".into(), + _ => { + let mut parts = day.split('-'); + match (parts.next(), parts.next(), parts.next()) { + (Some(y), Some(m), Some(d)) => { + let month = m.parse::().ok().and_then(|m| MONTHS.get(m - 1)); + format!("{} {} {y}", d.trim_start_matches('0'), month.unwrap_or(&m)) + } + _ => day.into(), + } + } + } +} + +/// One line saying what an entry did, in the grid's current terms: rows by the number the `#` +/// column shows now, and a row that has since gone as exactly that. +fn describe(entry: &Entry, rows: &HashMap) -> String { + let row = |id: &RowId| { + rows.get(id) + .map_or_else(|| "a deleted row".to_string(), |p| format!("row {}", p + 1)) + }; + let quote = |text: &str| match text.chars().count() { + 0 => "(empty)".to_string(), + n if n > 40 => format!("“{}…”", text.chars().take(40).collect::()), + _ => format!("“{text}”"), + }; + let count = |n: usize, one: &str, many: &str| match n { + 1 => format!("1 {one}"), + n => format!("{n} {many}"), + }; + let changes = &entry.changes; + match changes.as_slice() { + [ + Change::Cell { + row: id, + column, + before, + after, + }, + ] => format!( + "{column}, {}: {} → {}", + row(id), + quote(before), + quote(after) + ), + [Change::ColumnAdded { column, .. }] => format!("Added column {}", quote(column)), + [Change::ColumnRemoved { column, .. }] => format!("Deleted column {}", quote(column)), + [Change::ColumnRenamed { before, after }] => { + format!("Renamed column {} to {}", quote(before), quote(after)) + } + [Change::ColumnMoved { column, .. }] => format!("Moved column {}", quote(column)), + [ + Change::Note { + row: id, + column, + before, + after, + }, + ] => { + let what = match (before, after) { + (None, _) => "Added a note", + (_, None) => "Removed a note", + _ => "Edited a note", + }; + match (id, column) { + (Some(id), Some(column)) => format!("{what} on {column}, {}", row(id)), + (Some(id), None) => format!("{what} on {}", row(id)), + (None, Some(column)) => format!("{what} on column {column}"), + (None, None) => format!("{what} on the project"), + } + } + _ if changes.iter().all(|c| matches!(c, Change::Cell { .. })) => { + let mut columns: Vec<&str> = Vec::new(); + for change in changes { + if let Change::Cell { column, .. } = change + && !columns.contains(&column.as_str()) + { + columns.push(column); + } + } + let named = match columns.len() { + n if n > 3 => format!("{} and {} more", columns[..3].join(", "), n - 3), + _ => columns.join(", "), + }; + format!("{} in {named}", count(changes.len(), "cell", "cells")) + } + _ if changes.iter().all(|c| matches!(c, Change::RowAdded { .. })) => { + format!("Added {}", count(changes.len(), "row", "rows")) + } + _ if changes + .iter() + .all(|c| matches!(c, Change::RowRemoved { .. })) => + { + format!("Deleted {}", count(changes.len(), "row", "rows")) + } + _ => count(changes.len(), "change", "changes"), + } +} + +impl Focusable for HistoryPanel { + fn focus_handle(&self, _: &App) -> FocusHandle { + self.focus_handle.clone() + } +} + +impl EventEmitter for HistoryPanel {} + +impl BasePanel for HistoryPanel { + fn panel_name(&self) -> &'static str { + "HistoryPanel" + } + + fn closable(&self, _cx: &App) -> bool { + false + } + + fn zoomable(&self, _cx: &App) -> bool { + true + } +} + +impl Panel for HistoryPanel { + fn title(&mut self, _w: &mut Window, _cx: &mut Context) -> impl IntoElement { + SharedString::from("History") + } + + fn title_suffix( + &mut self, + _w: &mut Window, + cx: &mut Context, + ) -> Option { + let panel = cx.entity().downgrade(); + let (filter, named_only) = (self.filter, self.named_only); + Some( + h_flex() + .gap_1() + .items_center() + .child( + Button::new("history-filter") + .ghost() + .xsmall() + .label(if named_only { + "Named versions" + } else { + filter.label() + }) + .dropdown_menu({ + let panel = panel.clone(); + move |menu: PopupMenu, _window, _cx| { + let menu = Filter::ALL.into_iter().fold(menu, |menu, pick| { + let panel = panel.clone(); + menu.item( + PopupMenuItem::new(pick.label()) + .checked(!named_only && pick == filter) + .on_click(move |_, _, cx| { + panel + .update(cx, |this, cx| { + this.filter = pick; + this.named_only = false; + cx.notify(); + }) + .ok(); + }), + ) + }); + let panel = panel.clone(); + menu.separator().item( + PopupMenuItem::new("Named versions") + .checked(named_only) + .on_click(move |_, _, cx| { + panel + .update(cx, |this, cx| { + this.named_only = !this.named_only; + cx.notify(); + }) + .ok(); + }), + ) + } + }), + ) + .child( + Button::new("history-more") + .icon(IconName::Ellipsis) + .ghost() + .xsmall() + .dropdown_menu(move |menu: PopupMenu, _window, _cx| { + let panel = panel.clone(); + menu.item(PopupMenuItem::new("Clear History…").on_click( + move |_, window, cx| Self::confirm_clear(panel.clone(), window, cx), + )) + }), + ), + ) + } +} + +impl Render for HistoryPanel { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let theme = cx.theme(); + let (muted, border, hover_bg, accent) = ( + theme.muted_foreground, + theme.border, + theme.secondary_hover, + theme.accent_foreground, + ); + let crop = cx.try_global::().map_or(px(0.), |c| c.0); + let panel = cx.entity().downgrade(); + + let (rows, unsaved): (HashMap, Vec) = cx + .try_global::() + .and_then(|h| h.0.upgrade()) + .map(|state| { + let delegate = state.read(cx).delegate(); + ( + delegate + .row_ids() + .iter() + .enumerate() + .map(|(p, id)| (*id, p)) + .collect(), + delegate.unsaved_history().to_vec(), + ) + }) + .unwrap_or_default(); + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map_or(0, |d| d.as_secs() as i64); + let (filter, named_only) = (self.filter, self.named_only); + let admits = |entry: &Entry, label: bool| { + if named_only { + label + } else { + filter.admits(entry) + } + }; + + let unsaved: Vec = unsaved + .into_iter() + .rev() + .filter(|e| admits(e, false)) + .collect(); + let saved: Vec<&Listed> = self + .saved + .iter() + .filter(|l| admits(&l.entry, l.label.is_some())) + .collect(); + + // Bursts: runs of adjacent, unnamed entries by one person, one means, one day. + let mut bursts: Vec> = Vec::new(); + for listed in saved { + let joins = bursts.last().and_then(|b| b.last()).is_some_and(|prev| { + prev.label.is_none() + && listed.label.is_none() + && prev.day == listed.day + && prev.entry.author == listed.entry.author + && prev.entry.origin == listed.entry.origin + && matches!( + listed.entry.origin, + Origin::Typed | Origin::Details | Origin::Paste | Origin::Clear + ) + && prev.entry.at - listed.entry.at <= BURST_SECS + }); + match (joins, bursts.last_mut()) { + (true, Some(burst)) => burst.push(listed), + _ => bursts.push(vec![listed]), + } + } + + let entry_row = |id: ElementId, + entry: &Entry, + listed: Option<&Listed>, + meta: String, + indent: bool| { + let dimmed = matches!(entry.origin, Origin::Undo | Origin::Redo); + let naming = listed.and_then(|l| { + self.naming + .as_ref() + .filter(|(id, _, _)| *id == l.entry.id) + .map(|(_, input, _)| input.clone()) + }); + let actions = listed.cloned().map(|listed| { + let panel = panel.clone(); + move |menu: PopupMenu, _window: &mut Window, _cx: &mut Context| { + let single = match listed.entry.changes.as_slice() { + [ + Change::Cell { + row, + column, + before, + .. + }, + ] => Some((*row, column.clone(), before.clone())), + _ => None, + }; + let (restore, name, unname) = (listed.clone(), panel.clone(), panel.clone()); + let id = listed.entry.id; + menu.item( + PopupMenuItem::new("Restore Project to Here…").on_click( + move |_, window, cx| Self::confirm_restore(&restore, window, cx), + ), + ) + .when_some(single, |menu, (row, column, before)| { + menu.item(PopupMenuItem::new("Restore This Value").on_click( + move |_, _, cx| { + table::restore_value(row, &column, before.clone().into(), id, cx) + }, + )) + }) + .separator() + .item(PopupMenuItem::new("Name This Version…").on_click( + move |_, window, cx| { + name.update(cx, |this, cx| this.start_naming(id, window, cx)) + .ok(); + }, + )) + .when(listed.label.is_some(), |menu| { + menu.item(PopupMenuItem::new("Remove Name").on_click(move |_, _, cx| { + unname.update(cx, |this, cx| this.name(id, None, cx)).ok(); + })) + }) + } + }); + v_flex() + .id(id) + .w_full() + .gap_0p5() + .px_2() + .py_1() + .when(indent, |row| row.pl_6()) + .border_b_1() + .border_color(border) + .hover(|row| row.bg(hover_bg)) + .when(dimmed, |row| row.opacity(0.6)) + .when_some(listed.and_then(|l| l.label.clone()), |row, label| { + row.child( + div() + .text_xs() + .font_weight(FontWeight::SEMIBOLD) + .text_color(accent) + .child(label), + ) + }) + .when_some(naming, |row, input| row.child(Input::new(&input).xsmall())) + .child( + div() + .text_sm() + .overflow_hidden() + .text_ellipsis() + .child(describe(entry, &rows)), + ) + .child(div().text_xs().text_color(muted).child(meta)) + .map(|row| match actions { + Some(actions) => row.context_menu(actions).into_any_element(), + None => row.into_any_element(), + }) + }; + let meta = |entry: &Entry, when: String| { + [Some(entry.origin.label()), entry.author.clone(), Some(when)] + .into_iter() + .flatten() + .collect::>() + .join(" · ") + }; + let header = |text: String| { + div() + .px_2() + .pt_2() + .pb_1() + .text_xs() + .font_weight(FontWeight::SEMIBOLD) + .text_color(muted) + .child(text) + }; + + let empty = unsaved.is_empty() && bursts.is_empty(); + let mut list = v_flex() + .id("history-list") + .size_full() + .min_h_0() + .overflow_y_scroll() + .track_scroll(&self.scroll) + .vertical_scrollbar(&self.scroll) + .pr_2() + .pb(px(8.) + crop); + + if empty { + list = list.child(div().p_3().text_sm().text_color(muted).child( + match (cx.has_global::(), named_only) { + (false, _) => "Open a project to see its history.", + (true, true) => "No named versions yet. Right-click a change to name it.", + (true, false) => "No changes yet. Every edit to this project's data and notes will be listed here.", + }, + )); + } + if !unsaved.is_empty() { + list = list.child(header("Not saved yet".into())); + for (ix, entry) in unsaved.iter().enumerate() { + let ago = match (now - entry.at) / 60 { + 0 => "just now".to_string(), + m => format!("{m} min ago"), + }; + list = list.child(entry_row( + ElementId::NamedInteger("history-unsaved".into(), ix as u64), + entry, + None, + meta(entry, ago), + false, + )); + } + } + let mut day = None; + for burst in &bursts { + let head = burst[0]; + if day != Some(&head.day) { + day = Some(&head.day); + list = list.child(header(day_label(&head.day, head.days_ago))); + } + let id = head.entry.id; + let row_id = |listed: &Listed| { + ElementId::NamedInteger("history-entry".into(), listed.entry.id as u64) + }; + if burst.len() == 1 { + list = list.child(entry_row( + row_id(head), + &head.entry, + Some(head), + meta(&head.entry, head.time.clone()), + false, + )); + continue; + } + let open = self.expanded.contains(&id); + let last = burst[burst.len() - 1]; + let changes: Vec = burst + .iter() + .flat_map(|l| l.entry.changes.iter().cloned()) + .collect(); + let summary = Entry { + changes, + ..head.entry.clone() + }; + let toggle = panel.clone(); + list = list.child( + h_flex() + .id(ElementId::NamedInteger("history-burst".into(), id as u64)) + .w_full() + .gap_1() + .px_2() + .py_1() + .border_b_1() + .border_color(border) + .hover(|row| row.bg(hover_bg)) + .cursor_pointer() + .on_click(move |_, _, cx| { + toggle + .update(cx, |this, cx| { + if !this.expanded.remove(&id) { + this.expanded.insert(id); + } + cx.notify(); + }) + .ok(); + }) + .child( + gpui_component::Icon::new(if open { + IconName::ChevronDown + } else { + IconName::ChevronRight + }) + .xsmall() + .text_color(muted), + ) + .child( + v_flex() + .min_w_0() + .gap_0p5() + .child(div().text_sm().overflow_hidden().text_ellipsis().child( + format!("{} edits · {}", burst.len(), describe(&summary, &rows)), + )) + .child( + div().text_xs().text_color(muted).child(meta( + &head.entry, + format!("{}–{}", last.time, head.time), + )), + ), + ), + ); + if open { + for listed in burst { + list = list.child(entry_row( + row_id(listed), + &listed.entry, + Some(listed), + meta(&listed.entry, listed.time.clone()), + true, + )); + } + } + } + if self.more && !named_only { + list = list.child( + div().p_2().child( + Button::new("history-older") + .ghost() + .small() + .label("Load Older Changes") + .on_click(cx.listener(|this, _, _, cx| this.load_older(cx))), + ), + ); + } + + v_flex() + .size_full() + .track_focus(&self.focus_handle) + .id("history-panel") + .role(Role::Group) + .aria_label("History") + .child(list) + } +} + +#[cfg(test)] +mod tests { + // Never `use super::*` here: the parent has `use gpui::*` in scope. + use super::{day_label, describe}; + use settings::history::{Change, Entry, Origin}; + use std::collections::HashMap; + + #[test] + fn days_read_as_a_person_would_say_them() { + assert_eq!(day_label("2026-09-13", 0), "Today"); + assert_eq!(day_label("2026-09-12", 1), "Yesterday"); + assert_eq!(day_label("2026-09-03", 10), "3 Sep 2026"); + } + + /// A row is named by the number the grid shows for it now, not by where it was when edited. + #[test] + fn a_cell_edit_names_its_row_as_the_grid_numbers_it_now() { + let cell = |row, before: &str, after: &str| Change::Cell { + row, + column: "Title".into(), + before: before.into(), + after: after.into(), + }; + let rows = HashMap::from([(7, 2)]); + let one = Entry::new(Origin::Typed, vec![cell(7, "", "Harbour")], None); + assert_eq!(describe(&one, &rows), "Title, row 3: (empty) → “Harbour”"); + let gone = Entry::new(Origin::Typed, vec![cell(9, "a", "b")], None); + assert_eq!(describe(&gone, &rows), "Title, a deleted row: “a” → “b”"); + let many = Entry::new( + Origin::Paste, + vec![cell(7, "a", "b"), cell(9, "c", "d")], + None, + ); + assert_eq!(describe(&many, &rows), "2 cells in Title"); + } +} diff --git a/crates/workspace/src/panels/mod.rs b/crates/workspace/src/panels/mod.rs index 0ecb5992..a169f8b6 100644 --- a/crates/workspace/src/panels/mod.rs +++ b/crates/workspace/src/panels/mod.rs @@ -4,9 +4,11 @@ pub mod agent; pub mod details; +pub mod history; pub use agent::{ AGENT_FONT_KEY, AGENT_FONT_SIZE_KEY, AGENT_FONT_SIZES, AGENT_META, AgentCall, AgentPanel, Entry as AgentEntry, record, }; pub use details::{DETAILS_META, DetailsPanel}; +pub use history::{HISTORY_META, HistoryPanel}; From 2800df69a47e1c784e79cd6fe324a2fc1007ff29 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 13 Sep 2026 14:14:27 -0700 Subject: [PATCH 04/16] feat(history): show a row's and a cell's history Details gains a collapsible History section listing the selected item's changes, old value beside new, with a button to restore a saved value. The cell menu's "Show edit history" brings the History panel forward narrowed to that cell, following its column back through renames. The author setting now says it signs changes as well as notes. Refs #132 --- crates/app/src/app_settings/mod.rs | 11 +- crates/settings/src/history.rs | 119 +++++++++-- crates/table/src/note.rs | 13 ++ crates/workspace/src/lib.rs | 12 ++ crates/workspace/src/panels/details.rs | 268 +++++++++++++++++++++++++ crates/workspace/src/panels/history.rs | 168 +++++++++++++++- 6 files changed, 565 insertions(+), 26 deletions(-) diff --git a/crates/app/src/app_settings/mod.rs b/crates/app/src/app_settings/mod.rs index c815af7c..e6a64b1e 100644 --- a/crates/app/src/app_settings/mod.rs +++ b/crates/app/src/app_settings/mod.rs @@ -153,15 +153,16 @@ fn divided_group(cx: &App) -> SettingGroup { .border_color(cx.theme().border) } -/// Who a filed note is attributed to. Lives beside the table's own preferences because a note is -/// filed from the grid, and the name signing it is the archivist's, not the project's. +/// Who a filed note and a recorded change are attributed to. Lives beside the table's own +/// preferences because both are made from the grid, and the name signing them is the archivist's. fn notes_group(cx: &App) -> SettingGroup { divided_group(cx).title("Notes").item( Setting::Text { key: settings::NOTE_AUTHOR_KEY, - label: "Attribute notes to", - description: "Initials or a name, recorded on each note you file. Leave it empty to \ - file notes with a date and no author.", + label: "Attribute notes and changes to", + description: "Initials or a name, recorded on each note you file and each change in \ + the project history. Leave it empty to record them with a date and no \ + author.", } .into_item(cx), ) diff --git a/crates/settings/src/history.rs b/crates/settings/src/history.rs index d7452586..67f360b6 100644 --- a/crates/settings/src/history.rs +++ b/crates/settings/src/history.rs @@ -223,6 +223,15 @@ impl Entry { } } +/// Asks the History panel to show one cell's changes and come to the front — set by the grid's +/// "Show Edit History", which cannot reach the panel itself. +pub struct ShowCellHistory { + pub row: RowId, + pub column: String, +} + +impl gpui::Global for ShowCellHistory {} + /// Who the archivist has told qrate they are, shared with notes. `None` when unset — the log /// stays unsigned rather than guessing from the OS account. pub fn author(cx: &App) -> Option { @@ -296,18 +305,40 @@ pub struct Listed { /// Every entry newer than `after`, oldest first. A file that has never logged anything reads as /// an empty history. pub fn entries_after(path: &Path, after: EntryId) -> Result> { - Ok(select(path, "id > ?1 ORDER BY id", after, -1)? + Ok(select(path, "id > ?1 ORDER BY id", &[&after])? .into_iter() .map(|listed| listed.entry) .collect()) } -/// Up to `limit` entries older than `before`, newest first — one page of the History panel. -pub fn page(path: &Path, before: EntryId, limit: i64) -> Result> { - select(path, "id < ?1 ORDER BY id DESC", before, limit) +/// Up to `limit` entries older than `before`, newest first — one page of the History panel. With +/// `row`, only the entries that changed that row or a note on it. +pub fn page(path: &Path, before: EntryId, limit: i64, row: Option) -> Result> { + select( + path, + "id < ?1 AND (?3 IS NULL OR id IN (SELECT entry_id FROM __history_changes WHERE row_id = ?3)) + ORDER BY id DESC LIMIT ?2", + &[&before, &limit, &row], + ) +} + +/// Every name `column` has gone by, newest first and starting with its own — so a cell's history +/// still finds the edits made before its column was renamed. `renames` is `(before, after)` pairs, +/// newest first. +pub fn former_names<'a>( + column: &str, + renames: impl IntoIterator, +) -> Vec { + let mut names = vec![column.to_string()]; + for (before, after) in renames { + if names.last().is_some_and(|name| name == after) { + names.push(before.to_string()); + } + } + names } -fn select(path: &Path, filter: &str, bound: EntryId, limit: i64) -> Result> { +fn select(path: &Path, filter: &str, bound: &[&dyn rusqlite::ToSql]) -> Result> { let conn = crate::project::open_ro(path)?; let exists: i64 = conn.query_row( "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = '__history'", @@ -322,9 +353,9 @@ fn select(path: &Path, filter: &str, bound: EntryId, limit: i64) -> Result(0)?, @@ -368,6 +399,29 @@ fn select(path: &Path, filter: &str, bound: EntryId, limit: i64) -> Result Result> { + let conn = crate::project::open_ro(path)?; + let exists: i64 = conn.query_row( + "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = '__history_changes'", + [], + |r| r.get(0), + )?; + if exists == 0 { + return Ok(Vec::new()); + } + conn.prepare( + "SELECT change FROM __history_changes WHERE row_id IS NULL AND change LIKE '{\"ColumnRenamed\"%' + ORDER BY entry_id DESC, seq DESC", + )? + .query_map([], |r| r.get::<_, String>(0))? + .filter_map(|change| match serde_json::from_str(&change.ok()?) { + Ok(Change::ColumnRenamed { before, after }) => Some(Ok((before, after))), + _ => None, + }) + .collect() +} + /// Name an entry — "Before Islandora ingest" — or, with `None`, take the name away again. pub fn set_label(path: &Path, id: EntryId, label: Option<&str>) -> Result<()> { crate::project::open_rw(path)? @@ -392,7 +446,10 @@ pub fn clear(path: &Path) -> Result<()> { #[cfg(test)] mod tests { - use super::{Change, Entry, EntryId, Origin, clear, entries_after, page, set_label}; + use super::{ + Change, Entry, EntryId, Origin, clear, entries_after, former_names, page, renames, + set_label, + }; use crate::project::{ProjectSpec, create_project_file, save_dataset, write_notes}; fn project(name: &str) -> std::path::PathBuf { @@ -487,13 +544,13 @@ mod tests { .collect(); save_dataset(&path, &headers, &[1], &[vec!["5".into()]], &entries).unwrap(); - let first = page(&path, EntryId::MAX, 2).unwrap(); + let first = page(&path, EntryId::MAX, 2, None).unwrap(); assert_eq!( first.iter().map(|l| l.entry.id).collect::>(), vec![5, 4] ); assert_eq!(first[0].days_ago, 0, "written just now"); - let older = page(&path, 4, 10).unwrap(); + let older = page(&path, 4, 10, None).unwrap(); assert_eq!( older.iter().map(|l| l.entry.id).collect::>(), vec![3, 2, 1] @@ -501,18 +558,54 @@ mod tests { set_label(&path, 2, Some("Before ingest")).unwrap(); assert_eq!( - page(&path, 3, 1).unwrap()[0].label.as_deref(), + page(&path, 3, 1, None).unwrap()[0].label.as_deref(), Some("Before ingest") ); set_label(&path, 2, None).unwrap(); - assert_eq!(page(&path, 3, 1).unwrap()[0].label, None); + assert_eq!(page(&path, 3, 1, None).unwrap()[0].label, None); clear(&path).unwrap(); - assert!(page(&path, EntryId::MAX, 10).unwrap().is_empty()); + assert!(page(&path, EntryId::MAX, 10, None).unwrap().is_empty()); let data = crate::project::load_project_file(&path).unwrap(); assert_eq!(data.rows, vec![vec!["5".to_string()]]); } + /// A row's history is the entries that touched it, and a cell's follows its column back + /// through every rename. + #[test] + fn a_row_page_and_a_renamed_column_find_their_older_edits() { + let path = project("row.qrate"); + let headers = vec!["Title".to_string()]; + let other = Change::Cell { + row: 2, + column: "Name".into(), + before: String::new(), + after: "x".into(), + }; + let entries = vec![ + Entry::new(Origin::Typed, vec![edit("a", "b")], None), + Entry::new(Origin::Typed, vec![other], None), + Entry::new( + Origin::Structure, + vec![Change::ColumnRenamed { + before: "Name".into(), + after: "Title".into(), + }], + None, + ), + ]; + save_dataset(&path, &headers, &[1], &[vec!["b".into()]], &entries).unwrap(); + + let row = page(&path, EntryId::MAX, 10, Some(1)).unwrap(); + assert_eq!(row.iter().map(|l| l.entry.id).collect::>(), vec![1]); + + let renamed = renames(&path).unwrap(); + assert_eq!(renamed, vec![("Name".to_string(), "Title".to_string())]); + let pairs = renamed.iter().map(|(b, a)| (b.as_str(), a.as_str())); + assert_eq!(former_names("Title", pairs), vec!["Title", "Name"]); + assert_eq!(former_names("Other", [("Name", "Title")]), vec!["Other"]); + } + #[test] fn an_inverse_undoes_and_its_own_inverse_is_the_original() { let moved = Change::ColumnMoved { diff --git a/crates/table/src/note.rs b/crates/table/src/note.rs index b497d6c7..beda2b74 100644 --- a/crates/table/src/note.rs +++ b/crates/table/src/note.rs @@ -421,6 +421,19 @@ pub fn menu( }); }), ) + .when_some( + location.row_id.zip(location.column.clone()), + |menu, (row, column)| { + menu.item(PopupMenuItem::new("Show edit history").on_click( + move |_, _, cx| { + cx.set_global(settings::history::ShowCellHistory { + row, + column: column.to_string(), + }) + }, + )) + }, + ) .separator() } Target::Row(row) => { diff --git a/crates/workspace/src/lib.rs b/crates/workspace/src/lib.rs index 09c1cb63..1f7e1d1a 100644 --- a/crates/workspace/src/lib.rs +++ b/crates/workspace/src/lib.rs @@ -87,6 +87,8 @@ pub struct Workspace { _layout_sub: Subscription, /// Re-renders to mount/unmount the image viewer overlay when it opens or closes. _viewer_sub: Subscription, + /// Brings the History panel forward when the grid asks it to show a cell's changes. + _history_sub: Subscription, } impl Workspace { @@ -186,11 +188,21 @@ impl Workspace { ); let _viewer_sub = cx.observe_global::(|_this, cx| cx.notify()); + let _history_sub = cx.observe_global_in::( + window, + |this, window, cx| { + let name = crate::panels::HISTORY_META.name; + if !PanelRegistry::visible(name, &this.dock_area, cx) { + PanelRegistry::toggle(name, &this.dock_area, window, cx); + } + }, + ); Self { dock_area, _layout_sub, _viewer_sub, + _history_sub, } } diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index b724fac0..a4dfbd66 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -18,6 +18,7 @@ use gpui_component::{ v_flex, }; use preview::{can_preview, thumb}; +use settings::history::{Change, EntryId, Listed, Origin}; use table::{QrateTableDelegate, TableChanged, TableStateHandle}; use crate::BottomDockCrop; @@ -30,6 +31,9 @@ const IMAGE_PANE_HEIGHT_KEY: &str = "details_image_height"; /// repeatedly replace `CurrentProject` and wake every observer of it. const IMAGE_PANE_PERSIST_DEBOUNCE: Duration = Duration::from_millis(200); +/// How many of an item's most recent changes its History section lists. +const ROW_HISTORY_LIMIT: i64 = 50; + /// Height of the Notes sub-panel's header bar, which is the whole of it while collapsed. const NOTES_HEADER_H: f32 = 28.; @@ -80,6 +84,11 @@ pub struct DetailsPanel { /// Whether the Notes sub-panel is expanded. Collapsed, it leaves the split and becomes a /// header strip along the bottom of the panel — the chevron there is what opens it again. notes_open: bool, + /// Whether the History sub-panel is expanded, the same way `notes_open` works. + history_open: bool, + /// The front item's saved changes, with the row and file stamp they were read at — `retarget` + /// runs on every table change, and a query per keystroke would be paid for nothing. + row_history: Option<(settings::project::RowId, std::time::SystemTime, Vec)>, /// Commits the open field on Enter or when the editor loses focus. _editor_sub: Subscription, /// Pending image-pane height write. Replacing the task cancels its timer, coalescing an entire @@ -138,6 +147,8 @@ impl DetailsPanel { stack: 0, stack_hover: false, notes_open: true, + history_open: false, + row_history: None, _editor_sub, _image_height_task: None, anchor: Rc::default(), @@ -211,6 +222,7 @@ impl DetailsPanel { /// same file — this runs on every table change, and rebuilding would re-probe the file and /// throw away the position on every keystroke in the grid. fn retarget(&mut self, cx: &mut Context) { + self.load_row_history(cx); let path = self.selected_file(cx); if self.file == path { return; @@ -225,6 +237,37 @@ impl DetailsPanel { self.transport = path.and_then(|path| Transport::new(path, cx)); } + /// Re-read the front item's history when the item or the project file has changed since. + fn load_row_history(&mut self, cx: &mut Context) { + let row = self.front(cx).and_then(|row| { + let state = self.state.as_ref()?.upgrade()?; + state.read(cx).delegate().row_ids().get(row).copied() + }); + let file = cx + .try_global::() + .map(|p| p.file.clone()); + let (Some(row), Some(file)) = (row, file) else { + self.row_history = None; + return; + }; + let stamp = std::fs::metadata(&file) + .and_then(|m| m.modified()) + .unwrap_or(std::time::UNIX_EPOCH); + if self + .row_history + .as_ref() + .is_some_and(|(r, s, _)| *r == row && *s == stamp) + { + return; + } + let listed = settings::history::page(&file, EntryId::MAX, ROW_HISTORY_LIMIT, Some(row)) + .unwrap_or_else(|err| { + log::error!("couldn't read the selected row's history: {err}"); + Vec::new() + }); + self.row_history = Some((row, stamp, listed)); + } + /// Open `header`'s field for editing, seeded with its current text. The column is resolved by /// name — `row_fields` is in display order, which is not the data-column order after a move. fn edit_field( @@ -406,6 +449,218 @@ impl DetailsPanel { .into_any_element() } + /// The History sub-panel: what has happened to the front item, newest first — each field's old + /// value beside the new, with a way to put a saved one back. Unsaved changes lead, without that + /// button: there is no saved entry yet for a restore to name. + /// + /// Returns `AnyElement` for the same reason `notes_panel` does. + fn history_panel(&self, cx: &mut Context) -> AnyElement { + let Some((row_id, _, saved)) = self.row_history.as_ref() else { + return div().into_any_element(); + }; + let row_id = *row_id; + let unsaved: Vec<_> = self + .state + .as_ref() + .and_then(|w| w.upgrade()) + .map(|state| state.read(cx).delegate().unsaved_history().to_vec()) + .unwrap_or_default(); + let theme = cx.theme(); + let (muted, border, background, radius) = ( + theme.muted_foreground, + theme.border, + theme.background, + theme.radius, + ); + + // One line per change to this item: `(entry id if saved, what, meta, value to restore)`. + type Line = ( + Option, + String, + String, + Option<(String, SharedString)>, + ); + let quote = |text: &str| match text.is_empty() { + true => "(empty)".to_string(), + false => format!("“{text}”"), + }; + let lines_of = |entry: &settings::history::Entry, id: Option, when: String| { + let meta = [Some(entry.origin.label()), entry.author.clone(), Some(when)] + .into_iter() + .flatten() + .collect::>() + .join(" · "); + let dimmed = matches!(entry.origin, Origin::Undo | Origin::Redo); + entry + .changes + .iter() + .rev() + .filter_map(|change| { + let (what, restore) = match change { + Change::Cell { + row, + column, + before, + after, + } if *row == row_id => ( + format!("{column}: {} → {}", quote(before), quote(after)), + Some((column.clone(), SharedString::from(before.clone()))), + ), + Change::Note { + row: Some(row), + column, + before, + after, + } if *row == row_id => { + let verb = match (before, after) { + (None, _) => "added", + (_, None) => "removed", + _ => "edited", + }; + let on = column + .as_deref() + .map_or(String::new(), |c| format!(" on {c}")); + (format!("Note{on} {verb}"), None) + } + Change::RowAdded { row, .. } if *row == row_id => { + ("Item added".into(), None) + } + Change::RowRemoved { row, .. } if *row == row_id => { + ("Item deleted".into(), None) + } + _ => return None, + }; + let restore = restore.filter(|_| id.is_some() && !dimmed); + Some((id, what, meta.clone(), restore)) + }) + .collect::>() + }; + let lines: Vec = unsaved + .iter() + .rev() + .flat_map(|entry| lines_of(entry, None, "not saved yet".into())) + .chain(saved.iter().flat_map(|listed| { + let when = format!( + "{} {}", + super::history::day_label(&listed.day, listed.days_ago), + listed.time + ); + lines_of(&listed.entry, Some(listed.entry.id), when) + })) + .collect(); + + let (open, none) = (self.history_open, lines.is_empty()); + v_flex() + .size_full() + .min_h_0() + .border_t_1() + .border_color(border) + .child( + h_flex() + .flex_none() + .h(px(NOTES_HEADER_H)) + .items_center() + .gap_1p5() + .px_2() + .py_1() + .bg(background) + .child( + Button::new("details-history-toggle") + .icon(match open { + true => IconName::ChevronDown, + false => IconName::ChevronRight, + }) + .ghost() + .xsmall() + .tooltip(match open { + true => "Collapse history", + false => "Expand history", + }) + .on_click(cx.listener(|this, _, _, cx| { + this.history_open = !this.history_open; + cx.notify(); + })), + ) + .child(div().text_xs().font_semibold().child("History")) + .child(div().text_xs().text_color(muted).child(match lines.len() { + 0 => "none".to_string(), + n if n as i64 >= ROW_HISTORY_LIMIT => format!("{n}+"), + n => n.to_string(), + })), + ) + .when(open, |section| { + section.child( + div() + .flex_1() + .min_h_0() + .overflow_y_scrollbar() + .px_3() + .pb_2() + .child( + v_flex() + .gap_1() + .children(lines.into_iter().enumerate().map( + |(ix, (id, what, meta, restore))| { + h_flex() + .gap_1() + .items_start() + .p_1p5() + .rounded(radius) + .border_1() + .border_color(border) + .child( + v_flex() + .flex_1() + .min_w_0() + .gap_0p5() + .child(div().text_xs().child(what)) + .child( + div() + .text_xs() + .text_color(muted) + .child(meta), + ), + ) + .when_some( + id.zip(restore), + |line, (id, (column, before))| { + line.child( + Button::new(( + "details-history-restore", + ix, + )) + .icon(IconName::Undo2) + .ghost() + .xsmall() + .tooltip("Restore this value") + .on_click(move |_, _, cx| { + table::restore_value( + row_id, + &column, + before.clone(), + id, + cx, + ) + }), + ) + }, + ) + }, + )) + .when(none, |list| { + list.child( + div() + .text_xs() + .text_color(muted) + .child("No changes to this item yet."), + ) + }), + ), + ) + }) + .into_any_element() + } + /// Move the preview stack one item along, wrapping at both ends so a bundle can be walked in /// either direction without hunting for the end of it. fn step_stack(&mut self, forward: bool, cx: &mut Context) { @@ -796,6 +1051,11 @@ impl Render for DetailsPanel { // Built before the field rows below, which borrow `cx` for as long as they stay a lazy // iterator — this needs `&mut cx` and cannot wait for them. let notes = (gallery && count > 0).then(|| self.notes_panel(&picked, cx)); + let history = (count == 1).then(|| self.history_panel(cx)); + let history_range = match self.history_open { + true => px(80.)..px(360.), + false => px(NOTES_HEADER_H)..px(NOTES_HEADER_H), + }; // Collapsing pins the panel's size range to its header instead of taking it out of the // split. Removing it re-syncs the group — every panel's size is rescaled to the container // when the count changes — so the fields jumped on the way out and the notes came back at @@ -1149,6 +1409,14 @@ impl Render for DetailsPanel { .size_range(notes_range) .child(notes), ) + }) + .when_some(history, |split, history| { + split.child( + resizable_panel() + .size(px(160.)) + .size_range(history_range) + .child(history), + ) }), ), ) diff --git a/crates/workspace/src/panels/history.rs b/crates/workspace/src/panels/history.rs index de9ff23c..859ecacc 100644 --- a/crates/workspace/src/panels/history.rs +++ b/crates/workspace/src/panels/history.rs @@ -1,3 +1,4 @@ +use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::path::PathBuf; use std::time::{SystemTime, UNIX_EPOCH}; @@ -13,7 +14,7 @@ use gpui_component::{ button::{Button, ButtonVariants as _}, h_flex, v_flex, }; -use settings::history::{Change, Entry, EntryId, Listed, Origin}; +use settings::history::{Change, Entry, EntryId, Listed, Origin, ShowCellHistory}; use settings::project::{CurrentProject, RowId}; use table::{TableChanged, TableStateHandle}; @@ -94,6 +95,9 @@ pub struct HistoryPanel { expanded: HashSet, /// The entry being named, and the box the name is typed in. naming: Option<(EntryId, Entity, Subscription)>, + /// One cell to show the changes of, as its row and every name its column has had. + cell: Option<(RowId, Vec)>, + _cell_sub: Subscription, _handle_sub: Subscription, _table_sub: Option, _notes_sub: Subscription, @@ -112,6 +116,10 @@ impl HistoryPanel { named_only: false, expanded: HashSet::new(), naming: None, + cell: None, + _cell_sub: cx.observe_global::(|this: &mut Self, cx| { + this.show_cell(cx); + }), _handle_sub: cx.observe_global::(|this: &mut Self, cx| { this.bind(cx); cx.notify(); @@ -156,7 +164,8 @@ impl HistoryPanel { return; } let limit = PAGE.max(self.saved.len() as i64); - match settings::history::page(&file, EntryId::MAX, limit) { + let row = self.cell.as_ref().map(|(row, _)| *row); + match settings::history::page(&file, EntryId::MAX, limit, row) { Ok(page) => { self.more = page.len() as i64 == limit; self.saved = page; @@ -167,6 +176,53 @@ impl HistoryPanel { cx.notify(); } + /// Narrow the list to the cell the grid asked about. Its column's former names come from both + /// halves of the log, unsaved renames being the newest. + fn show_cell(&mut self, cx: &mut Context) { + let (Some(request), Some(file)) = ( + cx.try_global::(), + cx.try_global::().map(|p| p.file.clone()), + ) else { + return; + }; + let unsaved: Vec<(String, String)> = cx + .try_global::() + .and_then(|h| h.0.upgrade()) + .map(|state| { + state + .read(cx) + .delegate() + .unsaved_history() + .iter() + .rev() + .flat_map(|e| e.changes.iter().rev()) + .filter_map(|c| match c { + Change::ColumnRenamed { before, after } => { + Some((before.clone(), after.clone())) + } + _ => None, + }) + .collect() + }) + .unwrap_or_default(); + let saved = settings::history::renames(&file).unwrap_or_else(|err| { + log::error!("couldn't read column renames from the project history: {err}"); + Vec::new() + }); + let names = settings::history::former_names( + &request.column, + unsaved + .iter() + .chain(&saved) + .map(|(b, a)| (b.as_str(), a.as_str())), + ); + self.cell = Some((request.row, names)); + self.filter = Filter::All; + self.named_only = false; + self.saved.clear(); + self.reload(true, cx); + } + fn load_older(&mut self, cx: &mut Context) { let (Some(file), Some(last)) = ( cx.try_global::().map(|p| p.file.clone()), @@ -174,7 +230,8 @@ impl HistoryPanel { ) else { return; }; - match settings::history::page(&file, last, PAGE) { + let row = self.cell.as_ref().map(|(row, _)| *row); + match settings::history::page(&file, last, PAGE, row) { Ok(page) => { self.more = page.len() as i64 == PAGE; self.saved.extend(page); @@ -293,8 +350,36 @@ impl HistoryPanel { } } +/// `entry` cut down to what it did to one cell — its value, its notes, its row coming or going — or +/// `None` when it did nothing there. Borrowed untouched when there is no cell to narrow to. +fn narrowed<'a>(entry: &'a Entry, cell: Option<&(RowId, Vec)>) -> Option> { + let Some((row, names)) = cell else { + return Some(Cow::Borrowed(entry)); + }; + let named = |column: &str| names.iter().any(|name| name == column); + let changes: Vec = entry + .changes + .iter() + .filter(|change| match change { + Change::Cell { row: r, column, .. } => r == row && named(column), + Change::Note { row: r, column, .. } => { + *r == Some(*row) && column.as_deref().is_none_or(named) + } + Change::RowAdded { row: r, .. } | Change::RowRemoved { row: r, .. } => r == row, + _ => false, + }) + .cloned() + .collect(); + (!changes.is_empty()).then(|| { + Cow::Owned(Entry { + changes, + ..entry.clone() + }) + }) +} + /// `Today`, `Yesterday`, or `3 Sep 2026`. -fn day_label(day: &str, days_ago: i64) -> String { +pub(crate) fn day_label(day: &str, days_ago: i64) -> String { const MONTHS: [&str; 12] = [ "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec", ]; @@ -537,20 +622,32 @@ impl Render for HistoryPanel { } }; + let cell = self.cell.as_ref(); let unsaved: Vec = unsaved - .into_iter() + .iter() .rev() + .filter_map(|e| narrowed(e, cell).map(Cow::into_owned)) .filter(|e| admits(e, false)) .collect(); - let saved: Vec<&Listed> = self + let saved: Vec> = self .saved .iter() + .filter_map(|l| match narrowed(&l.entry, cell)? { + Cow::Borrowed(_) => Some(Cow::Borrowed(l)), + Cow::Owned(entry) => Some(Cow::Owned(Listed { entry, ..l.clone() })), + }) .filter(|l| admits(&l.entry, l.label.is_some())) .collect(); + let cell_label = cell.map(|(row, names)| { + let at = rows + .get(row) + .map_or_else(|| "a deleted row".to_string(), |p| format!("row {}", p + 1)); + format!("{}, {at}", names[0]) + }); // Bursts: runs of adjacent, unnamed entries by one person, one means, one day. let mut bursts: Vec> = Vec::new(); - for listed in saved { + for listed in saved.iter().map(|l| &**l) { let joins = bursts.last().and_then(|b| b.last()).is_some_and(|prev| { prev.label.is_none() && listed.label.is_none() @@ -818,6 +915,39 @@ impl Render for HistoryPanel { .id("history-panel") .role(Role::Group) .aria_label("History") + .when_some(cell_label, |panel, label| { + panel.child( + h_flex() + .flex_none() + .gap_1() + .items_center() + .px_2() + .py_1() + .border_b_1() + .border_color(border) + .child( + div() + .flex_1() + .min_w_0() + .text_xs() + .overflow_hidden() + .text_ellipsis() + .child(format!("Changes to {label}")), + ) + .child( + Button::new("history-cell-clear") + .icon(IconName::Close) + .ghost() + .xsmall() + .tooltip("Show every change") + .on_click(cx.listener(|this, _, _, cx| { + this.cell = None; + this.saved.clear(); + this.reload(true, cx); + })), + ), + ) + }) .child(list) } } @@ -825,10 +955,32 @@ impl Render for HistoryPanel { #[cfg(test)] mod tests { // Never `use super::*` here: the parent has `use gpui::*` in scope. - use super::{day_label, describe}; + use super::{day_label, describe, narrowed}; use settings::history::{Change, Entry, Origin}; use std::collections::HashMap; + /// Narrowed to one cell, an entry keeps that cell's edits under any name its column has had, + /// its row's notes and its row coming or going — and nothing about the cells beside it. + #[test] + fn a_cell_keeps_its_own_changes_under_any_former_name() { + let cell = |row, column: &str| Change::Cell { + row, + column: column.into(), + before: "a".into(), + after: "b".into(), + }; + let entry = Entry::new( + Origin::Paste, + vec![cell(1, "Name"), cell(1, "Date"), cell(2, "Name")], + None, + ); + let names = (1, vec!["Title".to_string(), "Name".to_string()]); + let kept = narrowed(&entry, Some(&names)).unwrap(); + assert_eq!(kept.changes, vec![cell(1, "Name")]); + let elsewhere = Entry::new(Origin::Typed, vec![cell(2, "Date")], None); + assert!(narrowed(&elsewhere, Some(&names)).is_none()); + } + #[test] fn days_read_as_a_person_would_say_them() { assert_eq!(day_label("2026-09-13", 0), "Today"); From eaa5bdd72799b6e07b55af6f74071cef1774cb68 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Mon, 14 Sep 2026 15:16:15 -0700 Subject: [PATCH 05/16] feat(history): show dates, a clock icon, and full-text tooltips Every change in the History panel and the Details section now reads with its date as well as its time, unsaved ones included. Truncated summaries show their full text on hover, like the Problems panel. The panel's status-bar icon is Lucide's history clock, shipped as an app asset, so PanelMeta now names an icon by asset path. Refs #132 --- assets/icons/history.svg | 1 + crates/app/src/assets.rs | 12 ++- crates/settings/src/history.rs | 21 ++++- crates/workspace/src/dock_button.rs | 15 ++-- crates/workspace/src/panel_registry.rs | 14 ++-- crates/workspace/src/panels/agent.rs | 2 +- crates/workspace/src/panels/details.rs | 22 ++++-- crates/workspace/src/panels/history.rs | 105 +++++++++++++++---------- 8 files changed, 123 insertions(+), 69 deletions(-) create mode 100644 assets/icons/history.svg diff --git a/assets/icons/history.svg b/assets/icons/history.svg new file mode 100644 index 00000000..937d293e --- /dev/null +++ b/assets/icons/history.svg @@ -0,0 +1 @@ + diff --git a/crates/app/src/assets.rs b/crates/app/src/assets.rs index a626bc69..15887078 100644 --- a/crates/app/src/assets.rs +++ b/crates/app/src/assets.rs @@ -8,7 +8,11 @@ use gpui::{AssetSource, Result, SharedString}; pub struct Assets; -const OWN: [(&str, &str); 3] = [ +const OWN: [(&str, &str); 4] = [ + ( + "icons/history.svg", + include_str!("../../../assets/icons/history.svg"), + ), ( "icons/panel-left-filled.svg", include_str!("../../../assets/icons/panel-left-filled.svg"), @@ -55,6 +59,12 @@ mod tests { .unwrap() .is_some_and(|svg| svg.starts_with(b" Result> { .collect()) } +/// Each of `ats` (unix seconds) as a local `(YYYY-MM-DD, HH:MM)` — the same clock [`page`] reads +/// saved entries by, for changes that have not reached a file yet. +pub fn local_times(ats: &[i64]) -> Result> { + let conn = Connection::open_in_memory()?; + let mut stmt = conn.prepare( + "SELECT date(?1, 'unixepoch', 'localtime'), strftime('%H:%M', ?1, 'unixepoch', 'localtime')", + )?; + ats.iter() + .map(|at| Ok(stmt.query_row([at], |r| Ok((r.get(0)?, r.get(1)?)))?)) + .collect() +} + /// Up to `limit` entries older than `before`, newest first — one page of the History panel. With /// `row`, only the entries that changed that row or a note on it. pub fn page(path: &Path, before: EntryId, limit: i64, row: Option) -> Result> { @@ -447,8 +459,8 @@ pub fn clear(path: &Path) -> Result<()> { #[cfg(test)] mod tests { use super::{ - Change, Entry, EntryId, Origin, clear, entries_after, former_names, page, renames, - set_label, + Change, Entry, EntryId, Origin, clear, entries_after, former_names, local_times, page, + renames, set_label, }; use crate::project::{ProjectSpec, create_project_file, save_dataset, write_notes}; @@ -545,6 +557,11 @@ mod tests { save_dataset(&path, &headers, &[1], &[vec!["5".into()]], &entries).unwrap(); let first = page(&path, EntryId::MAX, 2, None).unwrap(); + let (day, time) = local_times(&[first[0].entry.at]).unwrap().remove(0); + assert_eq!( + (day.as_str(), time.as_str()), + (first[0].day.as_str(), first[0].time.as_str()) + ); assert_eq!( first.iter().map(|l| l.entry.id).collect::>(), vec![5, 4] diff --git a/crates/workspace/src/dock_button.rs b/crates/workspace/src/dock_button.rs index a4359b6a..eccede87 100644 --- a/crates/workspace/src/dock_button.rs +++ b/crates/workspace/src/dock_button.rs @@ -30,7 +30,8 @@ pub struct DockToggleButton { id: SharedString, dock: WeakEntity, toggles: Toggles, - icon: IconName, + /// Asset path of the glyph. + icon: SharedString, /// Asset path drawn in place of `icon` while the dock is open — Lucide has no filled panel. open_icon: Option, /// Whether to show the live error/warning count beside the icon. @@ -49,15 +50,15 @@ impl DockToggleButton { placement: DockPlacement, ) -> Self { let (icon, open_icon) = match placement { - DockPlacement::Left => (IconName::PanelLeft, "icons/panel-left-filled.svg"), - DockPlacement::Right => (IconName::PanelRight, "icons/panel-right-filled.svg"), - _ => (IconName::PanelBottom, "icons/panel-bottom-filled.svg"), + DockPlacement::Left => ("icons/panel-left.svg", "icons/panel-left-filled.svg"), + DockPlacement::Right => ("icons/panel-right.svg", "icons/panel-right-filled.svg"), + _ => ("icons/panel-bottom.svg", "icons/panel-bottom-filled.svg"), }; Self { id: id.into(), dock, toggles: Toggles::Dock(placement), - icon, + icon: icon.into(), open_icon: Some(open_icon.into()), count: false, hint: None, @@ -90,7 +91,7 @@ impl DockToggleButton { id: meta.name.into(), dock, toggles: Toggles::Panel(meta), - icon: meta.icon.clone(), + icon: meta.icon.into(), open_icon: None, count: meta.badge, hint: None, @@ -157,7 +158,7 @@ impl Render for DockToggleButton { if !self.count { let icon = match (&self.open_icon, lit) { (Some(path), true) => Icon::empty().path(path.clone()), - _ => Icon::new(self.icon.clone()), + _ => Icon::empty().path(self.icon.clone()), }; return this.child(icon.small()); } diff --git a/crates/workspace/src/panel_registry.rs b/crates/workspace/src/panel_registry.rs index d1129299..60838223 100644 --- a/crates/workspace/src/panel_registry.rs +++ b/crates/workspace/src/panel_registry.rs @@ -11,12 +11,8 @@ use std::{collections::HashMap, sync::Arc}; use gpui::*; -use gpui_component::{ - IconName, - dock::{ - BasePanelView, DockArea, DockEvent, DockLayout, DockPlacement, InsertTarget, NodeId, - PaneRef, - }, +use gpui_component::dock::{ + BasePanelView, DockArea, DockEvent, DockLayout, DockPlacement, InsertTarget, NodeId, PaneRef, }; use crate::Workspace; @@ -44,7 +40,9 @@ pub fn bar_side(placement: DockPlacement) -> BarSide { pub struct PanelMeta { /// Matches `Panel::panel_name` and the name the panel is registered under for layout restore. pub name: &'static str, - pub icon: IconName, + /// Asset path of the status-bar glyph — a path rather than an `IconName` so a panel can use an + /// icon the library doesn't ship. + pub icon: &'static str, pub label: &'static str, pub default_placement: DockPlacement, /// Show the live error/warning counts beside the icon instead of the icon alone. @@ -55,7 +53,7 @@ pub struct PanelMeta { /// `PanelMeta` — its declaration sits here instead of beside it. pub static PROBLEMS_META: PanelMeta = PanelMeta { name: "ProblemsPanel", - icon: IconName::TriangleAlert, + icon: "icons/triangle-alert.svg", label: "Problems", default_placement: DockPlacement::Bottom, badge: true, diff --git a/crates/workspace/src/panels/agent.rs b/crates/workspace/src/panels/agent.rs index 5532f40b..98c0497a 100644 --- a/crates/workspace/src/panels/agent.rs +++ b/crates/workspace/src/panels/agent.rs @@ -21,7 +21,7 @@ use crate::panel_registry::PanelMeta; /// Where Agent starts out and what it puts in the status bar. pub static AGENT_META: PanelMeta = PanelMeta { name: "AgentPanel", - icon: IconName::Star, + icon: "icons/star.svg", label: "Agent", default_placement: DockPlacement::Right, badge: false, diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index a4dfbd66..fdeb52c2 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -49,7 +49,7 @@ const VALUE_LINE_CLAMP: usize = 4; /// starting point — the user can dock it anywhere, and that choice is what gets persisted. pub static DETAILS_META: PanelMeta = PanelMeta { name: "DetailsPanel", - icon: IconName::Info, + icon: "icons/info.svg", label: "Details", default_placement: DockPlacement::Left, badge: false, @@ -535,16 +535,24 @@ impl DetailsPanel { }) .collect::>() }; + let ats: Vec = unsaved.iter().map(|entry| entry.at).collect(); + let times = settings::history::local_times(&ats).unwrap_or_else(|err| { + log::error!("couldn't read the local time of unsaved changes: {err}"); + Vec::new() + }); let lines: Vec = unsaved .iter() + .enumerate() .rev() - .flat_map(|entry| lines_of(entry, None, "not saved yet".into())) - .chain(saved.iter().flat_map(|listed| { - let when = format!( - "{} {}", - super::history::day_label(&listed.day, listed.days_ago), - listed.time + .flat_map(|(ix, entry)| { + let when = times.get(ix).map_or_else( + || "not saved yet".to_string(), + |(day, time)| format!("{}, not saved yet", super::history::when(day, time)), ); + lines_of(entry, None, when) + }) + .chain(saved.iter().flat_map(|listed| { + let when = super::history::when(&listed.day, &listed.time); lines_of(&listed.entry, Some(listed.entry.id), when) })) .collect(); diff --git a/crates/workspace/src/panels/history.rs b/crates/workspace/src/panels/history.rs index 859ecacc..5c2bf1c7 100644 --- a/crates/workspace/src/panels/history.rs +++ b/crates/workspace/src/panels/history.rs @@ -23,7 +23,7 @@ use crate::panel_registry::PanelMeta; pub static HISTORY_META: PanelMeta = PanelMeta { name: "HistoryPanel", - icon: IconName::Undo2, + icon: "icons/history.svg", label: "History", default_placement: DockPlacement::Right, badge: false, @@ -378,27 +378,38 @@ fn narrowed<'a>(entry: &'a Entry, cell: Option<&(RowId, Vec)>) -> Option }) } -/// `Today`, `Yesterday`, or `3 Sep 2026`. -pub(crate) fn day_label(day: &str, days_ago: i64) -> String { +/// `3 Sep 2026`, from a `YYYY-MM-DD` day. +fn date_label(day: &str) -> String { const MONTHS: [&str; 12] = [ "Jan", "Feb", "Mar", "Apr", "May", "Jun", "Jul", "Aug", "Sep", "Oct", "Nov", "Dec", ]; + let mut parts = day.split('-'); + match (parts.next(), parts.next(), parts.next()) { + (Some(y), Some(m), Some(d)) => { + let month = m + .parse::() + .ok() + .and_then(|m| MONTHS.get(m.checked_sub(1)?)); + format!("{} {} {y}", d.trim_start_matches('0'), month.unwrap_or(&m)) + } + _ => day.into(), + } +} + +/// What a day's header says: `Today`, `Yesterday`, or the date. +fn day_label(day: &str, days_ago: i64) -> String { match days_ago { 0 => "Today".into(), 1 => "Yesterday".into(), - _ => { - let mut parts = day.split('-'); - match (parts.next(), parts.next(), parts.next()) { - (Some(y), Some(m), Some(d)) => { - let month = m.parse::().ok().and_then(|m| MONTHS.get(m - 1)); - format!("{} {} {y}", d.trim_start_matches('0'), month.unwrap_or(&m)) - } - _ => day.into(), - } - } + _ => date_label(day), } } +/// When one change was made: `3 Sep 2026, 14:02`. +pub(crate) fn when(day: &str, time: &str) -> String { + format!("{}, {time}", date_label(day)) +} + /// One line saying what an entry did, in the grid's current terms: rows by the number the `#` /// column shows now, and a row that has since gone as exactly that. fn describe(entry: &Entry, rows: &HashMap) -> String { @@ -610,9 +621,6 @@ impl Render for HistoryPanel { ) }) .unwrap_or_default(); - let now = SystemTime::now() - .duration_since(UNIX_EPOCH) - .map_or(0, |d| d.as_secs() as i64); let (filter, named_only) = (self.filter, self.named_only); let admits = |entry: &Entry, label: bool| { if named_only { @@ -666,6 +674,17 @@ impl Render for HistoryPanel { } } + // ponytail: always offered, since gpui cannot tell us whether the text was cut + let clipped = |text: String| { + div() + .id("text") + .overflow_hidden() + .text_ellipsis() + .child(text.clone()) + .tooltip(move |window, cx| { + gpui_component::tooltip::Tooltip::new(text.clone()).build(window, cx) + }) + }; let entry_row = |id: ElementId, entry: &Entry, listed: Option<&Listed>, @@ -741,13 +760,7 @@ impl Render for HistoryPanel { ) }) .when_some(naming, |row, input| row.child(Input::new(&input).xsmall())) - .child( - div() - .text_sm() - .overflow_hidden() - .text_ellipsis() - .child(describe(entry, &rows)), - ) + .child(clipped(describe(entry, &rows)).text_sm()) .child(div().text_xs().text_color(muted).child(meta)) .map(|row| match actions { Some(actions) => row.context_menu(actions).into_any_element(), @@ -794,16 +807,21 @@ impl Render for HistoryPanel { } if !unsaved.is_empty() { list = list.child(header("Not saved yet".into())); + let ats: Vec = unsaved.iter().map(|e| e.at).collect(); + let times = settings::history::local_times(&ats).unwrap_or_else(|err| { + log::error!("couldn't read the local time of unsaved changes: {err}"); + Vec::new() + }); for (ix, entry) in unsaved.iter().enumerate() { - let ago = match (now - entry.at) / 60 { - 0 => "just now".to_string(), - m => format!("{m} min ago"), - }; + let at = times.get(ix).map_or_else( + || "not saved".to_string(), + |(day, time)| format!("{}, not saved", when(day, time)), + ); list = list.child(entry_row( ElementId::NamedInteger("history-unsaved".into(), ix as u64), entry, None, - meta(entry, ago), + meta(entry, at), false, )); } @@ -824,7 +842,7 @@ impl Render for HistoryPanel { row_id(head), &head.entry, Some(head), - meta(&head.entry, head.time.clone()), + meta(&head.entry, when(&head.day, &head.time)), false, )); continue; @@ -874,15 +892,18 @@ impl Render for HistoryPanel { v_flex() .min_w_0() .gap_0p5() - .child(div().text_sm().overflow_hidden().text_ellipsis().child( - format!("{} edits · {}", burst.len(), describe(&summary, &rows)), - )) .child( - div().text_xs().text_color(muted).child(meta( - &head.entry, - format!("{}–{}", last.time, head.time), - )), - ), + clipped(format!( + "{} edits · {}", + burst.len(), + describe(&summary, &rows) + )) + .text_sm(), + ) + .child(div().text_xs().text_color(muted).child(meta( + &head.entry, + format!("{}, {}–{}", date_label(&head.day), last.time, head.time), + ))), ), ); if open { @@ -891,7 +912,7 @@ impl Render for HistoryPanel { row_id(listed), &listed.entry, Some(listed), - meta(&listed.entry, listed.time.clone()), + meta(&listed.entry, when(&listed.day, &listed.time)), true, )); } @@ -926,13 +947,10 @@ impl Render for HistoryPanel { .border_b_1() .border_color(border) .child( - div() + clipped(format!("Changes to {label}")) .flex_1() .min_w_0() - .text_xs() - .overflow_hidden() - .text_ellipsis() - .child(format!("Changes to {label}")), + .text_xs(), ) .child( Button::new("history-cell-clear") @@ -986,6 +1004,7 @@ mod tests { assert_eq!(day_label("2026-09-13", 0), "Today"); assert_eq!(day_label("2026-09-12", 1), "Yesterday"); assert_eq!(day_label("2026-09-03", 10), "3 Sep 2026"); + assert_eq!(super::when("2026-09-03", "14:02"), "3 Sep 2026, 14:02"); } /// A row is named by the number the grid shows for it now, not by where it was when edited. From 02700848edd7b426632c37a4a371cf22d4033d2e Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sat, 19 Sep 2026 22:26:16 -0700 Subject: [PATCH 06/16] chore(dev): configure zccache reuse --- .cargo/config.toml | 4 ++++ .zed/tasks.json | 5 +++++ 2 files changed, 9 insertions(+) diff --git a/.cargo/config.toml b/.cargo/config.toml index 0eb1c9d6..230e93d2 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -2,3 +2,7 @@ # 16 logical cores, but a full gpui rebuild at -j16 starves the editor and the app. # Leaves headroom so the machine stays usable while cargo works. jobs = 10 + +[env] +# Make Rust artifacts path-independent so zccache can reuse them across worktrees. +ZCCACHE_PATH_REMAP = "auto" diff --git a/.zed/tasks.json b/.zed/tasks.json index d3895146..51764559 100644 --- a/.zed/tasks.json +++ b/.zed/tasks.json @@ -53,6 +53,11 @@ "reveal": "always", "use_new_terminal": true }, + { + "label": "zccache status", + "command": "zccache status", + "reveal": "always" + }, { "label": "lazygit", "command": "lazygit" From 5e8ac788f75fdb27dd95f4e6f8320ad72bc44c7f Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sat, 19 Sep 2026 23:34:15 -0700 Subject: [PATCH 07/16] fix(details): restore notes and history resizing --- Cargo.lock | 1 - crates/workspace/Cargo.toml | 1 - crates/workspace/src/panels/details.rs | 249 ++++++++++++++----------- 3 files changed, 135 insertions(+), 116 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index cc4289b2..29efc64b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10685,7 +10685,6 @@ version = "0.5.0-beta.1" dependencies = [ "agent-runtime", "diagnostics", - "gpui-base", "gpui-component", "gpui-pre", "log", diff --git a/crates/workspace/Cargo.toml b/crates/workspace/Cargo.toml index 0dd0593d..5ed2768e 100644 --- a/crates/workspace/Cargo.toml +++ b/crates/workspace/Cargo.toml @@ -6,7 +6,6 @@ edition.workspace = true [dependencies] log.workspace = true gpui.workspace = true -gpui-base.workspace = true gpui-component.workspace = true schemars.workspace = true serde.workspace = true diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 51ed5a70..87a2ac3d 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -6,7 +6,6 @@ use std::time::Duration; use gpui::prelude::FluentBuilder as _; use gpui::*; -use gpui_base::resize_handle; use gpui_component::{ ActiveTheme, IconName, Sizable, StyledExt as _, button::{Button, ButtonVariants}, @@ -42,21 +41,6 @@ const ROW_HISTORY_LIMIT: i64 = 50; /// Height of the Notes sub-panel's header bar, which is the whole of it while collapsed. const NOTES_HEADER_H: f32 = 28.; -#[derive(Clone, Copy, PartialEq)] -enum DetailsSubPane { - Notes, - History, -} - -#[derive(Clone)] -struct DragDetailsSubPane(DetailsSubPane); - -impl Render for DragDetailsSubPane { - fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { - Empty - } -} - /// How much of the window the open field editor may span when the panel itself is narrower. const EDITOR_MAX_WINDOW_SHARE: f32 = 0.4; @@ -108,7 +92,6 @@ pub struct DetailsPanel { history_open: bool, notes_height: Pixels, history_height: Pixels, - resize_start: Option<(DetailsSubPane, Pixels, Pixels)>, /// The front item's saved changes, with the row and file stamp they were read at — `retarget` /// runs on every table change, and a query per keystroke would be paid for nothing. row_history: Option<(settings::project::RowId, std::time::SystemTime, Vec)>, @@ -173,7 +156,6 @@ impl DetailsPanel { history_open: false, notes_height: px(180.), history_height: px(160.), - resize_start: None, row_history: None, _editor_sub, _image_height_task: None, @@ -596,6 +578,7 @@ impl DetailsPanel { let (open, none) = (self.history_open, lines.is_empty()); v_flex() + .debug_selector(|| "details-history-panel".into()) .size_full() .min_h_0() .border_t_1() @@ -1280,40 +1263,38 @@ impl Render for DetailsPanel { .children(self.field_editor(window, cx)) .into_any_element(); - let panel = cx.entity().downgrade(); - let handle = |id: &'static str, pane: DetailsSubPane, height: Pixels| { - let panel = panel.clone(); - resize_handle(id, Axis::Vertical).on_drag( - DragDetailsSubPane(pane), - move |drag, position, _, cx| { - if let Some(panel) = panel.upgrade() { - panel.update(cx, |this, _| { - this.resize_start = Some((pane, position.y, height)); - }); - } - cx.new(|_| drag.as_ref().clone()) - }, - ) - }; - let fields_and_notes = match notes { - Some(notes) if self.notes_open => v_flex() - .size_full() - .min_h_0() - .child(div().flex_1().min_h(px(80.)).pr_2().child(fields)) - .child( - div() - .relative() - .flex_none() - .h(self.notes_height) - .child(handle( - "details-notes-resize", - DetailsSubPane::Notes, - self.notes_height, - )) - .child(notes), - ) - .into_any_element(), + Some(notes) if self.notes_open => { + let panel = cx.entity().downgrade(); + v_resizable("details-notes-split") + .on_resize(move |state, _, cx| { + let Some(height) = state.read(cx).sizes().get(1).copied() else { + return; + }; + if let Some(panel) = panel.upgrade() { + panel.update(cx, |this, _| this.notes_height = height); + } + if cx + .try_global::() + .is_some() + { + settings::project::CurrentProject::set_text( + NOTES_PANE_HEIGHT_KEY, + format!("{}", f32::from(height)).into(), + cx, + ); + } + }) + .child(resizable_panel().min_h(px(80.)).pr_2().child(fields)) + .child( + resizable_panel() + .size(self.notes_height) + .size_range(px(80.)..px(320.)) + .flex_none() + .child(notes), + ) + .into_any_element() + } Some(notes) => v_flex() .size_full() .min_h_0() @@ -1324,23 +1305,37 @@ impl Render for DetailsPanel { }; let details_content = match history { - Some(history) if self.history_open => v_flex() - .size_full() - .min_h_0() - .child(div().flex_1().min_h(px(80.)).child(fields_and_notes)) - .child( - div() - .relative() - .flex_none() - .h(self.history_height) - .child(handle( - "details-history-resize", - DetailsSubPane::History, - self.history_height, - )) - .child(history), - ) - .into_any_element(), + Some(history) if self.history_open => { + let panel = cx.entity().downgrade(); + v_resizable("details-history-split") + .on_resize(move |state, _, cx| { + let Some(height) = state.read(cx).sizes().get(1).copied() else { + return; + }; + if let Some(panel) = panel.upgrade() { + panel.update(cx, |this, _| this.history_height = height); + } + if cx + .try_global::() + .is_some() + { + settings::project::CurrentProject::set_text( + HISTORY_PANE_HEIGHT_KEY, + format!("{}", f32::from(height)).into(), + cx, + ); + } + }) + .child(resizable_panel().min_h(px(80.)).child(fields_and_notes)) + .child( + resizable_panel() + .size(self.history_height) + .size_range(px(80.)..px(360.)) + .flex_none() + .child(history), + ) + .into_any_element() + } Some(history) => v_flex() .size_full() .min_h_0() @@ -1350,51 +1345,6 @@ impl Render for DetailsPanel { None => fields_and_notes, }; - let details_content = div() - .size_full() - .min_h_0() - .on_drag_move( - cx.listener(|this, event: &DragMoveEvent, _, cx| { - let pane = event.drag(cx).0; - let Some((started_pane, start_y, start_height)) = this.resize_start else { - return; - }; - if pane != started_pane { - return; - } - let height = (start_height + start_y - event.event.position.y).clamp( - px(80.), - match pane { - DetailsSubPane::Notes => px(320.), - DetailsSubPane::History => px(360.), - }, - ); - let key = match pane { - DetailsSubPane::Notes => { - this.notes_height = height; - NOTES_PANE_HEIGHT_KEY - } - DetailsSubPane::History => { - this.history_height = height; - HISTORY_PANE_HEIGHT_KEY - } - }; - if cx - .try_global::() - .is_some() - { - settings::project::CurrentProject::set_text( - key, - format!("{}", f32::from(height)).into(), - cx, - ); - } - cx.notify(); - }), - ) - .child(details_content) - .into_any_element(); - // Own context + tracked focus so Ctrl+Z reaches the grid's history from in here. While the // field editor holds focus its own deeper `Input` context wins, which keeps Ctrl+Z as // text-undo mid-edit. @@ -1567,7 +1517,9 @@ mod tests { // No `use super::*`: chain-globbing `gpui::*` shadows the built-in `#[test]` and recurses (see CLAUDE.md). use std::path::{Path, PathBuf}; - use gpui::{Context, IntoElement, Render, TestAppContext, Window, px}; + use gpui::{ + Context, IntoElement, Modifiers, MouseButton, Render, TestAppContext, Window, point, px, + }; use gpui::VisualTestContext; @@ -1713,6 +1665,75 @@ mod tests { cx.debug_bounds("details-notes-panel").is_some(), "the Notes section is present in the regular grid view" ); + + let history = cx + .debug_bounds("details-history-panel") + .expect("History is rendered"); + let start = point(history.center().x, history.top()); + cx.simulate_mouse_down(start, MouseButton::Left, Modifiers::default()); + // The first move crosses GPUI's drag threshold and installs the drag value; the next one + // is the resize itself. + cx.simulate_mouse_move( + point(start.x, start.y - px(5.)), + MouseButton::Left, + Modifiers::default(), + ); + cx.simulate_mouse_move( + point(start.x, start.y - px(25.)), + MouseButton::Left, + Modifiers::default(), + ); + cx.simulate_mouse_move( + point(start.x, start.y - px(45.)), + MouseButton::Left, + Modifiers::default(), + ); + cx.simulate_mouse_up( + point(start.x, start.y - px(45.)), + MouseButton::Left, + Modifiers::default(), + ); + panel.read_with(cx, |panel, _| { + assert!( + panel.history_height > px(190.), + "dragging the painted handle resizes History; height stayed {:?}", + panel.history_height + ); + }); + + panel.update(cx, |panel, cx| { + panel.history_open = false; + panel.notes_open = true; + cx.notify(); + }); + cx.update(|window, cx| window.draw(cx).clear(cx)); + let notes = cx + .debug_bounds("details-notes-panel") + .expect("Notes is rendered"); + let start = point(notes.center().x, notes.top()); + cx.simulate_mouse_down(start, MouseButton::Left, Modifiers::default()); + cx.simulate_mouse_move( + point(start.x, start.y - px(5.)), + MouseButton::Left, + Modifiers::default(), + ); + cx.simulate_mouse_move( + point(start.x, start.y - px(25.)), + MouseButton::Left, + Modifiers::default(), + ); + cx.simulate_mouse_up( + point(start.x, start.y - px(25.)), + MouseButton::Left, + Modifiers::default(), + ); + panel.read_with(cx, |panel, _| { + assert!( + panel.notes_height > px(210.), + "dragging the native divider resizes Notes; height stayed {:?}", + panel.notes_height + ); + }); } /// A bundle reports what its items agree on and counts what they don't, and typing into a From 24885146e58592b8e824ced7f06eaef8f447a645 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:28:19 -0700 Subject: [PATCH 08/16] feat(workspace): show one panel per dock instead of a tab strip The status bar already carries one button per panel and is the real switcher, so a dock holding two tools drew a tab strip that said the same thing twice. Each panel now reports `visible` only while it is the one its group is showing, which is the gate the skin already uses to draw a plain title bar in place of the strip. `set_active` is driven off the group's active index rather than off visibility, so exactly one panel per dock ever answers yes and there is no feedback loop. Nothing in PanelRegistry changes: `toggle` already brought a hidden panel to the front and closed the dock on the shown one. History also drew its own ellipsis button beside the one the tab bar always draws. Its item moves onto `Panel::dropdown_menu`, which the library builds from whichever panel is on screen, so the menu follows the dock's contents for free. --- crates/diagnostics/src/panel.rs | 14 ++++ crates/workspace/src/lib.rs | 49 +++++++++++ crates/workspace/src/panels/agent.rs | 14 ++++ crates/workspace/src/panels/history.rs | 112 ++++++++++++++----------- 4 files changed, 140 insertions(+), 49 deletions(-) diff --git a/crates/diagnostics/src/panel.rs b/crates/diagnostics/src/panel.rs index f7b06a83..b9307f79 100644 --- a/crates/diagnostics/src/panel.rs +++ b/crates/diagnostics/src/panel.rs @@ -350,6 +350,9 @@ fn project(mut items: Vec<&Diagnostic>, expanded: &BTreeSet) -> Vec /// Bottom dock: every open problem, filtered by severity and source, click to jump to it. pub struct ProblemsPanel { focus_handle: FocusHandle, + /// Whether this is the panel its dock is showing, which is what [`Self::visible`] + /// reports: a dock with one visible panel draws a title bar instead of a tab strip. + active: bool, filter: Filter, /// Sources unchecked in the multi-select filter. Empty means all diagnostic sources. excluded_sources: BTreeSet, @@ -374,6 +377,7 @@ impl ProblemsPanel { pub fn new(_window: &mut Window, cx: &mut Context) -> Self { let mut this = Self { focus_handle: cx.focus_handle(), + active: false, filter: Filter::All, excluded_sources: BTreeSet::new(), sources: Rc::default(), @@ -537,6 +541,16 @@ impl BasePanel for ProblemsPanel { false } + fn set_active(&mut self, active: bool, _w: &mut Window, cx: &mut Context) { + self.active = active; + cx.notify(); + } + + // Siblings stay docked and loaded, just unshown: this is what replaces the tab strip. + fn visible(&self, _cx: &App) -> bool { + self.active + } + fn zoomable(&self, _cx: &App) -> bool { true } diff --git a/crates/workspace/src/lib.rs b/crates/workspace/src/lib.rs index 1f7e1d1a..5c4d248f 100644 --- a/crates/workspace/src/lib.rs +++ b/crates/workspace/src/lib.rs @@ -816,6 +816,55 @@ mod tests { assert!(!cx.read(|cx| dock_area.read(cx).is_zoomed())); } + /// The edge docks show one panel at a time, and `Panel::visible` is what collapses the tab + /// strip into a title bar — so exactly one of the two right-dock panels may answer yes. + #[gpui::test] + fn only_the_panel_a_dock_is_showing_reports_itself_visible(cx: &mut TestAppContext) { + cx.update(|cx| { + gpui_component::init(cx); + cx.set_global(settings::AppSettings::default()); + cx.set_global(settings::SettingsPersistence::default()); + }); + let (workspace, cx) = cx.add_window_view(Workspace::new); + let dock_area = cx.update(|_, cx| workspace.read(cx).dock_area.clone()); + cx.run_until_parked(); + + let shown = |cx: &mut VisualTestContext| { + cx.update(|_, cx| { + PanelRegistry::entries(cx) + .iter() + .filter(|entry| { + entry.placement == DockPlacement::Right && entry.view.visible(cx) + }) + .map(|entry| entry.meta.name) + .collect::>() + }) + }; + + let first = shown(cx); + assert_eq!( + first.len(), + 1, + "the right dock shows one panel, not a strip of tabs" + ); + + let hidden = cx + .update(|_, cx| { + PanelRegistry::entries(cx) + .iter() + .find(|entry| { + entry.placement == DockPlacement::Right && entry.meta.name != first[0] + }) + .map(|entry| entry.meta.name) + }) + .expect("Agent and History both dock right"); + + cx.update(|window, cx| PanelRegistry::toggle(hidden, &dock_area, window, cx)); + cx.run_until_parked(); + + assert_eq!(shown(cx), vec![hidden], "showing one puts its sibling away"); + } + /// The real thing, lifted out of a project whose left dock came back with an "Unnamed" tab /// beside Details on every launch: an empty `TabPanel` sitting in a tab slot, and an /// `active_index` of 1 that points past the end once it is gone. diff --git a/crates/workspace/src/panels/agent.rs b/crates/workspace/src/panels/agent.rs index 98c0497a..5a1dc372 100644 --- a/crates/workspace/src/panels/agent.rs +++ b/crates/workspace/src/panels/agent.rs @@ -270,6 +270,9 @@ fn took(call: &AgentCall) -> SharedString { /// Right dock: what the external agent has asked qrate for, and what it got back. pub struct AgentPanel { focus_handle: FocusHandle, + /// Whether this is the panel its dock is showing, which is what [`Self::visible`] + /// reports: a dock with one visible panel draws a title bar instead of a tab strip. + active: bool, view: View, scroll: ScrollHandle, terminal: agent_runtime::AgentTerminal, @@ -318,6 +321,7 @@ impl AgentPanel { }); Self { focus_handle: cx.focus_handle(), + active: false, view: View::Terminal, scroll: scroll.clone(), terminal: Default::default(), @@ -374,6 +378,16 @@ impl BasePanel for AgentPanel { false } + fn set_active(&mut self, active: bool, _w: &mut Window, cx: &mut Context) { + self.active = active; + cx.notify(); + } + + // Siblings stay docked and loaded, just unshown: this is what replaces the tab strip. + fn visible(&self, _cx: &App) -> bool { + self.active + } + fn zoomable(&self, _cx: &App) -> bool { true } diff --git a/crates/workspace/src/panels/history.rs b/crates/workspace/src/panels/history.rs index 5c2bf1c7..bc6dbe9a 100644 --- a/crates/workspace/src/panels/history.rs +++ b/crates/workspace/src/panels/history.rs @@ -82,6 +82,9 @@ impl Filter { /// straight from the table and sit on top, where there is nothing to restore to yet. pub struct HistoryPanel { focus_handle: FocusHandle, + /// Whether this is the panel its dock is showing, which is what [`Self::visible`] + /// reports: a dock with one visible panel draws a title bar instead of a tab strip. + active: bool, scroll: ScrollHandle, saved: Vec, /// Whether a "Load older" would find anything. @@ -108,6 +111,7 @@ impl HistoryPanel { pub fn new(_window: &mut Window, cx: &mut Context) -> Self { let mut this = Self { focus_handle: cx.focus_handle(), + active: false, scroll: ScrollHandle::new(), saved: Vec::new(), more: false, @@ -512,6 +516,16 @@ impl BasePanel for HistoryPanel { false } + fn set_active(&mut self, active: bool, _w: &mut Window, cx: &mut Context) { + self.active = active; + cx.notify(); + } + + // Siblings stay docked and loaded, just unshown: this is what replaces the tab strip. + fn visible(&self, _cx: &App) -> bool { + self.active + } + fn zoomable(&self, _cx: &App) -> bool { true } @@ -530,65 +544,65 @@ impl Panel for HistoryPanel { let panel = cx.entity().downgrade(); let (filter, named_only) = (self.filter, self.named_only); Some( - h_flex() - .gap_1() - .items_center() - .child( - Button::new("history-filter") - .ghost() - .xsmall() - .label(if named_only { - "Named versions" - } else { - filter.label() - }) - .dropdown_menu({ - let panel = panel.clone(); - move |menu: PopupMenu, _window, _cx| { - let menu = Filter::ALL.into_iter().fold(menu, |menu, pick| { - let panel = panel.clone(); - menu.item( - PopupMenuItem::new(pick.label()) - .checked(!named_only && pick == filter) - .on_click(move |_, _, cx| { - panel - .update(cx, |this, cx| { - this.filter = pick; - this.named_only = false; - cx.notify(); - }) - .ok(); - }), - ) - }); + h_flex().gap_1().items_center().child( + Button::new("history-filter") + .ghost() + .xsmall() + .label(if named_only { + "Named versions" + } else { + filter.label() + }) + .dropdown_menu({ + let panel = panel.clone(); + move |menu: PopupMenu, _window, _cx| { + let menu = Filter::ALL.into_iter().fold(menu, |menu, pick| { let panel = panel.clone(); - menu.separator().item( - PopupMenuItem::new("Named versions") - .checked(named_only) + menu.item( + PopupMenuItem::new(pick.label()) + .checked(!named_only && pick == filter) .on_click(move |_, _, cx| { panel .update(cx, |this, cx| { - this.named_only = !this.named_only; + this.filter = pick; + this.named_only = false; cx.notify(); }) .ok(); }), ) - } - }), - ) - .child( - Button::new("history-more") - .icon(IconName::Ellipsis) - .ghost() - .xsmall() - .dropdown_menu(move |menu: PopupMenu, _window, _cx| { + }); let panel = panel.clone(); - menu.item(PopupMenuItem::new("Clear History…").on_click( - move |_, window, cx| Self::confirm_clear(panel.clone(), window, cx), - )) - }), - ), + menu.separator().item( + PopupMenuItem::new("Named versions") + .checked(named_only) + .on_click(move |_, _, cx| { + panel + .update(cx, |this, cx| { + this.named_only = !this.named_only; + cx.notify(); + }) + .ok(); + }), + ) + } + }), + ), + ) + } + + /// The tab bar already draws one ⋯ menu, and builds it from whichever panel its dock is + /// showing — so History’s own items belong there rather than in a second button beside it. + fn dropdown_menu( + &mut self, + menu: PopupMenu, + _w: &mut Window, + cx: &mut Context, + ) -> PopupMenu { + let panel = cx.entity().downgrade(); + menu.item( + PopupMenuItem::new("Clear History…") + .on_click(move |_, window, cx| Self::confirm_clear(panel.clone(), window, cx)), ) } } From 4d0cb341a3e896a922f2e76778a83da1fee39b25 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:28:44 -0700 Subject: [PATCH 09/16] fix(details): give the notes and history splits floors that fit their contents Dragging the History handle up hid the Notes header entirely, and the fields painted through the section below it. The three splits nest, but each pane declared a minimum with no reference to what it holds: the history split's first pane floored at the fields' own 80px while holding the fields *and* Notes, and the preview split's second pane took the 100px default while holding all three sections. The resizer shrank them past what their children fit in, and nothing clipped the surplus. The floors were not even the ones written. gpui-base applies a caller's style refinement and then overwrites min_h with size_range.start, and the drag clamp reads size_range too, so `min_h` on a resizable_panel binds nothing at either end. Those panes never set a range, so both ends used the 100px default. Every floor is now the sum of its contents, via size_range, and a collapsed section is boxed at its real height (header plus the rule it draws above itself) rather than 1px short. The panel clips at its own edge so a floor that is still wrong truncates a section instead of drawing two over each other. Also carries the Details panel's own set_active/visible hooks from the previous commit: the two topics share this file and splitting it would have meant rewriting it. --- crates/workspace/src/panels/details.rs | 88 +++++++++++++++++++++++--- 1 file changed, 80 insertions(+), 8 deletions(-) diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 87a2ac3d..8028241d 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -40,6 +40,24 @@ const ROW_HISTORY_LIMIT: i64 = 50; /// Height of the Notes sub-panel's header bar, which is the whole of it while collapsed. const NOTES_HEADER_H: f32 = 28.; +/// What a collapsed sub-panel occupies: its header, plus the rule the section draws above it. +const SECTION_STRIP_H: f32 = NOTES_HEADER_H + 1.; +/// The least height a sub-panel, or the field list above it, is worth showing in. +const SECTION_MIN_H: f32 = 80.; + +/// What a sub-panel costs the height above it — nothing when the selection has none to show, its +/// header strip while collapsed, its own floor while open. +/// +/// The splits nest, so every pane's floor is the sum of what its pane holds. Written once here +/// because a floor computed independently of its contents is what let History be dragged over +/// Notes: the pane shrank to a minimum its own children could not fit in, and they overflowed it. +fn section_footprint(shown: bool, open: bool) -> f32 { + match (shown, open) { + (false, _) => 0., + (true, false) => SECTION_STRIP_H, + (true, true) => SECTION_MIN_H, + } +} /// How much of the window the open field editor may span when the panel itself is narrower. const EDITOR_MAX_WINDOW_SHARE: f32 = 0.4; @@ -63,6 +81,9 @@ pub static DETAILS_META: PanelMeta = PanelMeta { /// label/value list, per the main-workspace design. pub struct DetailsPanel { focus_handle: FocusHandle, + /// Whether this is the panel its dock is showing, which is what [`Self::visible`] + /// reports: a dock with one visible panel draws a title bar instead of a tab strip. + active: bool, /// Live table state, read for the selected row. state: Option>>, /// Re-binds `state` whenever `TablePanel` publishes a new table (project reload, dock @@ -144,6 +165,7 @@ impl DetailsPanel { ); let mut this = Self { focus_handle: cx.focus_handle(), + active: false, state: None, _handle_sub, _table_sub: None, @@ -829,6 +851,16 @@ impl BasePanel for DetailsPanel { false } + fn set_active(&mut self, active: bool, _w: &mut Window, cx: &mut Context) { + self.active = active; + cx.notify(); + } + + // Siblings stay docked and loaded, just unshown: this is what replaces the tab strip. + fn visible(&self, _cx: &App) -> bool { + self.active + } + fn zoomable(&self, _cx: &App) -> bool { true } @@ -1113,6 +1145,8 @@ impl Render for DetailsPanel { // iterator — this needs `&mut cx` and cannot wait for them. let notes = (count > 0).then(|| self.notes_panel(&picked, cx)); let history = (count == 1).then(|| self.history_panel(cx)); + let notes_floor = section_footprint(notes.is_some(), self.notes_open); + let history_floor = section_footprint(history.is_some(), self.history_open); // Hand-built attribute list, not `DescriptionList`/`DataTable`: the fields are fixed pairs, // and it reads as a list rather than a second grid — alternating rows carry the structure, @@ -1285,11 +1319,19 @@ impl Render for DetailsPanel { ); } }) - .child(resizable_panel().min_h(px(80.)).pr_2().child(fields)) + // `size_range`, not `min_h`: the resizer's drag clamp reads the range, and the + // panel overwrites a caller's `min_h` with the range's start on every render — + // so a floor written as `min_h` binds nothing at either end. + .child( + resizable_panel() + .size_range(px(SECTION_MIN_H)..Pixels::MAX) + .pr_2() + .child(fields), + ) .child( resizable_panel() .size(self.notes_height) - .size_range(px(80.)..px(320.)) + .size_range(px(SECTION_MIN_H)..px(320.)) .flex_none() .child(notes), ) @@ -1299,7 +1341,13 @@ impl Render for DetailsPanel { .size_full() .min_h_0() .child(div().flex_1().min_h_0().pr_2().child(fields)) - .child(div().flex_none().h(px(NOTES_HEADER_H)).child(notes)) + .child( + div() + .flex_none() + .h(px(SECTION_STRIP_H)) + .overflow_hidden() + .child(notes), + ) .into_any_element(), None => fields, }; @@ -1326,11 +1374,18 @@ impl Render for DetailsPanel { ); } }) - .child(resizable_panel().min_h(px(80.)).child(fields_and_notes)) + // The floor carries Notes with it: this pane holds the fields *and* whatever + // Notes is currently taking, so stopping at the fields' own floor is what let + // the History handle be dragged up over the Notes header. + .child( + resizable_panel() + .size_range(px(SECTION_MIN_H + notes_floor)..Pixels::MAX) + .child(fields_and_notes), + ) .child( resizable_panel() .size(self.history_height) - .size_range(px(80.)..px(360.)) + .size_range(px(SECTION_MIN_H)..px(360.)) .flex_none() .child(history), ) @@ -1340,7 +1395,13 @@ impl Render for DetailsPanel { .size_full() .min_h_0() .child(div().flex_1().min_h_0().child(fields_and_notes)) - .child(div().flex_none().h(px(NOTES_HEADER_H)).child(history)) + .child( + div() + .flex_none() + .h(px(SECTION_STRIP_H)) + .overflow_hidden() + .child(history), + ) .into_any_element(), None => fields_and_notes, }; @@ -1373,7 +1434,10 @@ impl Render for DetailsPanel { } })) .child( - div().size_full().min_h_0().child( + // Clipped at the panel's own edge: the floors below are what keep the sections + // apart, and this is what makes a floor that is still wrong read as a truncated + // section rather than two of them drawn over each other. + div().size_full().min_h_0().overflow_hidden().child( v_resizable("details-preview-split") .on_resize({ let panel = cx.entity().downgrade(); @@ -1505,7 +1569,15 @@ impl Render for DetailsPanel { }), ) }) - .child(resizable_panel().child(details_content)), + // Same sum one level up: dragging the photo down may not push the fields, + // Notes and History below the height the three of them need. + .child( + resizable_panel() + .size_range( + px(SECTION_MIN_H + notes_floor + history_floor)..Pixels::MAX, + ) + .child(details_content), + ), ), ) .into_any_element() From 325d196b3235820ea417c024dcd4c4c29eb3eaee Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:37:03 -0700 Subject: [PATCH 10/16] perf(history): stop the log's cost growing with the project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The change log is append-only and a working project accumulates one entry per edit, so every read path that scaled with the whole log rather than with the page being shown was a stall waiting to happen. Reading a page ran one `WHERE entry_id = ?` per entry, and the only index on `__history_changes` was `(row_id, column_name)` — so a page of 200 was 200 full scans of every change ever recorded. The changes for a page now come back in one query, and the table carries an `(entry_id, seq)` index. Existing projects pick the index up at their next write; the batching already removes the N+1 without it. The panel's unforced refresh re-read every page the reader had loaded, so writing a note cost more the further back they had paged. It now re-reads only the newest page and splices it onto the pages below, which the log being append-only makes sound. A forced reload — a rename, a clear — still re-reads the lot. The Details panel's History section built and formatted every line even while collapsed, when all it draws is a header strip; it now asks only whether anything is behind it. --- crates/settings/src/history.rs | 43 +++++++-- crates/workspace/src/panels/details.rs | 69 +++++++++----- crates/workspace/src/panels/history.rs | 125 ++++++++++++++++++++++++- 3 files changed, 204 insertions(+), 33 deletions(-) diff --git a/crates/settings/src/history.rs b/crates/settings/src/history.rs index c86cb0dd..81acf439 100644 --- a/crates/settings/src/history.rs +++ b/crates/settings/src/history.rs @@ -10,6 +10,7 @@ //! `project::save_dataset` and `project::write_notes`), so the log never claims a change the file //! doesn't hold. +use std::collections::HashMap; use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; @@ -183,8 +184,9 @@ impl Change { } } - /// The row and column this change is looked up by. - fn key(&self) -> (Option, Option<&str>) { + /// The row and column a change concerns, which is how a reader asks whether it is about the + /// item they are looking at without formatting it first. + pub fn key(&self) -> (Option, Option<&str>) { match self { Change::Cell { row, column, .. } => (Some(*row), Some(column)), Change::RowAdded { row, .. } | Change::RowRemoved { row, .. } => (Some(*row), None), @@ -258,6 +260,7 @@ const HISTORY_DDL: &str = r#" change TEXT NOT NULL ); CREATE INDEX IF NOT EXISTS __history_changes_cell ON __history_changes(row_id, column_name); + CREATE INDEX IF NOT EXISTS __history_changes_entry ON __history_changes(entry_id, seq); "#; /// Write `entries` on `conn`, inside whatever transaction the caller holds. @@ -400,13 +403,37 @@ fn select(path: &Path, filter: &str, bound: &[&dyn rusqlite::ToSql]) -> Result>>()?; - let mut changes = - conn.prepare("SELECT change FROM __history_changes WHERE entry_id = ?1 ORDER BY seq")?; + // One query for the whole page's changes rather than one per entry. Per entry, each lookup + // was a scan of `__history_changes` — a table that grows with the project, not with the page — + // so reading 200 entries cost 200 scans of everything ever edited. + // + // The id list is interpolated because rusqlite binds no arrays without `rarray`; every value + // is an `EntryId` this function just read out of the same table, so there is no text here to + // escape. + let mut changes: HashMap> = HashMap::new(); + if !listed.is_empty() { + let ids = listed + .iter() + .map(|listed| listed.entry.id.to_string()) + .collect::>() + .join(","); + let mut stmt = conn.prepare(&format!( + "SELECT entry_id, change FROM __history_changes + WHERE entry_id IN ({ids}) ORDER BY entry_id, seq" + ))?; + let rows = stmt.query_map([], |r| { + Ok((r.get::<_, EntryId>(0)?, r.get::<_, String>(1)?)) + })?; + for row in rows { + let (id, change) = row?; + changes + .entry(id) + .or_default() + .push(serde_json::from_str(&change).context("Read history change")?); + } + } for listed in &mut listed { - listed.entry.changes = changes - .query_map([listed.entry.id], |r| r.get::<_, String>(0))? - .map(|c| serde_json::from_str(&c?).context("Read history change")) - .collect::>()?; + listed.entry.changes = changes.remove(&listed.entry.id).unwrap_or_default(); } Ok(listed) } diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 8028241d..20e2e88a 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -576,29 +576,54 @@ impl DetailsPanel { }) .collect::>() }; - let ats: Vec = unsaved.iter().map(|entry| entry.at).collect(); - let times = settings::history::local_times(&ats).unwrap_or_else(|err| { - log::error!("couldn't read the local time of unsaved changes: {err}"); - Vec::new() - }); - let lines: Vec = unsaved - .iter() - .enumerate() - .rev() - .flat_map(|(ix, entry)| { - let when = times.get(ix).map_or_else( - || "not saved yet".to_string(), - |(day, time)| format!("{}, not saved yet", super::history::when(day, time)), - ); - lines_of(entry, None, when) - }) - .chain(saved.iter().flat_map(|listed| { - let when = super::history::when(&listed.day, &listed.time); - lines_of(&listed.entry, Some(listed.entry.id), when) - })) - .collect(); + // Collapsed, this section is a header strip, and the only thing it still has to know is + // whether anything sits behind it. Building every line to answer that formatted the whole + // of the item's history on every frame of a panel showing none of it — and the history is + // the part of this file that grows without limit. + let open = self.history_open; + let lines: Vec = match open { + false => Vec::new(), + true => { + let ats: Vec = unsaved.iter().map(|entry| entry.at).collect(); + let times = settings::history::local_times(&ats).unwrap_or_else(|err| { + log::error!("couldn't read the local time of unsaved changes: {err}"); + Vec::new() + }); + unsaved + .iter() + .enumerate() + .rev() + .flat_map(|(ix, entry)| { + let when = times.get(ix).map_or_else( + || "not saved yet".to_string(), + |(day, time)| { + format!("{}, not saved yet", super::history::when(day, time)) + }, + ); + lines_of(entry, None, when) + }) + .chain(saved.iter().flat_map(|listed| { + let when = super::history::when(&listed.day, &listed.time); + lines_of(&listed.entry, Some(listed.entry.id), when) + })) + .collect() + } + }; - let (open, none) = (self.history_open, lines.is_empty()); + // `saved` is already the entries that touch this item, so it answers for itself; the + // unsaved ones are not filtered and have to be asked. + let none = match open { + true => lines.is_empty(), + false => { + saved.is_empty() + && !unsaved.iter().any(|entry| { + entry + .changes + .iter() + .any(|change| change.key().0 == Some(row_id)) + }) + } + }; v_flex() .debug_selector(|| "details-history-panel".into()) .size_full() diff --git a/crates/workspace/src/panels/history.rs b/crates/workspace/src/panels/history.rs index bc6dbe9a..6f3889e1 100644 --- a/crates/workspace/src/panels/history.rs +++ b/crates/workspace/src/panels/history.rs @@ -167,12 +167,31 @@ impl HistoryPanel { if !force && self.read_at == fresh { return; } - let limit = PAGE.max(self.saved.len() as i64); + // The log is append-only, so a refresh nothing asked for only has to look at the newest + // page: everything below it is already what it will ever be. Re-reading every page the + // reader had loaded made each note they wrote cost more the further back they had paged. + // A forced reload still re-reads the lot, which is what a rename or a clear needs. + let limit = match force { + true => PAGE.max(self.saved.len() as i64), + false => PAGE, + }; let row = self.cell.as_ref().map(|(row, _)| *row); match settings::history::page(&file, EntryId::MAX, limit, row) { Ok(page) => { - self.more = page.len() as i64 == limit; + // Pages the re-read didn't reach, kept rather than dropped — otherwise an + // unforced refresh would silently collapse the list back to one page. + let cutoff = page.last().map_or(EntryId::MAX, |listed| listed.entry.id); + let older = self + .saved + .iter() + .position(|listed| listed.entry.id < cutoff) + .map(|ix| self.saved.split_off(ix)) + .unwrap_or_default(); + if older.is_empty() { + self.more = page.len() as i64 == limit; + } self.saved = page; + self.saved.extend(older); } Err(err) => log::error!("couldn't read the project history: {err}"), } @@ -987,10 +1006,110 @@ impl Render for HistoryPanel { #[cfg(test)] mod tests { // Never `use super::*` here: the parent has `use gpui::*` in scope. - use super::{day_label, describe, narrowed}; + use super::{HistoryPanel, PAGE, day_label, describe, narrowed}; + use gpui::TestAppContext; use settings::history::{Change, Entry, Origin}; use std::collections::HashMap; + /// A project holding `entries` changes to one row, and the panel pointed at it. + fn project_with(name: &str, entries: usize) -> std::path::PathBuf { + let dir = std::env::temp_dir().join("qrate-history-panel-test"); + let _ = std::fs::create_dir_all(&dir); + let path = dir.join(name); + let _ = std::fs::remove_file(&path); + let headers = vec!["Title".to_string()]; + settings::project::create_project_file( + &path, + &settings::project::ProjectSpec { + name: "H", + source: "CSV", + headers: &headers, + rows: &[vec!["one".to_string()]], + ..Default::default() + }, + ) + .unwrap(); + append(&path, 0, entries); + path + } + + /// `count` more entries on top of whatever the file already holds. + fn append(path: &std::path::Path, from: usize, count: usize) { + let headers = vec!["Title".to_string()]; + let log: Vec = (from..from + count) + .map(|n| { + Entry::new( + Origin::Typed, + vec![Change::Cell { + row: 1, + column: "Title".into(), + before: n.to_string(), + after: (n + 1).to_string(), + }], + None, + ) + }) + .collect(); + settings::project::save_dataset( + path, + &headers, + &[1], + &[vec![(from + count).to_string()]], + &log, + ) + .unwrap(); + } + + /// An unforced refresh re-reads only the newest page, so it must splice that page onto the + /// older ones already loaded rather than replacing them — otherwise writing a note would + /// collapse the list back to one page under whoever had paged through a long history. + #[gpui::test] + fn a_refresh_keeps_the_pages_the_reader_already_loaded(cx: &mut TestAppContext) { + let extra = 5; + let total = PAGE as usize + extra; + let path = project_with("merge.qrate", total); + cx.update(|cx| { + gpui_component::init(cx); + cx.set_global(settings::AppSettings::default()); + cx.set_global(settings::SettingsPersistence::default()); + cx.set_global(settings::project::CurrentProject { + file: path.clone(), + data: settings::project::load_project_file(&path).unwrap(), + }); + }); + + let (panel, cx) = cx.add_window_view(HistoryPanel::new); + cx.run_until_parked(); + + panel.update(cx, |this, _| { + assert_eq!(this.saved.len(), PAGE as usize, "one page to begin with"); + assert!(this.more, "and more behind it"); + }); + + cx.update(|_, cx| panel.update(cx, |this, cx| this.load_older(cx))); + panel.update(cx, |this, _| { + assert_eq!(this.saved.len(), total, "the rest paged in"); + assert!(!this.more, "nothing left behind it"); + }); + + // A write the panel didn't ask about — a note, in real use — moves the file's stamp and + // so gets past the freshness guard. + append(&path, total, 1); + cx.update(|_, cx| panel.update(cx, |this, cx| this.reload(false, cx))); + panel.update(cx, |this, _| { + assert_eq!( + this.saved.len(), + total + 1, + "the new entry arrives and the older pages stay" + ); + let ids: Vec<_> = this.saved.iter().map(|l| l.entry.id).collect(); + let mut descending = ids.clone(); + descending.sort_by(|a, b| b.cmp(a)); + descending.dedup(); + assert_eq!(ids, descending, "still newest first, with nothing doubled"); + }); + } + /// Narrowed to one cell, an entry keeps that cell's edits under any name its column has had, /// its row's notes and its row coming or going — and nothing about the cells beside it. #[test] From 0dca7f2aebe8640de8d3ec7fab9bd2ed600f4cf1 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:41:08 -0700 Subject: [PATCH 11/16] test(history): pin the order a page hands back each entry's changes The batched change fetch distributes one query's rows across the entries of a page, so an entry holding several changes is what catches a distribution that mixes two entries together or loses the sequence they were written in. Nothing covered that: every entry the existing tests write holds exactly one change, and a reordered pair reads as the wrong before-value for a restore. --- crates/settings/src/history.rs | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/crates/settings/src/history.rs b/crates/settings/src/history.rs index 81acf439..3a8c7083 100644 --- a/crates/settings/src/history.rs +++ b/crates/settings/src/history.rs @@ -660,4 +660,36 @@ mod tests { assert_eq!(edit("a", "b").inverse(), edit("b", "a")); assert_eq!(moved.inverse().inverse(), moved); } + + /// A page fetches every entry's changes in one query and hands each entry its own back. Two + /// entries, each with several changes, is the arrangement that catches a distribution that + /// mixes them up or loses the order they were written in — which a restore reads as the wrong + /// value to put back. + #[test] + fn a_page_keeps_each_entrys_changes_together_and_in_order() { + let path = project("change-order.qrate"); + let headers = vec!["Title".to_string()]; + let first = Entry::new( + Origin::Paste, + vec![edit("a", "b"), edit("b", "c"), edit("c", "d")], + None, + ); + let second = Entry::new(Origin::Typed, vec![edit("d", "e"), edit("e", "f")], None); + save_dataset( + &path, + &headers, + &[1], + &[vec!["f".into()]], + &[first.clone(), second.clone()], + ) + .unwrap(); + + let listed = page(&path, EntryId::MAX, 10, None).unwrap(); + let changes: Vec<_> = listed + .iter() + .map(|listed| listed.entry.changes.clone()) + .collect(); + // Newest first, so the second entry leads. + assert_eq!(changes, vec![second.changes, first.changes]); + } } From 99041f0470dd82a9816e61bc302728b33b1a695d Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:05:05 -0700 Subject: [PATCH 12/16] perf(history): build only the rows the panel is showing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every loaded entry became an element on every frame — a hover, a scroll, a keystroke in the grid each rebuilt the whole list. One page is 200 entries and "Load older" has no ceiling, so the panel got heavier the longer a researcher worked and the further back they looked. The list is now flattened to one addressable row per line and drawn through `v_virtual_list`, which builds only the visible range. The rows hold indices into the filtered view rather than text, so formatting a description — the expensive part — happens for the rows on screen and nowhere else. The list places rows by fixed heights and gives the rows those same heights, rather than measuring: measuring means building every row to ask how tall it is, which is the cost this removes. The constants and the rows they belong to have to be changed together, which is what the note on them says. The unsaved entries' local times were already fetched in one call; they now happen once per render rather than once per row built. --- crates/workspace/src/panels/history.rs | 687 +++++++++++++++---------- 1 file changed, 419 insertions(+), 268 deletions(-) diff --git a/crates/workspace/src/panels/history.rs b/crates/workspace/src/panels/history.rs index 6f3889e1..0b9fa574 100644 --- a/crates/workspace/src/panels/history.rs +++ b/crates/workspace/src/panels/history.rs @@ -1,6 +1,7 @@ use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::path::PathBuf; +use std::rc::Rc; use std::time::{SystemTime, UNIX_EPOCH}; use gpui::prelude::FluentBuilder as _; @@ -8,11 +9,10 @@ use gpui::*; use gpui_component::dock::{BasePanel, DockPlacement, Panel, PanelEvent}; use gpui_component::input::{Input, InputEvent, InputState}; use gpui_component::menu::{ContextMenuExt as _, DropdownMenu as _, PopupMenu, PopupMenuItem}; -use gpui_component::scroll::ScrollableElement as _; use gpui_component::{ - ActiveTheme as _, IconName, Sizable as _, + ActiveTheme as _, IconName, Sizable as _, VirtualListScrollHandle, button::{Button, ButtonVariants as _}, - h_flex, v_flex, + h_flex, v_flex, v_virtual_list, }; use settings::history::{Change, Entry, EntryId, Listed, Origin, ShowCellHistory}; use settings::project::{CurrentProject, RowId}; @@ -36,6 +36,37 @@ const PAGE: i64 = 200; /// one burst of work and collapse under one row. const BURST_SECS: i64 = 5 * 60; +/// One line of the list, as the virtual list addresses it. Holds indices rather than text: the +/// whole list is walked every render to place it, but only the rows on screen are ever formatted. +enum Row { + Header(SharedString), + /// Into [`HistoryPanel::pending`]. + Pending(usize), + /// Into [`HistoryPanel::view`]. + Saved { + ix: usize, + indent: bool, + }, + /// The head of a run of `len` entries in [`HistoryPanel::view`], collapsed under one row. + Burst { + ix: usize, + len: usize, + }, + More, +} + +/// The heights the list places rows by, which are also the heights the rows are given. Measuring +/// instead would mean building every row to find out how tall it is, which is the whole cost +/// virtualizing is here to avoid — so a row is told its height rather than asked for it. Change +/// one of these and change the row it belongs to. +const HEADER_H: f32 = 28.; +const ROW_H: f32 = 48.; +/// The extra line a named version carries above its description. +const LABEL_H: f32 = 18.; +/// The box that appears while that name is being typed. +const NAMING_H: f32 = 28.; +const MORE_H: f32 = 40.; + #[derive(Clone, Copy, PartialEq, Eq)] enum Filter { All, @@ -85,7 +116,7 @@ pub struct HistoryPanel { /// Whether this is the panel its dock is showing, which is what [`Self::visible`] /// reports: a dock with one visible panel draws a title bar instead of a tab strip. active: bool, - scroll: ScrollHandle, + list_scroll: VirtualListScrollHandle, saved: Vec, /// Whether a "Load older" would find anything. more: bool, @@ -100,6 +131,13 @@ pub struct HistoryPanel { naming: Option<(EntryId, Entity, Subscription)>, /// One cell to show the changes of, as its row and every name its column has had. cell: Option<(RowId, Vec)>, + /// What the list is showing, rebuilt each render: the flattened rows, the saved entries they + /// index into once filtered and narrowed, and the entries not on disk yet with the meta line + /// each needs. On `self` rather than in `render` because the virtual list draws a range of + /// them later, from a callback that is handed the panel and nothing else. + list: Rc>, + view: Rc>, + pending: Rc>, _cell_sub: Subscription, _handle_sub: Subscription, _table_sub: Option, @@ -112,7 +150,7 @@ impl HistoryPanel { let mut this = Self { focus_handle: cx.focus_handle(), active: false, - scroll: ScrollHandle::new(), + list_scroll: VirtualListScrollHandle::new(), saved: Vec::new(), more: false, read_at: None, @@ -121,6 +159,9 @@ impl HistoryPanel { expanded: HashSet::new(), naming: None, cell: None, + list: Rc::default(), + view: Rc::default(), + pending: Rc::default(), _cell_sub: cx.observe_global::(|this: &mut Self, cx| { this.show_cell(cx); }), @@ -626,8 +667,38 @@ impl Panel for HistoryPanel { } } -impl Render for HistoryPanel { - fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { +impl HistoryPanel { + /// How tall the list places a row, which is also the height the row is given — see the + /// constants above. + fn row_size(&self, row: &Row) -> Size { + let height = match row { + Row::Header(_) => HEADER_H, + Row::More => MORE_H, + Row::Pending(_) | Row::Burst { .. } => ROW_H, + Row::Saved { ix, .. } => match self.view.get(*ix) { + None => ROW_H, + Some(listed) => { + let naming = self + .naming + .as_ref() + .is_some_and(|(id, _, _)| *id == listed.entry.id); + ROW_H + + if listed.label.is_some() { LABEL_H } else { 0. } + + if naming { NAMING_H } else { 0. } + } + }, + }; + size(px(0.), px(height)) + } + + /// One row of the list, built only when it is on screen. `AnyElement` because the arms are + /// different elements, which is the case the crate's style note allows it for. + fn draw_row( + &self, + row: &Row, + positions: &HashMap, + cx: &mut Context, + ) -> AnyElement { let theme = cx.theme(); let (muted, border, hover_bg, accent) = ( theme.muted_foreground, @@ -635,77 +706,8 @@ impl Render for HistoryPanel { theme.secondary_hover, theme.accent_foreground, ); - let crop = cx.try_global::().map_or(px(0.), |c| c.0); let panel = cx.entity().downgrade(); - - let (rows, unsaved): (HashMap, Vec) = cx - .try_global::() - .and_then(|h| h.0.upgrade()) - .map(|state| { - let delegate = state.read(cx).delegate(); - ( - delegate - .row_ids() - .iter() - .enumerate() - .map(|(p, id)| (*id, p)) - .collect(), - delegate.unsaved_history().to_vec(), - ) - }) - .unwrap_or_default(); - let (filter, named_only) = (self.filter, self.named_only); - let admits = |entry: &Entry, label: bool| { - if named_only { - label - } else { - filter.admits(entry) - } - }; - - let cell = self.cell.as_ref(); - let unsaved: Vec = unsaved - .iter() - .rev() - .filter_map(|e| narrowed(e, cell).map(Cow::into_owned)) - .filter(|e| admits(e, false)) - .collect(); - let saved: Vec> = self - .saved - .iter() - .filter_map(|l| match narrowed(&l.entry, cell)? { - Cow::Borrowed(_) => Some(Cow::Borrowed(l)), - Cow::Owned(entry) => Some(Cow::Owned(Listed { entry, ..l.clone() })), - }) - .filter(|l| admits(&l.entry, l.label.is_some())) - .collect(); - let cell_label = cell.map(|(row, names)| { - let at = rows - .get(row) - .map_or_else(|| "a deleted row".to_string(), |p| format!("row {}", p + 1)); - format!("{}, {at}", names[0]) - }); - - // Bursts: runs of adjacent, unnamed entries by one person, one means, one day. - let mut bursts: Vec> = Vec::new(); - for listed in saved.iter().map(|l| &**l) { - let joins = bursts.last().and_then(|b| b.last()).is_some_and(|prev| { - prev.label.is_none() - && listed.label.is_none() - && prev.day == listed.day - && prev.entry.author == listed.entry.author - && prev.entry.origin == listed.entry.origin - && matches!( - listed.entry.origin, - Origin::Typed | Origin::Details | Origin::Paste | Origin::Clear - ) - && prev.entry.at - listed.entry.at <= BURST_SECS - }); - match (joins, bursts.last_mut()) { - (true, Some(burst)) => burst.push(listed), - _ => bursts.push(vec![listed]), - } - } + let height = self.row_size(row).height; // ponytail: always offered, since gpui cannot tell us whether the text was cut let clipped = |text: String| { @@ -718,88 +720,6 @@ impl Render for HistoryPanel { gpui_component::tooltip::Tooltip::new(text.clone()).build(window, cx) }) }; - let entry_row = |id: ElementId, - entry: &Entry, - listed: Option<&Listed>, - meta: String, - indent: bool| { - let dimmed = matches!(entry.origin, Origin::Undo | Origin::Redo); - let naming = listed.and_then(|l| { - self.naming - .as_ref() - .filter(|(id, _, _)| *id == l.entry.id) - .map(|(_, input, _)| input.clone()) - }); - let actions = listed.cloned().map(|listed| { - let panel = panel.clone(); - move |menu: PopupMenu, _window: &mut Window, _cx: &mut Context| { - let single = match listed.entry.changes.as_slice() { - [ - Change::Cell { - row, - column, - before, - .. - }, - ] => Some((*row, column.clone(), before.clone())), - _ => None, - }; - let (restore, name, unname) = (listed.clone(), panel.clone(), panel.clone()); - let id = listed.entry.id; - menu.item( - PopupMenuItem::new("Restore Project to Here…").on_click( - move |_, window, cx| Self::confirm_restore(&restore, window, cx), - ), - ) - .when_some(single, |menu, (row, column, before)| { - menu.item(PopupMenuItem::new("Restore This Value").on_click( - move |_, _, cx| { - table::restore_value(row, &column, before.clone().into(), id, cx) - }, - )) - }) - .separator() - .item(PopupMenuItem::new("Name This Version…").on_click( - move |_, window, cx| { - name.update(cx, |this, cx| this.start_naming(id, window, cx)) - .ok(); - }, - )) - .when(listed.label.is_some(), |menu| { - menu.item(PopupMenuItem::new("Remove Name").on_click(move |_, _, cx| { - unname.update(cx, |this, cx| this.name(id, None, cx)).ok(); - })) - }) - } - }); - v_flex() - .id(id) - .w_full() - .gap_0p5() - .px_2() - .py_1() - .when(indent, |row| row.pl_6()) - .border_b_1() - .border_color(border) - .hover(|row| row.bg(hover_bg)) - .when(dimmed, |row| row.opacity(0.6)) - .when_some(listed.and_then(|l| l.label.clone()), |row, label| { - row.child( - div() - .text_xs() - .font_weight(FontWeight::SEMIBOLD) - .text_color(accent) - .child(label), - ) - }) - .when_some(naming, |row, input| row.child(Input::new(&input).xsmall())) - .child(clipped(describe(entry, &rows)).text_sm()) - .child(div().text_xs().text_color(muted).child(meta)) - .map(|row| match actions { - Some(actions) => row.context_menu(actions).into_any_element(), - None => row.into_any_element(), - }) - }; let meta = |entry: &Entry, when: String| { [Some(entry.origin.label()), entry.author.clone(), Some(when)] .into_iter() @@ -807,93 +727,174 @@ impl Render for HistoryPanel { .collect::>() .join(" · ") }; - let header = |text: String| { - div() + + match row { + Row::Header(text) => div() + .h(height) .px_2() .pt_2() .pb_1() .text_xs() .font_weight(FontWeight::SEMIBOLD) .text_color(muted) - .child(text) - }; + .child(text.clone()) + .into_any_element(), - let empty = unsaved.is_empty() && bursts.is_empty(); - let mut list = v_flex() - .id("history-list") - .size_full() - .min_h_0() - .overflow_y_scroll() - .track_scroll(&self.scroll) - .vertical_scrollbar(&self.scroll) - .pr_2() - .pb(px(8.) + crop); + Row::More => div() + .h(height) + .p_2() + .child( + Button::new("history-older") + .ghost() + .small() + .label("Load Older Changes") + .on_click(cx.listener(|this, _, _, cx| this.load_older(cx))), + ) + .into_any_element(), - if empty { - list = list.child(div().p_3().text_sm().text_color(muted).child( - match (cx.has_global::(), named_only) { - (false, _) => "Open a project to see its history.", - (true, true) => "No named versions yet. Right-click a change to name it.", - (true, false) => "No changes yet. Every edit to this project's data and notes will be listed here.", - }, - )); - } - if !unsaved.is_empty() { - list = list.child(header("Not saved yet".into())); - let ats: Vec = unsaved.iter().map(|e| e.at).collect(); - let times = settings::history::local_times(&ats).unwrap_or_else(|err| { - log::error!("couldn't read the local time of unsaved changes: {err}"); - Vec::new() - }); - for (ix, entry) in unsaved.iter().enumerate() { - let at = times.get(ix).map_or_else( - || "not saved".to_string(), - |(day, time)| format!("{}, not saved", when(day, time)), - ); - list = list.child(entry_row( - ElementId::NamedInteger("history-unsaved".into(), ix as u64), - entry, - None, - meta(entry, at), - false, - )); - } - } - let mut day = None; - for burst in &bursts { - let head = burst[0]; - if day != Some(&head.day) { - day = Some(&head.day); - list = list.child(header(day_label(&head.day, head.days_ago))); + Row::Pending(ix) => { + let Some((entry, at)) = self.pending.get(*ix) else { + return div().into_any_element(); + }; + v_flex() + .id(ElementId::NamedInteger( + "history-unsaved".into(), + *ix as u64, + )) + .h(height) + .w_full() + .gap_0p5() + .px_2() + .py_1() + .border_b_1() + .border_color(border) + .hover(|row| row.bg(hover_bg)) + .when(matches!(entry.origin, Origin::Undo | Origin::Redo), |row| { + row.opacity(0.6) + }) + .child(clipped(describe(entry, positions)).text_sm()) + .child( + div() + .text_xs() + .text_color(muted) + .child(meta(entry, at.clone())), + ) + .into_any_element() } - let id = head.entry.id; - let row_id = |listed: &Listed| { - ElementId::NamedInteger("history-entry".into(), listed.entry.id as u64) - }; - if burst.len() == 1 { - list = list.child(entry_row( - row_id(head), - &head.entry, - Some(head), - meta(&head.entry, when(&head.day, &head.time)), - false, - )); - continue; + + Row::Saved { ix, indent } => { + let Some(listed) = self.view.get(*ix) else { + return div().into_any_element(); + }; + let entry = &listed.entry; + let id = entry.id; + let naming = self + .naming + .as_ref() + .filter(|(being_named, _, _)| *being_named == id) + .map(|(_, input, _)| input.clone()); + let actions = { + let listed = listed.clone(); + let panel = panel.clone(); + move |menu: PopupMenu, _window: &mut Window, _cx: &mut Context| { + let single = match listed.entry.changes.as_slice() { + [ + Change::Cell { + row, + column, + before, + .. + }, + ] => Some((*row, column.clone(), before.clone())), + _ => None, + }; + let (restore, name, unname) = + (listed.clone(), panel.clone(), panel.clone()); + let labelled = listed.label.is_some(); + menu.item(PopupMenuItem::new("Restore Project to Here…").on_click( + move |_, window, cx| Self::confirm_restore(&restore, window, cx), + )) + .when_some(single, |menu, (row, column, before)| { + menu.item(PopupMenuItem::new("Restore This Value").on_click( + move |_, _, cx| { + table::restore_value( + row, + &column, + before.clone().into(), + id, + cx, + ) + }, + )) + }) + .separator() + .item(PopupMenuItem::new("Name This Version…").on_click( + move |_, window, cx| { + name.update(cx, |this, cx| this.start_naming(id, window, cx)) + .ok(); + }, + )) + .when(labelled, |menu| { + menu.item(PopupMenuItem::new("Remove Name").on_click( + move |_, _, cx| { + unname.update(cx, |this, cx| this.name(id, None, cx)).ok(); + }, + )) + }) + } + }; + v_flex() + .id(ElementId::NamedInteger("history-entry".into(), id as u64)) + .h(height) + .w_full() + .gap_0p5() + .px_2() + .py_1() + .when(*indent, |row| row.pl_6()) + .border_b_1() + .border_color(border) + .hover(|row| row.bg(hover_bg)) + .when(matches!(entry.origin, Origin::Undo | Origin::Redo), |row| { + row.opacity(0.6) + }) + .when_some(listed.label.clone(), |row, label| { + row.child( + div() + .text_xs() + .font_weight(FontWeight::SEMIBOLD) + .text_color(accent) + .child(label), + ) + }) + .when_some(naming, |row, input| row.child(Input::new(&input).xsmall())) + .child(clipped(describe(entry, positions)).text_sm()) + .child( + div() + .text_xs() + .text_color(muted) + .child(meta(entry, when(&listed.day, &listed.time))), + ) + .context_menu(actions) + .into_any_element() } - let open = self.expanded.contains(&id); - let last = burst[burst.len() - 1]; - let changes: Vec = burst - .iter() - .flat_map(|l| l.entry.changes.iter().cloned()) - .collect(); - let summary = Entry { - changes, - ..head.entry.clone() - }; - let toggle = panel.clone(); - list = list.child( + + Row::Burst { ix, len } => { + let (Some(head), Some(last)) = (self.view.get(*ix), self.view.get(ix + len - 1)) + else { + return div().into_any_element(); + }; + let id = head.entry.id; + let open = self.expanded.contains(&id); + let summary = Entry { + changes: self.view[*ix..ix + len] + .iter() + .flat_map(|listed| listed.entry.changes.iter().cloned()) + .collect(), + ..head.entry.clone() + }; h_flex() .id(ElementId::NamedInteger("history-burst".into(), id as u64)) + .h(height) .w_full() .gap_1() .px_2() @@ -903,7 +904,7 @@ impl Render for HistoryPanel { .hover(|row| row.bg(hover_bg)) .cursor_pointer() .on_click(move |_, _, cx| { - toggle + panel .update(cx, |this, cx| { if !this.expanded.remove(&id) { this.expanded.insert(id); @@ -913,10 +914,9 @@ impl Render for HistoryPanel { .ok(); }) .child( - gpui_component::Icon::new(if open { - IconName::ChevronDown - } else { - IconName::ChevronRight + gpui_component::Icon::new(match open { + true => IconName::ChevronDown, + false => IconName::ChevronRight, }) .xsmall() .text_color(muted), @@ -926,43 +926,153 @@ impl Render for HistoryPanel { .min_w_0() .gap_0p5() .child( - clipped(format!( - "{} edits · {}", - burst.len(), - describe(&summary, &rows) - )) - .text_sm(), + clipped(format!("{len} edits · {}", describe(&summary, positions))) + .text_sm(), ) .child(div().text_xs().text_color(muted).child(meta( &head.entry, format!("{}, {}–{}", date_label(&head.day), last.time, head.time), ))), - ), - ); - if open { - for listed in burst { - list = list.child(entry_row( - row_id(listed), - &listed.entry, - Some(listed), - meta(&listed.entry, when(&listed.day, &listed.time)), - true, - )); + ) + .into_any_element() + } + } + } +} + +impl Render for HistoryPanel { + fn render(&mut self, _window: &mut Window, cx: &mut Context) -> impl IntoElement { + let theme = cx.theme(); + let (muted, border) = (theme.muted_foreground, theme.border); + let crop = cx.try_global::().map_or(px(0.), |c| c.0); + + let (positions, unsaved): (HashMap, Vec) = cx + .try_global::() + .and_then(|h| h.0.upgrade()) + .map(|state| { + let delegate = state.read(cx).delegate(); + ( + delegate + .row_ids() + .iter() + .enumerate() + .map(|(p, id)| (*id, p)) + .collect(), + delegate.unsaved_history().to_vec(), + ) + }) + .unwrap_or_default(); + let (filter, named_only) = (self.filter, self.named_only); + let admits = |entry: &Entry, label: bool| { + if named_only { + label + } else { + filter.admits(entry) + } + }; + + let cell = self.cell.as_ref(); + let unsaved: Vec = unsaved + .iter() + .rev() + .filter_map(|e| narrowed(e, cell).map(Cow::into_owned)) + .filter(|e| admits(e, false)) + .collect(); + // The times come back in one call rather than one per row, because every one of these is + // a query and the list below only formats what is on screen. + let ats: Vec = unsaved.iter().map(|entry| entry.at).collect(); + let times = settings::history::local_times(&ats).unwrap_or_else(|err| { + log::error!("couldn't read the local time of unsaved changes: {err}"); + Vec::new() + }); + let pending: Vec<(Entry, String)> = unsaved + .into_iter() + .enumerate() + .map(|(ix, entry)| { + let at = times.get(ix).map_or_else( + || "not saved".to_string(), + |(day, time)| format!("{}, not saved", when(day, time)), + ); + (entry, at) + }) + .collect(); + let view: Vec = self + .saved + .iter() + .filter_map(|l| match narrowed(&l.entry, cell)? { + Cow::Borrowed(_) => Some(Cow::Borrowed(l)), + Cow::Owned(entry) => Some(Cow::Owned(Listed { entry, ..l.clone() })), + }) + .filter(|l| admits(&l.entry, l.label.is_some())) + .map(Cow::into_owned) + .collect(); + let cell_label = cell.map(|(row, names)| { + let at = positions + .get(row) + .map_or_else(|| "a deleted row".to_string(), |p| format!("row {}", p + 1)); + format!("{}, {at}", names[0]) + }); + + // Flattened to one addressable line per row. Bursts — runs of adjacent, unnamed entries by + // one person, one means, one day — are found here and referred to by where they start. + let mut list: Vec = Vec::new(); + if !pending.is_empty() { + list.push(Row::Header("Not saved yet".into())); + list.extend((0..pending.len()).map(Row::Pending)); + } + let mut day: Option = None; + let mut ix = 0; + while ix < view.len() { + let head = &view[ix]; + let mut len = 1; + while let Some(next) = view.get(ix + len) { + let prev = &view[ix + len - 1]; + let joins = prev.label.is_none() + && next.label.is_none() + && prev.day == next.day + && prev.entry.author == next.entry.author + && prev.entry.origin == next.entry.origin + && matches!( + next.entry.origin, + Origin::Typed | Origin::Details | Origin::Paste | Origin::Clear + ) + && prev.entry.at - next.entry.at <= BURST_SECS; + if !joins { + break; } + len += 1; + } + if day.as_deref() != Some(head.day.as_str()) { + day = Some(head.day.clone()); + list.push(Row::Header(day_label(&head.day, head.days_ago).into())); } + match len { + 1 => list.push(Row::Saved { ix, indent: false }), + _ => { + let open = self.expanded.contains(&head.entry.id); + list.push(Row::Burst { ix, len }); + if open { + list.extend((ix..ix + len).map(|ix| Row::Saved { ix, indent: true })); + } + } + } + ix += len; } if self.more && !named_only { - list = list.child( - div().p_2().child( - Button::new("history-older") - .ghost() - .small() - .label("Load Older Changes") - .on_click(cx.listener(|this, _, _, cx| this.load_older(cx))), - ), - ); + list.push(Row::More); } + let empty = pending.is_empty() && view.is_empty(); + self.pending = Rc::new(pending); + self.view = Rc::new(view); + self.list = Rc::new(list); + let sizes = Rc::new( + self.list + .iter() + .map(|row| self.row_size(row)) + .collect::>(), + ); + v_flex() .size_full() .track_focus(&self.focus_handle) @@ -980,10 +1090,20 @@ impl Render for HistoryPanel { .border_b_1() .border_color(border) .child( - clipped(format!("Changes to {label}")) + div() + .id("history-cell") .flex_1() .min_w_0() - .text_xs(), + .overflow_hidden() + .text_ellipsis() + .text_xs() + .child(format!("Changes to {label}")) + .tooltip(move |window, cx| { + gpui_component::tooltip::Tooltip::new(format!( + "Changes to {label}" + )) + .build(window, cx) + }), ) .child( Button::new("history-cell-clear") @@ -999,7 +1119,38 @@ impl Render for HistoryPanel { ), ) }) - .child(list) + .when(empty, |panel| { + panel.child(div().p_3().text_sm().text_color(muted).child( + match (cx.has_global::(), named_only) { + (false, _) => "Open a project to see its history.", + (true, true) => "No named versions yet. Right-click a change to name it.", + (true, false) => { + "No changes yet. Every edit to this project's data and notes will be listed here." + } + }, + )) + }) + .when(!empty, |panel| { + panel + .child( + v_virtual_list(cx.entity(), "history-list", sizes, { + let positions = Rc::new(positions); + move |this, range, _window, cx| { + let list = this.list.clone(); + list[range] + .iter() + .map(|row| this.draw_row(row, &positions, cx)) + .collect::>() + } + }) + .pr_2() + .pb(px(8.) + crop) + .track_scroll(&self.list_scroll), + ) + .child(gpui_component::scroll::Scrollbar::vertical( + &self.list_scroll, + )) + }) } } From e87e5ab1166a456716ee3170dfde021cfdf438ce Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:31:14 -0700 Subject: [PATCH 13/16] feat(history): let a project cap how much of its change log it keeps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The log had no retention path at all: it only ever grew, and "Clear History…" was the single lever — all of it or none. A collection worked on for months has no way to bound its `.qrate` file short of throwing the whole audit trail away. Retention is off by default, because this log is an audit trail and deleting from it is a decision the archivist makes rather than one qrate makes for them. Set a limit and the oldest changes go, except that a named version is never dropped: a name is the archivist saying this is the entry to come back to, so the rule they set to bound a file must not be what takes it away. Named entries also still count towards the number kept, so turning a limit on can never cost more than the number it names. Pruning happens when the project opens, not as entries are written. The History panel holds the page it last read, and pruning underneath it would leave the list pointing at entries that are no longer there. --- crates/app/src/app_settings/mod.rs | 15 ++++ crates/project-wizard/src/project.rs | 13 ++++ crates/settings/src/history.rs | 105 ++++++++++++++++++++++++++- 3 files changed, 132 insertions(+), 1 deletion(-) diff --git a/crates/app/src/app_settings/mod.rs b/crates/app/src/app_settings/mod.rs index e6a64b1e..225b4ccd 100644 --- a/crates/app/src/app_settings/mod.rs +++ b/crates/app/src/app_settings/mod.rs @@ -97,6 +97,7 @@ pub fn build_pages(cx: &App) -> Vec { ) .group(saving_group(cx)) .group(notes_group(cx)) + .group(history_group(cx)) .group(previews_group(cx)), columns_page(cx), project_page(cx), @@ -168,6 +169,20 @@ fn notes_group(cx: &App) -> SettingGroup { ) } +/// How much of the change log a project keeps. Set per project as well as app-wide, because the +/// answer belongs to the collection being catalogued rather than to the machine. +fn history_group(cx: &App) -> SettingGroup { + divided_group(cx).title("History").item( + Setting::Dropdown { + key: settings::history::HISTORY_LIMIT_KEY, + label: "Changes to keep", + description: "The change log is an audit trail and keeps everything unless you say \n otherwise. A limit drops the oldest changes when the project is opened; \n named versions are never dropped.", + options: settings::history::HISTORY_LIMITS, + } + .into_item(cx), + ) +} + /// Every plugin on disk, running or not: whether it runs at all, what it is allowed to reach, and /// why it did not load when it did not. /// diff --git a/crates/project-wizard/src/project.rs b/crates/project-wizard/src/project.rs index 859a4a6f..34b89881 100644 --- a/crates/project-wizard/src/project.rs +++ b/crates/project-wizard/src/project.rs @@ -48,6 +48,19 @@ pub fn open_project(file: &Path, cx: &mut gpui::App) -> anyhow::Result { }; let name = project.display_name(); cx.set_global(project); + // Retention is applied on open rather than as entries are written: the History panel holds + // the page it last read, and pruning underneath it would leave the list pointing at entries + // that are no longer there. + if let Some(keep) = settings::history::limit(cx) { + match settings::history::prune(file, keep) { + Ok(0) => {} + Ok(gone) => log::info!("pruned {gone} history entries over the {keep} kept"), + Err(err) => log::warn!( + "couldn't apply this project's history retention, so its log still holds \ + everything: {err}" + ), + } + } Ok(name) } diff --git a/crates/settings/src/history.rs b/crates/settings/src/history.rs index 3a8c7083..ae373c3c 100644 --- a/crates/settings/src/history.rs +++ b/crates/settings/src/history.rs @@ -461,6 +461,62 @@ pub fn renames(path: &Path) -> Result> { .collect() } +/// How many entries a project keeps. Unset — or anything that isn't a positive number — keeps +/// every one, which is the default: this log is an audit trail, so nothing goes unless asked. +pub const HISTORY_LIMIT_KEY: &str = "history_limit"; + +/// What the Settings window offers for [`HISTORY_LIMIT_KEY`]. +pub const HISTORY_LIMITS: &[(&str, &str)] = &[ + ("", "Keep everything (default)"), + ("50000", "Keep the newest 50,000"), + ("10000", "Keep the newest 10,000"), + ("1000", "Keep the newest 1,000"), +]; + +/// The retention rule in force for the open project, or `None` to keep everything. +pub fn limit(cx: &App) -> Option { + if !cx.has_global::() { + return None; + } + crate::effective_text(HISTORY_LIMIT_KEY, cx) + .parse::() + .ok() + .filter(|keep| *keep > 0) +} + +/// Drop all but the newest `keep` entries, sparing every named version. Returns how many went. +/// +/// A name is the archivist saying this entry matters, so a rule set to bound a file's size does +/// not quietly take those with it — which is also why the count a caller logs can be lower than +/// the number over the limit. +pub fn prune(path: &Path, keep: i64) -> Result { + let conn = crate::project::open_rw(path)?; + let exists: i64 = conn.query_row( + "SELECT count(*) FROM sqlite_master WHERE type = 'table' AND name = '__history'", + [], + |r| r.get(0), + )?; + if exists == 0 { + return Ok(0); + } + // Named entries are spared twice over: they are never in the doomed set, and they still count + // towards the newest `keep`, so turning a rule on cannot cost more than the number it names. + let doomed = "SELECT id FROM __history WHERE label IS NULL + AND id NOT IN (SELECT id FROM __history ORDER BY id DESC LIMIT ?1)"; + conn.execute( + &format!("DELETE FROM __history_changes WHERE entry_id IN ({doomed})"), + [keep], + ) + .context("Prune history changes")?; + let gone = conn + .execute( + &format!("DELETE FROM __history WHERE id IN ({doomed})"), + [keep], + ) + .context("Prune history entries")?; + Ok(gone) +} + /// Name an entry — "Before Islandora ingest" — or, with `None`, take the name away again. pub fn set_label(path: &Path, id: EntryId, label: Option<&str>) -> Result<()> { crate::project::open_rw(path)? @@ -487,7 +543,7 @@ pub fn clear(path: &Path) -> Result<()> { mod tests { use super::{ Change, Entry, EntryId, Origin, clear, entries_after, former_names, local_times, page, - renames, set_label, + prune, renames, set_label, }; use crate::project::{ProjectSpec, create_project_file, save_dataset, write_notes}; @@ -661,6 +717,53 @@ mod tests { assert_eq!(moved.inverse().inverse(), moved); } + /// Retention keeps the newest entries and spares every named one, however old. A name is the + /// archivist marking that entry as the one to come back to, so a rule set to bound a file's + /// size must not be what takes it away — and the changes of a dropped entry go with it rather + /// than being left behind pointing at an id that no longer exists. + #[test] + fn pruning_keeps_the_newest_and_never_a_named_version() { + let path = project("prune.qrate"); + let headers = vec!["Title".to_string()]; + let log: Vec = (0..6) + .map(|n| { + Entry::new( + Origin::Typed, + vec![edit(&n.to_string(), &(n + 1).to_string())], + None, + ) + }) + .collect(); + save_dataset(&path, &headers, &[1], &[vec!["6".into()]], &log).unwrap(); + + // The oldest entry of the six, named — the one a retention rule would otherwise reach first. + set_label(&path, 1, Some("Before ingest")).unwrap(); + + let gone = prune(&path, 2).unwrap(); + assert_eq!(gone, 3, "six, less the newest two, less the named one"); + + let kept = page(&path, EntryId::MAX, 10, None).unwrap(); + let ids: Vec<_> = kept.iter().map(|l| l.entry.id).collect(); + assert_eq!( + ids, + vec![6, 5, 1], + "the newest two, and the named one below them" + ); + assert_eq!( + kept[2].label.as_deref(), + Some("Before ingest"), + "kept because it is named, not because it is recent" + ); + // Every surviving entry still has its own changes, and no dropped entry left any behind. + assert!( + kept.iter().all(|l| l.entry.changes.len() == 1), + "a pruned entry takes its changes with it" + ); + + // Nothing over the limit left to drop. + assert_eq!(prune(&path, 2).unwrap(), 0); + } + /// A page fetches every entry's changes in one query and hands each entry its own back. Two /// entries, each with several changes, is the arrangement that catches a distribution that /// mixes them up or loses the order they were written in — which a restore reads as the wrong From 23bf9341bcee96046b32531a47e95f7f2495cc7c Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 20 Sep 2026 22:59:54 -0700 Subject: [PATCH 14/16] refactor(history): convert timestamps in Rust instead of through SQLite MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Converting a unix timestamp to the archivist's local day and time needs timezone rules, and this crate had no date library — so it borrowed SQLite's calendar. Saved entries got theirs as three extra `localtime` columns on the page query; entries still in memory had no query to ride on, so `local_times` opened a brand-new empty in-memory database purely to do the arithmetic, once per timestamp, on every render. chrono is already in the tree through calamine with `clock` enabled, so one function converts both now and nothing new is compiled. The clock is read once per batch rather than once per row, the page query loses three columns, and a saved entry and an unsaved one agree about what day a change was made on because they are converted by the same code. An hour a clock change repeated resolves to its first reading rather than refusing to be a time; one a clock change skipped is shown as the UTC it was recorded as rather than not shown at all. --- Cargo.lock | 1 + Cargo.toml | 3 + crates/settings/Cargo.toml | 1 + crates/settings/src/history.rs | 89 ++++++++++++++++++-------- crates/workspace/src/panels/details.rs | 5 +- crates/workspace/src/panels/history.rs | 8 +-- 6 files changed, 71 insertions(+), 36 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 29efc64b..65b639bb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7620,6 +7620,7 @@ name = "settings" version = "0.5.0-beta.1" dependencies = [ "anyhow", + "chrono", "dirs 6.0.0", "gpui-component", "gpui-pre", diff --git a/Cargo.toml b/Cargo.toml index 4220c536..7e89af38 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -54,6 +54,9 @@ anyhow = "1" # Reads xlsx/xlsm/xlsb/xls/ods through one API, so the wizard gains every desktop # spreadsheet format for the price of one extension match. calamine = { version = "0.31", features = ["dates"] } +# Already in the tree through calamine, with `clock` on — so the change log converts its +# timestamps to local time in Rust rather than borrowing an in-memory SQLite for the calendar. +chrono = "0.4" csv = "1.3" # SPIKE: gpui-kit 0.6. Upstream moved off zed's git repo onto the crates.io `gpui-pre*` # snapshot, so these are renamed packages rather than new names in our source — every diff --git a/crates/settings/Cargo.toml b/crates/settings/Cargo.toml index 254cf09a..cde35a13 100644 --- a/crates/settings/Cargo.toml +++ b/crates/settings/Cargo.toml @@ -6,6 +6,7 @@ edition.workspace = true [dependencies] log.workspace = true anyhow.workspace = true +chrono.workspace = true dirs.workspace = true gpui.workspace = true gpui-component.workspace = true diff --git a/crates/settings/src/history.rs b/crates/settings/src/history.rs index ae373c3c..ee62281f 100644 --- a/crates/settings/src/history.rs +++ b/crates/settings/src/history.rs @@ -15,6 +15,7 @@ use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::{Context as _, Result}; +use chrono::{DateTime, Local, NaiveDate, TimeZone as _}; use gpui::App; use rusqlite::{Connection, params}; use serde::{Deserialize, Serialize}; @@ -314,15 +315,38 @@ pub fn entries_after(path: &Path, after: EntryId) -> Result> { .collect()) } -/// Each of `ats` (unix seconds) as a local `(YYYY-MM-DD, HH:MM)` — the same clock [`page`] reads -/// saved entries by, for changes that have not reached a file yet. -pub fn local_times(ats: &[i64]) -> Result> { - let conn = Connection::open_in_memory()?; - let mut stmt = conn.prepare( - "SELECT date(?1, 'unixepoch', 'localtime'), strftime('%H:%M', ?1, 'unixepoch', 'localtime')", - )?; +/// A unix timestamp as the History panel reads it: the local day, the local time, and how many +/// whole days back that day is from `today`. +/// +/// `today` is handed in because a page converts hundreds of these, reading the clock is the +/// expensive half, and the answer only changes at midnight. +fn parts(at: i64, today: NaiveDate) -> (String, String, i64) { + // An hour that a clock change repeated resolves to its first reading rather than refusing to + // be a time at all; one that a clock change skipped has no local reading, so it is shown as + // the UTC it was recorded as instead of not being shown. + let when = match Local.timestamp_opt(at, 0).earliest() { + Some(when) => when.naive_local(), + None => DateTime::from_timestamp(at, 0) + .unwrap_or_default() + .naive_utc(), + }; + ( + when.format("%Y-%m-%d").to_string(), + when.format("%H:%M").to_string(), + (today - when.date()).num_days(), + ) +} + +/// Each of `ats` (unix seconds) as a local `(YYYY-MM-DD, HH:MM)`, for changes that have not +/// reached a file yet. Entries read out of a file are converted by [`select`] as they are read, +/// through the same [`parts`], so the two agree about what day a change was made on. +pub fn local_times(ats: &[i64]) -> Vec<(String, String)> { + let today = Local::now().date_naive(); ats.iter() - .map(|at| Ok(stmt.query_row([at], |r| Ok((r.get(0)?, r.get(1)?)))?)) + .map(|at| { + let (day, time, _) = parts(*at, today); + (day, time) + }) .collect() } @@ -363,31 +387,25 @@ fn select(path: &Path, filter: &str, bound: &[&dyn rusqlite::ToSql]) -> Result(0)?, - r.get::<_, i64>(1)?, - r.get::<_, Option>(2)?, - r.get::<_, String>(3)?, - ), - ( - r.get::<_, Option>(4)?, - r.get::<_, String>(5)?, - r.get::<_, String>(6)?, - r.get::<_, i64>(7)?, - ), + r.get::<_, EntryId>(0)?, + r.get::<_, i64>(1)?, + r.get::<_, Option>(2)?, + r.get::<_, String>(3)?, + r.get::<_, Option>(4)?, )) })? .map(|row| { - let ((id, at, author, origin), (label, day, time, days_ago)) = row?; + let (id, at, author, origin, label) = row?; + let (day, time, days_ago) = parts(at, today); Ok(Listed { entry: Entry { id, @@ -640,7 +658,7 @@ mod tests { save_dataset(&path, &headers, &[1], &[vec!["5".into()]], &entries).unwrap(); let first = page(&path, EntryId::MAX, 2, None).unwrap(); - let (day, time) = local_times(&[first[0].entry.at]).unwrap().remove(0); + let (day, time) = local_times(&[first[0].entry.at]).remove(0); assert_eq!( (day.as_str(), time.as_str()), (first[0].day.as_str(), first[0].time.as_str()) @@ -717,6 +735,25 @@ mod tests { assert_eq!(moved.inverse().inverse(), moved); } + /// The History panel splits a day on `-` to name the month and prints the time straight + /// through, so the shape these come back in is load-bearing even though the values are + /// whatever the machine's own clock and zone say. SQLite used to guarantee it; now that both + /// a saved entry and an unsaved one are converted by the same Rust, nothing else would catch + /// the format drifting. + #[test] + fn a_local_timestamp_keeps_the_shape_the_panel_parses() { + let (day, time) = local_times(&[1_700_000_000]).remove(0); + assert_eq!(day.len(), 10, "expected YYYY-MM-DD, got {day}"); + let fields: Vec<&str> = day.split('-').collect(); + assert_eq!(fields.len(), 3, "expected YYYY-MM-DD, got {day}"); + assert!( + fields.iter().all(|f| f.chars().all(|c| c.is_ascii_digit())), + "expected YYYY-MM-DD, got {day}" + ); + assert_eq!(time.len(), 5, "expected HH:MM, got {time}"); + assert_eq!(&time[2..3], ":", "expected HH:MM, got {time}"); + } + /// Retention keeps the newest entries and spares every named one, however old. A name is the /// archivist marking that entry as the one to come back to, so a rule set to bound a file's /// size must not be what takes it away — and the changes of a dropped entry go with it rather diff --git a/crates/workspace/src/panels/details.rs b/crates/workspace/src/panels/details.rs index 20e2e88a..5e99255d 100644 --- a/crates/workspace/src/panels/details.rs +++ b/crates/workspace/src/panels/details.rs @@ -585,10 +585,7 @@ impl DetailsPanel { false => Vec::new(), true => { let ats: Vec = unsaved.iter().map(|entry| entry.at).collect(); - let times = settings::history::local_times(&ats).unwrap_or_else(|err| { - log::error!("couldn't read the local time of unsaved changes: {err}"); - Vec::new() - }); + let times = settings::history::local_times(&ats); unsaved .iter() .enumerate() diff --git a/crates/workspace/src/panels/history.rs b/crates/workspace/src/panels/history.rs index 0b9fa574..887a952d 100644 --- a/crates/workspace/src/panels/history.rs +++ b/crates/workspace/src/panels/history.rs @@ -978,13 +978,9 @@ impl Render for HistoryPanel { .filter_map(|e| narrowed(e, cell).map(Cow::into_owned)) .filter(|e| admits(e, false)) .collect(); - // The times come back in one call rather than one per row, because every one of these is - // a query and the list below only formats what is on screen. + // One call rather than one per row: it reads the clock once for the batch. let ats: Vec = unsaved.iter().map(|entry| entry.at).collect(); - let times = settings::history::local_times(&ats).unwrap_or_else(|err| { - log::error!("couldn't read the local time of unsaved changes: {err}"); - Vec::new() - }); + let times = settings::history::local_times(&ats); let pending: Vec<(Entry, String)> = unsaved .into_iter() .enumerate() From b51729ddcd8449f3fc9c3841e84f5602d0ff124f Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 20 Sep 2026 23:02:20 -0700 Subject: [PATCH 15/16] fix(ci): run clippy without zccache's path remapping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The lint step has been a no-op since the remap was configured. Cargo runs a linted crate as ` ` and clippy-driver copes by dropping the rustc path, but not once the remap has rewritten it — so the crate sees that path as a second input file and rustc stops with "multiple input filenames provided" on the first crate it reaches. A plain crate lints fine; every qrate worktree sets the remap, which is why this only shows up here. `[env]` in `.cargo/config.toml` does not force, so an empty value in the environment wins while zccache stays the wrapper and keeps caching. Only this step loses the cross-worktree sharing the remap buys; the test step below it, and every ordinary build, keep it. --- scripts/ci.sh | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/scripts/ci.sh b/scripts/ci.sh index 7f8a338d..39885019 100644 --- a/scripts/ci.sh +++ b/scripts/ci.sh @@ -6,5 +6,12 @@ set -euo pipefail export CARGO_TARGET_DIR="${CARGO_TARGET_DIR:-target/ci}" cargo fmt --all --check -cargo clippy --workspace --all-targets -- -D warnings -A dead_code +# Clippy runs without zccache's path remapping. Cargo runs a linted crate as +# ` `, and clippy-driver copes with that by dropping the +# rustc path — but not once the remap has rewritten it, so the crate sees that path as a second +# input file and stops with "multiple input filenames provided". `[env]` in `.cargo/config.toml` +# does not force, so an empty value here wins while zccache stays the wrapper and keeps caching. +# The cost is that this step's artifacts are not shared between worktrees; `cargo test` below +# keeps the remap, and so does every ordinary build. +ZCCACHE_PATH_REMAP= cargo clippy --workspace --all-targets -- -D warnings -A dead_code cargo test --workspace From 9ea194e5951c084ab179cf09a510d353d2b2d0d4 Mon Sep 17 00:00:00 2001 From: devnull03 <56480041+devnull03@users.noreply.github.com> Date: Sun, 20 Sep 2026 23:14:15 -0700 Subject: [PATCH 16/16] chore: drop PLAN.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One tracked path was holding a different plan on every branch, so each planning branch collided with main on it as a matter of course — three open PRs conflict on this file and nothing else. Nothing references it, and git history keeps every version that was ever in it. --- PLAN.md | 520 -------------------------------------------------------- 1 file changed, 520 deletions(-) delete mode 100644 PLAN.md diff --git a/PLAN.md b/PLAN.md deleted file mode 100644 index c90b5ffa..00000000 --- a/PLAN.md +++ /dev/null @@ -1,520 +0,0 @@ -# Grouped diagnostics and reusable value clustering - -## Implementation status - -The seven implementation phases are complete. The full cluster-review workspace remains future work in issue #125. - -- The Problems panel groups explicit producer metadata and retains atomic diagnostics. -- Expansion supports mouse and keyboard input. Occurrence rows retain exact-location navigation. -- Column validators use `ColumnFinding`. Notes, plugin scripts, and bridge protocol 2 keep their existing storage and wire formats. -- Spelling emits one finding per distinct observed word per cell, rather than one combined sentence per cell. -- The new `clustering` crate separates pure matching from diagnostics and fixes. -- Value-variant fixes verify the expected cell text. Each validation run clears the previous candidate snapshot. -- Group menus can fix all spelling or capitalization occurrences in one undo step. -- Connected value-variant groups can use any displayed member or save a distinct-cluster decision. -- Documentation describes scopes, counts, grouping keys, and OpenRefine credit. - -Implementation details that refine the original proposal: - -- Tab counts mean diagnostic occurrences, not unique affected locations. One cell can contain two words or several cluster members. -- Spelling keys retain the observed token and dictionary code. This avoids combining distinct displayed spellings or correction targets. -- Cluster keys retain the sorted exact displayed forms and the column. Normalization generates links, not automatic edits. -- A `begin_run` validator hook clears candidate state for deleted columns and changed projects. -- The table crate needs adapter and diagnostic-literal updates. Its production edit, navigation, and undo paths do not change. - -Validation covers pure grouping, 100 repeated spelling findings, all scopes, source and severity filters, -expansion persistence, mouse navigation, keyboard expansion, stale fixes, Unicode matching, connected clusters, -the 2,000-value comparison bound, and a real table fix followed by undo and revalidation. -UI checks use GPUI test windows, not an interactive review of a user's project. - -Final local gate: `scripts/ci.sh` passed on Windows. Formatting and workspace Clippy passed. -The workspace suite passed 375 tests, with 10 existing network or external-plugin tests ignored. - -## Purpose - -This branch will make repeated findings easier to review. It will also move value clustering out -of the spell checker. - -A spelling mistake can occur in 100 cells. The Problems panel now shows 100 independent rows. The -panel should show one problem group with an occurrence count. The user can expand the group and -jump to each affected location. - -Value variants have the same need. Several linked forms of one name are one review question, -not unrelated notes or separate pair decisions. - -This branch keeps review inline. A separate future task can add custom canonical text, manual -membership editing, row previews, and reusable mappings. - -## Completed baseline - -The branch already contains these spell-check upgrades: - -- qrate loads all installed dictionaries. -- Each value selects a dictionary only when the evidence is strong enough. -- Ambiguous and short unknown values do not default to English. -- Capitalization findings come from the `spell` source, with the same fixes and ignores as misspellings. -- Candidate proper names do not become ordinary spelling warnings. -- The opt-in `value variants` validator finds formatting, token order, diacritic, and close-spelling - differences. -- Existing fixes replace one whole cell through the undoable table edit path. - -These behaviors stay in place during the diagnostic and crate changes. - -## Root cause - -The diagnostic model stores one `Diagnostic` for one exact `Location`. This is correct for cell -markers, navigation, notes, fixes, and source replacement. - -The display model uses the same shape. `ProblemsPanel` converts each `Diagnostic` directly into one -visible row. It has no stable key that says several diagnostics describe the same problem. - -The validator boundary also limits built-in synchronous validators. `ColumnValidator::validate` -returns `(row, severity, message)`. It can report a cell in the current column, but it cannot report -a column-wide finding. - -`Location` itself already supports all required scopes: - -- dataset: no row and no column; -- column: a column and no row; -- row: a row and no column; -- cell: both a row and a column. - -The branch does not need another location representation. It needs a clear scope view, a richer -validator result, and a separate grouped panel projection. - -## Design rules - -1. Keep each diagnostic attached to its exact location. -2. Let a producer define group identity. Do not parse a human message. -3. Group only diagnostics from the same dataset, source, severity, and producer key. -4. Keep diagnostics without group metadata as independent rows. -5. Keep group aggregation in the diagnostics crate. -6. Keep matching algorithms out of the diagnostics crate. -7. Never treat text similarity as proof of entity identity. -8. Do not add bulk replacement in the first grouped-panel change. -9. Preserve source replacement and validator severity overrides. -10. Preserve one-cell fixes on expanded occurrence rows. - -## Proposed diagnostic contracts - -### Location scope - -Add a derived scope method instead of replacing `Location`: - -```rust -pub enum Scope<'a> { - Dataset, - Column(&'a str), - Row(usize), - Cell { row: usize, column: &'a str }, -} - -impl Location { - pub fn scope(&self) -> Scope<'_>; -} -``` - -This method gives the panel one exhaustive scope match. It does not migrate stored notes or change -the existing table lookup keys. - -Add constructors for new call sites: - -```rust -Location::dataset(dataset) -Location::column(dataset, column) -Location::row(dataset, row, row_id) -Location::cell(dataset, row, row_id, column) -``` - -Existing struct literals can migrate when their files change. This branch does not need a -repository-wide mechanical rewrite. - -### Diagnostic group - -Add optional producer metadata to `Diagnostic`: - -```rust -pub struct DiagnosticGroup { - pub key: SharedString, - pub summary: SharedString, -} - -pub struct Diagnostic { - pub location: Location, - pub severity: Severity, - pub source: Source, - pub message: SharedString, - pub group: Option, - pub filed: Option, -} -``` - -`key` is machine-readable and stable within one source. `summary` is the text for the collapsed -group row. - -Examples: - -| Source | Group key | Summary | -|---|---|---| -| `spell` | `en:recieve` | `“recieve” is misspelled` | -| `capitalization` | `alice→Alice` | `Use “Alice” instead of “alice”` | -| `value variants` | sorted exact cluster members | `3 values may be variants: formatting differs, accents or diacritics differ` | - -The panel groups by `(dataset, source, severity, group.key)`. This prevents an override from hiding -the severity of some occurrences inside a different tab. - -Authored notes remain ungrouped. Existing validators remain ungrouped until they supply a group. - -### Validator finding - -Replace the tuple return value with a named type: - -```rust -pub struct ColumnFinding { - pub row: Option, - pub severity: Severity, - pub message: SharedString, - pub group: Option, -} -``` - -`row: Some(row)` reports a cell. `row: None` reports the current column. - -The named type makes later fields possible without another tuple migration. It also makes the scope -of each result visible at its construction site. - -The plugin script contract does not change in this branch. `plugin-host` will translate its current -row findings into ungrouped `ColumnFinding` values. - -Row-wide and dataset-wide producers continue to publish addressed `Diagnostic` values. A -column-wise validator must not claim a row-wide result because it only owns one column snapshot. - -## Grouped panel projection - -Keep `Diagnostics::items` as the source of truth. Build a derived panel model during -`ProblemsPanel::refresh`. - -```rust -enum ProblemEntry { - Group { - id: ProblemGroupId, - severity: Severity, - summary: SharedString, - source: SharedString, - occurrences: Vec, - }, - Occurrence(Occurrence), -} -``` - -The implementation can use a flat visible list after it applies expansion state. This keeps -`uniform_list` and its fixed row height. - -### Collapsed group row - -A collapsed row will show: - -- the severity icon; -- the group summary; -- the source; -- an `N occurrences` badge; -- an expand control. - -Clicking the expand control will show the occurrence rows. The group row will not choose an -arbitrary location or apply a fix. - -### Occurrence row - -An occurrence row will keep the current behavior: - -- show `Row N · Column`, row, column, or dataset scope; -- jump to the location on click; -- offer the current one-cell spelling and fix menus; -- read the current cell text when the menu opens. - -Column, row, and dataset occurrences have no cell text. Their context menus will omit cell fixes. - -### Counts and filters - -Severity tabs keep diagnostic occurrence totals. `Warnings (100)` means 100 warning findings, -which can include more than one distinct problem at the same location. - -The visible list will show fewer top-level rows after grouping. Each group badge will state its own -occurrence count. - -Source filtering runs before tab counts and grouping. Severity filtering runs before grouping. -Each severity tab keeps its own total regardless of the active tab. - -### Expansion state - -Store expanded `ProblemGroupId` values in `ProblemsPanel`. Clear keys that no longer exist after a -diagnostic refresh. - -Derive the ID from dataset, source, severity, and producer group key. Do not use the visible summary -or an item index. - -## Reusable clustering crate - -Create `crates/clustering`. - -The crate will own: - -- Unicode token normalization; -- strict, sorted, and diacritic-insensitive keys; -- candidate blocking; -- normalized Damerau-Levenshtein comparison; -- value frequencies and source rows; -- pair evidence and stable pair keys; -- connected review clusters and stable member-set keys; -- the `value variants` validator; -- its one-cell fix provider. - -The crate will depend on: - -- `caseless`; -- `unicode-normalization`; -- `strsim`; -- `diagnostics`; -- `settings`; -- `gpui` for validator registration state and fix lookup. - -The core matching module must stay free of GPUI types. The validator adapter can convert core -results into `SharedString`, diagnostics, and fixes. - -Move these dependencies out of `spellcheck`: - -- `caseless`; -- `unicode-normalization`; -- `strsim`. - -After the move, `spellcheck` will own dictionaries, language selection, spelling, capitalization, -and custom words. It will not own general value comparison. - -The app will register `clustering::ValueVariants` independently from the spell checker. The source -name remains `value variants` so existing severity settings continue to work. - -## Value-variant grouping - -The matcher returns evidence pairs. The review adapter treats each pair as a graph link and shows -each connected component as one cluster. - -For each cluster, the clustering crate returns: - -- every exact displayed value; -- the rows for each value; -- the matching reasons; -- a stable key from the sorted member set. - -All diagnostics for a cluster use the same `DiagnosticGroup`. The resolver offers every member as -a canonical choice and applies the result as one undoable edit. The user can instead mark all -current members as distinct. Similarity never changes data without that explicit choice. - -The future cluster-review workspace can add custom canonical text, member editing, row previews, -and reusable mappings. - -## Files and crates affected - -### `crates/diagnostics` - -- Add `DiagnosticGroup`, `ColumnFinding`, and the derived `Scope`. -- Update `address` and `ColumnValidator`. -- Add a pure panel projection that groups filtered diagnostics. -- Add expansion state and grouped rows to `ProblemsPanel`. -- Keep `Diagnostics::set`, `Diagnostics::at`, and the row index atomic. -- Test grouping, counts, filters, scopes, expansion, navigation, and fixes. - -### `crates/spellcheck` - -- Return `ColumnFinding` from spelling and capitalization validators. -- Add stable group keys for repeated spelling and capitalization findings. -- Remove value-variant code and general comparison dependencies. -- Keep dictionary behavior unchanged. - -### `crates/clustering` - -- Receive the current value-variant implementation and tests. -- Separate the pure matcher from the diagnostic adapter. -- Return pair evidence, connected clusters, frequencies, rows, and stable keys. -- Publish grouped value-variant findings. - -### `crates/checks` - -- Return `ColumnFinding` from date checks. -- Convert authority results to the named type. -- Leave findings ungrouped unless the rule has a clear stable subject. - -### `crates/plugin-host` - -- Adapt plugin validator output to `ColumnFinding`. -- Keep the public Luau result format unchanged. -- Leave plugin findings ungrouped in this phase. - -### `crates/app` - -- Register the value-variant validator and fixes from `clustering`. -- Remove the spell-check ownership link. - -### Workspace and documentation - -- Add `crates/clustering` to the workspace. -- Update `Cargo.lock`. -- Update `CLAUDE.md` crate map. -- Update `docs/diagnostics.md`. -- Add OpenRefine credit to the diagnostic documentation. - -### Existing integration points - -- `table` supplies one-cell fix hooks and exact location navigation. Its adapters need the new diagnostic types. -- `workspace` already hosts `ProblemsPanel`. -- `settings` already stores the opt-in value-variant setting. -- `plugin-api` does not expose grouped diagnostic metadata yet. - -## Implementation sequence - -### 1. Pin current behavior - -- Add tests for 100 identical spelling findings. -- Add tests for connected value variants that occur in many rows. -- Add tests for dataset, column, row, and cell labels. -- Confirm that fixes query the current cell value. - -### 2. Add the diagnostic contracts - -- Add `DiagnosticGroup`. -- Add `ColumnFinding`. -- Add `Location::scope` and constructors. -- Update built-in validator implementations. -- Keep plugin output compatible through its adapter. -- Confirm that source replacement still clears stale findings. - -### 3. Add the pure group projection - -- Group only diagnostics with explicit metadata. -- Keep ungrouped diagnostics independent. -- Calculate occurrence totals and top-level group totals. -- Filter before grouping. -- Sort groups by severity, summary, and stable key. -- Sort occurrences by table location. -- Test mixed scopes and mixed severity overrides. - -### 4. Render grouped diagnostics - -- Render collapsed groups with occurrence badges. -- Expand groups inline. -- Keep the list virtualized. -- Keep navigation and context menus on occurrence rows. -- Add keyboard and accessibility labels to expansion controls. -- Remove stale expansion keys after revalidation. - -### 5. Teach spelling to group - -- Group repeated misspellings by language and normalized token. -- Group capitalization findings by observed and canonical forms. -- Keep different replacement targets in different groups. -- Confirm that 100 repeated mistakes produce one collapsed row and 100 occurrences. - -### 6. Extract value clustering - -- Create the `clustering` crate. -- Move the matcher without changing thresholds. -- Split pure matching from the GPUI adapter. -- Publish one group for each connected value cluster. -- Offer every member as a canonical group fix. -- Run the existing 2,000-value blocking test in the new crate. - -### 7. Document and validate - -- Document all four diagnostic scopes. -- Document group identity and count semantics. -- Credit OpenRefine as a design influence. -- Run focused tests after each crate change. -- Run `./scripts/ci.sh` before the branch is ready for review. -- Test expansion, filtering, navigation, fixes, undo, and revalidation in the app. - -## OpenRefine credit and license boundary - -OpenRefine influenced the proposed review flow and the strict-to-broad clustering order: - -- OpenRefine, “Cluster and edit”: - -- OpenRefine, “Clustering Methods In-depth”: - -- OpenRefine source: - - -OpenRefine documentation uses CC BY 4.0. OpenRefine source code uses the BSD 3-Clause license. - -This branch will independently implement the algorithms from their published descriptions and -standard algorithm references. It will not copy OpenRefine source code, UI assets, or documentation -text. - -Add this acknowledgment to `docs/diagnostics.md`: - -> qrate's value-clustering workflow is inspired by OpenRefine's Cluster and edit feature. qrate -> uses an independent implementation designed for its diagnostics and cataloging workflow. - -Link both feature names to the OpenRefine documentation. Do not imply that OpenRefine endorses -qrate. - -This change credits design influence and uses an independent implementation. It does not distribute OpenRefine material. -If future work copies or adapts upstream material, review its actual license and add the required notices. - -If later work copies or adapts OpenRefine code: - -1. Record the exact upstream file and commit. -2. Keep the upstream copyright notice. -3. Add the BSD 3-Clause license text to `NOTICES`. -4. Mark the adapted qrate file. -5. Keep documentation excerpts under CC BY 4.0 with title, author, source, license, and change notes. - -## Future issue: cluster review workspace - -Track the full workspace in [GitHub issue #125](https://github.com/devnull03/qrate/issues/125). - -Track this outside the current implementation: - -- a dedicated cluster review workspace; -- value frequencies and affected-row counts; -- a custom canonical value; -- member inclusion and exclusion; -- sample row and thumbnail context; -- match evidence and strategy controls; -- skip and reject judgments; -- richer decision management and export; -- bulk replacement across a user-edited cluster; -- re-cluster after an accepted change; -- value-frequency facets; -- authority reconciliation candidates; -- mapping export and reuse. - -This issue should cite OpenRefine as a design influence. It should state that qrate will use an -independent implementation. - -## Non-goals - -- Do not build the full cluster-review workspace on this branch. -- Do not add phonetic, n-gram, PPM, or authority clustering on this branch. -- Do not merge similar values automatically. -- Do not group diagnostics by message text. -- Do not replace atomic diagnostics with aggregate records. -- Do not change stored note schema. -- Do not change the plugin API. - -## Definition of done - -- Repeated equivalent findings appear as one collapsed group. -- Each group shows its occurrence count. -- A user can expand a group and jump to every exact location. -- Existing one-cell fixes work from expanded occurrences. -- Group fixes use one table edit, one validation pass, and one undo step. -- A user can save all current cluster members as distinct for one column. -- Dataset, column, row, and cell scopes have clear labels. -- Source and severity filters produce correct groups and counts. -- Spelling and capitalization emit stable group metadata. -- Value clustering lives outside `spellcheck`. -- Value-variant clusters emit stable group metadata. -- Similarity never causes an automatic data change. -- Existing spell-check and value-variant behavior stays intact. -- Tests cover grouping, scopes, filters, fixes, revalidation, and comparison bounds. -- The documentation credits OpenRefine without claiming affiliation. -- The full workspace test and CI script pass.