diff --git a/src/documents/blocks/content.rs b/src/documents/blocks/content.rs index 77506e9..35e8afc 100644 --- a/src/documents/blocks/content.rs +++ b/src/documents/blocks/content.rs @@ -61,7 +61,12 @@ impl BlockEditor { let check = Checkbox::new((id.clone(), format!("todo-{key}")), done).on_change( move |on, _, cx| { entity.update(cx, |editor, cx| { - editor.set_kind(key, BlockKind::Todo(on), cx) + if editor + .kind_of(key) + .is_some_and(|kind| matches!(kind, BlockKind::Todo(_))) + { + editor.set_kind(key, BlockKind::Todo(on), cx) + } }) }, ); @@ -106,7 +111,9 @@ impl BlockEditor { .on_mouse_down(MouseButton::Left, |_, window, _| window.prevent_default()) .on_click(move |_, _, cx| { entity.update(cx, |editor, cx| { - editor.set_kind(key, BlockKind::Toggle(!open), cx) + if editor.kind_of(key) == Some(&BlockKind::Toggle(open)) { + editor.set_kind(key, BlockKind::Toggle(!open), cx) + } }) }) .child(Disclosure::new( diff --git a/src/documents/blocks/edit.rs b/src/documents/blocks/edit.rs index 5ba9ab7..39cbb78 100644 --- a/src/documents/blocks/edit.rs +++ b/src/documents/blocks/edit.rs @@ -7,7 +7,9 @@ use super::{Block, BlockData, BlockEditor, BlockEvent, BlockKind, Snapshot}; impl BlockEditor { /// Splits a prose block at the caret: the text after it moves to a new block of the kind that follows. pub(crate) fn split(&mut self, key: u64, window: &mut Window, cx: &mut Context) { - let ix = self.index(key); + let Some(ix) = self.index(key) else { + return; + }; let kind = self.blocks[ix].kind.clone(); let field = self.blocks[ix].fields[0].clone(); if field.read(cx).is_empty() @@ -43,7 +45,9 @@ impl BlockEditor { /// Backspace at a prose block's start: another kind turns back into text; text joins the prose above, or an empty block goes. pub(crate) fn join(&mut self, key: u64, window: &mut Window, cx: &mut Context) { - let ix = self.index(key); + let Some(ix) = self.index(key) else { + return; + }; if self.blocks[ix].kind != BlockKind::Paragraph { self.turn_into(key, BlockKind::Paragraph, window, cx); return; diff --git a/src/documents/blocks/keys.rs b/src/documents/blocks/keys.rs index 41e3cfb..91d67ea 100644 --- a/src/documents/blocks/keys.rs +++ b/src/documents/blocks/keys.rs @@ -9,6 +9,35 @@ fn paragraph() -> BlockData { } impl BlockEditor { + /// Where `key` sits, logged and none when the block has left. + pub(super) fn found(&self, key: u64, what: &str) -> Option { + let at = self.blocks.iter().position(|block| block.key == key); + if at.is_none() { + log::error!("block editor: no block {key} to {what}"); + } + at + } + + /// Where `key` sits now; a listener painted for a block since gone finds none. + pub(crate) fn index(&self, key: u64) -> Option { + self.found(key, "act on") + } + + /// What `key` is now; a box painted for it acts only while it still is that. + pub(crate) fn kind_of(&self, key: u64) -> Option<&BlockKind> { + Some(&self.blocks[self.index(key)?].kind) + } + + /// The block holding `key`'s field `field` now, when it still has that field. + pub(crate) fn place(&self, key: u64, field: usize) -> Option { + let ix = self.index(key)?; + let held = field < self.blocks[ix].fields.len(); + if !held { + log::error!("block editor: block {key} has no field {field} now"); + } + held.then_some(ix) + } + /// The fields in reading order, as `(block, field)`: a closed toggle hides its body. fn reading_order(&self) -> Vec<(u64, usize)> { self.blocks @@ -40,7 +69,10 @@ impl BlockEditor { window: &mut Window, cx: &mut Context, ) -> bool { - let kind = self.blocks[self.index(key)].kind.clone(); + let Some(ix) = self.place(key, field) else { + return true; + }; + let kind = self.blocks[ix].kind.clone(); match kind { kind if kind.is_prose() => self.split(key, window, cx), BlockKind::Toggle(open) if field == 0 => { @@ -54,7 +86,7 @@ impl BlockEditor { } BlockKind::Table { columns, merged } => { let below = field + columns; - if below >= self.blocks[self.index(key)].fields.len() { + if below >= self.blocks[ix].fields.len() { self.add_row(key, window, cx); } let shown = shown_cell(&merged, columns, below); @@ -73,7 +105,10 @@ impl BlockEditor { window: &mut Window, cx: &mut Context, ) -> bool { - let block = &self.blocks[self.index(key)]; + let Some(ix) = self.place(key, field) else { + return true; + }; + let block = &self.blocks[ix]; let input = block.fields[field].read(cx); if input.cursor() != 0 || !input.selection().is_empty() { return false; @@ -101,7 +136,10 @@ impl BlockEditor { window: &mut Window, cx: &mut Context, ) -> bool { - let block = &self.blocks[self.index(key)]; + let Some(ix) = self.place(key, field) else { + return true; + }; + let block = &self.blocks[ix]; if !at_edge(block.fields[field].read(cx), down) { return false; } @@ -118,10 +156,10 @@ impl BlockEditor { } } let order = self.reading_order(); - let at = order - .iter() - .position(|place| *place == (key, field)) - .expect("a focused field is in reading order"); + let Some(at) = order.iter().position(|place| *place == (key, field)) else { + log::error!("block editor: field {field} of block {key} is not shown now"); + return true; + }; let target = if down { order[at + 1..].iter().find(|(other, _)| *other != key) } else { @@ -134,7 +172,10 @@ impl BlockEditor { } return false; }; - let end = self.blocks[self.index(target)].fields[target_field] + let Some(found) = self.index(target) else { + return true; + }; + let end = self.blocks[found].fields[target_field] .read(cx) .text() .len(); diff --git a/src/documents/blocks/media.rs b/src/documents/blocks/media.rs index af07391..626a32a 100644 --- a/src/documents/blocks/media.rs +++ b/src/documents/blocks/media.rs @@ -43,28 +43,44 @@ pub(crate) fn share(align: Align, column: Bounds, x: Pixels) -> f32 { (reach / width).clamp(NARROWEST, 1.0) } -fn media_of(kind: &mut BlockKind) -> &mut Media { +/// A media block's media; a block turned into something else since its controls painted has none. +fn media_of(kind: &mut BlockKind) -> Option<&mut Media> { match kind { - BlockKind::Image(media) | BlockKind::Video(media) | BlockKind::Embed(media) => media, - other => unreachable!("{} holds no media", other.label()), + BlockKind::Image(media) | BlockKind::Video(media) | BlockKind::Embed(media) => Some(media), + other => { + log::error!("block editor: a {} block holds no media now", other.label()); + None + } } } impl BlockEditor { /// Sets a picture's width while its edge is dragged; the drag's start records undo. - fn resize(&mut self, key: u64, width: f32, first: bool, cx: &mut Context) { + pub(super) fn resize(&mut self, key: u64, width: f32, first: bool, cx: &mut Context) { + let Some(ix) = self.index(key) else { + return; + }; + let mut kind = self.blocks[ix].kind.clone(); + let Some(media) = media_of(&mut kind) else { + return; + }; + media.width = width; if first { self.before_change(cx); } - let ix = self.index(key); - media_of(&mut self.blocks[ix].kind).width = width; + self.blocks[ix].kind = kind; self.after_change(cx); } pub(crate) fn align(&mut self, key: u64, align: Align, cx: &mut Context) { - let ix = self.index(key); + let Some(ix) = self.index(key) else { + return; + }; let mut kind = self.blocks[ix].kind.clone(); - media_of(&mut kind).align = align; + let Some(media) = media_of(&mut kind) else { + return; + }; + media.align = align; self.set_kind(key, kind, cx); } } diff --git a/src/documents/blocks/mod.rs b/src/documents/blocks/mod.rs index 64a8744..c7c1e40 100644 --- a/src/documents/blocks/mod.rs +++ b/src/documents/blocks/mod.rs @@ -228,7 +228,9 @@ impl BlockEditor { window: &mut Window, cx: &mut Context, ) { - let ix = self.index(key); + let Some(ix) = self.index(key) else { + return; + }; let field = Self::field(&self.blocks[ix].kind, "", window, cx); let watched = Self::watch(key, &field, window, cx); let block = &mut self.blocks[ix]; @@ -238,19 +240,14 @@ impl BlockEditor { } pub(crate) fn remove_field(&mut self, key: u64, field: usize) { - let ix = self.index(key); + let Some(ix) = self.index(key) else { + return; + }; let block = &mut self.blocks[ix]; block.fields.remove(field); drop(block._changes.remove(field)); } - pub(crate) fn index(&self, key: u64) -> usize { - self.blocks - .iter() - .position(|block| block.key == key) - .unwrap_or_else(|| panic!("block {key} left the editor")) - } - fn on_field( &mut self, key: u64, @@ -259,7 +256,9 @@ impl BlockEditor { window: &mut Window, cx: &mut Context, ) { - let ix = self.index(key); + let Some(ix) = self.index(key) else { + return; + }; match event { InputEvent::Changed => { let typed = field.focus_handle(cx).is_focused(window); @@ -342,8 +341,10 @@ impl BlockEditor { window: &mut Window, cx: &mut App, ) { - let block = &self.blocks[self.index(key)]; - let Some(field) = block.fields.get(field) else { + let Some(ix) = self.index(key) else { + return; + }; + let Some(field) = self.blocks[ix].fields.get(field) else { return; }; if let Some(at) = at { @@ -360,10 +361,15 @@ impl BlockEditor { window: &mut Window, cx: &mut Context, ) -> u64 { + let at = match after { + None => 0, + Some(after) => self + .found(after, "insert after") + .map_or(self.blocks.len(), |ix| ix + 1), + }; self.before_change(cx); let key = self.fresh_key(); let block = self.build(data, key, window, cx); - let at = after.map_or(0, |after| self.index(after) + 1); log::info!("block editor: {} block at {at}", block.kind.label()); let end = block .fields @@ -383,8 +389,10 @@ impl BlockEditor { window: &mut Window, cx: &mut Context, ) { + let Some(ix) = self.found(key, "turn") else { + return; + }; self.before_change(cx); - let ix = self.index(key); let first = self.blocks[ix] .fields .first() @@ -403,7 +411,9 @@ impl BlockEditor { /// Changes a block's kind in place, as a to-do's box or a toggle's state does. pub fn set_kind(&mut self, key: u64, kind: BlockKind, cx: &mut Context) { - let ix = self.index(key); + let Some(ix) = self.found(key, "set the kind of") else { + return; + }; assert!( kind.accepts(self.blocks[ix].fields.len()), "set_kind keeps the fields; turn_into changes them" @@ -414,8 +424,10 @@ impl BlockEditor { } pub fn remove(&mut self, key: u64, window: &mut Window, cx: &mut Context) { + let Some(ix) = self.found(key, "remove") else { + return; + }; self.before_change(cx); - let ix = self.index(key); log::info!("block editor: block {key} removed"); self.blocks.remove(ix); self.after_change(cx); @@ -430,13 +442,22 @@ impl BlockEditor { } pub fn duplicate(&mut self, key: u64, window: &mut Window, cx: &mut Context) { - let ix = self.index(key); + let Some(ix) = self.found(key, "duplicate") else { + return; + }; let data = self.snapshot(cx).blocks[ix].1.clone(); self.insert(Some(key), data, window, cx); } /// Moves the block at `from` to place `to`. pub fn move_block(&mut self, from: usize, to: usize, cx: &mut Context) { + if from >= self.blocks.len() { + log::error!( + "block editor: no block at {from} of {} to move", + self.blocks.len() + ); + return; + } self.before_change(cx); log::info!("block editor: block moved from {from} to {to}"); let block = self.blocks.remove(from); diff --git a/src/documents/blocks/table.rs b/src/documents/blocks/table.rs index 1bb5695..753f9d7 100644 --- a/src/documents/blocks/table.rs +++ b/src/documents/blocks/table.rs @@ -10,12 +10,16 @@ use crate::{ theme::ControlSize, }; -/// A table's shape: columns, rows, and the cells that fold into their left neighbor. -fn shape(kind: &BlockKind, fields: usize) -> (usize, usize, &[(usize, usize)]) { +/// Columns, rows, and the cells folded into their left neighbor. +type Shape<'a> = (usize, usize, &'a [(usize, usize)]); + +/// A table's shape; none once the block is no table. +fn shape(kind: &BlockKind, fields: usize) -> Option> { let BlockKind::Table { columns, merged } = kind else { - unreachable!("a table block is a table") + log::error!("block editor: a {} block is no table now", kind.label()); + return None; }; - (*columns, fields / columns, merged) + Some((*columns, fields / columns, merged)) } /// How many columns the cell at `(row, column)` spans: itself and every cell folded into it. @@ -55,8 +59,13 @@ pub(crate) fn without_row(merged: &[(usize, usize)], row: usize) -> Vec<(usize, impl BlockEditor { /// Adds a row of empty cells at the table's foot. pub(crate) fn add_row(&mut self, key: u64, window: &mut Window, cx: &mut Context) { - let ix = self.index(key); - let (columns, _, _) = shape(&self.blocks[ix].kind, self.blocks[ix].fields.len()); + let Some(ix) = self.index(key) else { + return; + }; + let Some((columns, _, _)) = shape(&self.blocks[ix].kind, self.blocks[ix].fields.len()) + else { + return; + }; self.before_change(cx); for _ in 0..columns { self.add_field(key, None, window, cx); @@ -67,8 +76,14 @@ impl BlockEditor { /// Adds a column of empty cells at the table's right. pub(crate) fn add_column(&mut self, key: u64, window: &mut Window, cx: &mut Context) { - let ix = self.index(key); - let (columns, rows, merged) = shape(&self.blocks[ix].kind, self.blocks[ix].fields.len()); + let Some(ix) = self.index(key) else { + return; + }; + let Some((columns, rows, merged)) = + shape(&self.blocks[ix].kind, self.blocks[ix].fields.len()) + else { + return; + }; let merged = merged.to_vec(); self.before_change(cx); for row in (0..rows).rev() { @@ -90,8 +105,18 @@ impl BlockEditor { column: bool, cx: &mut Context, ) { - let ix = self.index(key); - let (columns, rows, merged) = shape(&self.blocks[ix].kind, self.blocks[ix].fields.len()); + let Some(ix) = self.index(key) else { + return; + }; + let Some((columns, rows, merged)) = + shape(&self.blocks[ix].kind, self.blocks[ix].fields.len()) + else { + return; + }; + if cell >= self.blocks[ix].fields.len() { + log::error!("table {key}: cell {cell} left the table"); + return; + } let merged = merged.to_vec(); let (row, at) = (cell / columns, cell % columns); if (column && columns == 1) || (!column && rows <= 2) { @@ -129,8 +154,17 @@ impl BlockEditor { /// Folds the cell right of `cell` into it, moving its text over; or, when already merged, parts them again. pub(crate) fn merge_right(&mut self, key: u64, cell: usize, cx: &mut Context) { - let ix = self.index(key); - let (columns, _, merged) = shape(&self.blocks[ix].kind, self.blocks[ix].fields.len()); + let Some(ix) = self.index(key) else { + return; + }; + let Some((columns, _, merged)) = shape(&self.blocks[ix].kind, self.blocks[ix].fields.len()) + else { + return; + }; + if cell >= self.blocks[ix].fields.len() { + log::error!("table {key}: cell {cell} left the table"); + return; + } let (row, column) = (cell / columns, cell % columns); let spanned = span(merged, columns, row, column); if column + spanned >= columns && spanned == 1 { @@ -168,7 +202,8 @@ pub(super) fn table( window: &mut Window, cx: &mut Context, ) -> AnyElement { - let (columns, rows, merged) = shape(&block.kind, block.fields.len()); + let (columns, rows, merged) = + shape(&block.kind, block.fields.len()).expect("a table block draws as a table"); let merged = merged.to_vec(); let (key, id) = (block.key, BlockEditor::id(cx)); let focused = editor diff --git a/src/documents/blocks/tests.rs b/src/documents/blocks/tests/mod.rs similarity index 99% rename from src/documents/blocks/tests.rs rename to src/documents/blocks/tests/mod.rs index c0584c0..f0a3ef4 100644 --- a/src/documents/blocks/tests.rs +++ b/src/documents/blocks/tests/mod.rs @@ -3,6 +3,8 @@ use gpui::{AppContext as _, Entity, Focusable, TestAppContext, VisualTestContext use super::{BlockData, BlockEditor, BlockKind}; use crate::{forms, theme::Theme}; +mod stale; + fn open( blocks: Vec, cx: &mut TestAppContext, diff --git a/src/documents/blocks/tests/stale.rs b/src/documents/blocks/tests/stale.rs new file mode 100644 index 0000000..ae17f92 --- /dev/null +++ b/src/documents/blocks/tests/stale.rs @@ -0,0 +1,141 @@ +use gpui::{Focusable, TestAppContext}; + +use super::{blocks, caret, open, settle, text}; +use crate::{ + documents::blocks::{Align, BlockData, BlockKind, Media}, + forms, +}; + +#[gpui::test] +fn a_key_the_user_deleted_changes_nothing(cx: &mut TestAppContext) { + let paragraph = |text: &str| BlockData::new(BlockKind::Paragraph, [text]); + let (editor, cx) = open(vec![paragraph("one"), paragraph("two")], cx); + cx.update(|window, cx| { + editor.update(cx, |editor, cx| { + let gone = 999; + editor.set_kind(gone, BlockKind::Bullet, cx); + editor.turn_into(gone, BlockKind::Bullet, window, cx); + editor.remove(gone, window, cx); + editor.duplicate(gone, window, cx); + editor.move_block(9, 0, cx); + editor.insert(Some(gone), paragraph("three"), window, cx); + }) + }); + let texts: Vec = editor.read_with(cx, |editor, cx| { + editor + .blocks(cx) + .into_iter() + .map(|block| block.texts[0].clone()) + .collect() + }); + assert_eq!( + texts, + ["one", "two", "three"], + "an insert after a gone block lands at the end" + ); +} + +#[gpui::test] +fn keys_in_a_field_whose_block_left_change_nothing(cx: &mut TestAppContext) { + let paragraph = |text: &str| BlockData::new(BlockKind::Paragraph, [text]); + let (editor, cx) = open(vec![paragraph("one"), paragraph("two")], cx); + caret(&editor, 0, 0, cx); + cx.update(|window, cx| { + let key = editor.read(cx).blocks[0].key; + editor.update(cx, |editor, cx| editor.remove(key, window, cx)); + window.dispatch_action(Box::new(forms::Enter), cx); + }); + settle(cx); + assert_eq!(blocks(&editor, cx), [text(BlockKind::Paragraph, "two")]); +} + +#[gpui::test] +fn up_from_a_body_folded_away_changes_nothing(cx: &mut TestAppContext) { + let toggle = BlockData::new(BlockKind::Toggle(true), ["title", "body"]); + let (editor, cx) = open(vec![toggle], cx); + cx.update(|window, cx| { + let body = editor.read(cx).blocks[0].fields[1].clone(); + window.focus(&body.focus_handle(cx), cx); + }); + settle(cx); + cx.update(|window, cx| { + let key = editor.read(cx).blocks[0].key; + editor.update(cx, |editor, cx| { + editor.set_kind(key, BlockKind::Toggle(false), cx) + }); + window.dispatch_action(Box::new(forms::Up), cx); + }); + settle(cx); + assert_eq!(blocks(&editor, cx)[0].0, BlockKind::Toggle(false)); +} + +#[gpui::test] +fn table_and_media_tools_on_a_block_turned_paragraph_change_nothing(cx: &mut TestAppContext) { + let table = BlockKind::Table { + columns: 2, + merged: vec![], + }; + let image = BlockKind::Image(Media::new("missing.png")); + let (editor, cx) = open( + vec![ + BlockData::new(table, ["h1", "h2", "a", "b"]), + BlockData::new(image, ["caption"]), + ], + cx, + ); + cx.update(|window, cx| { + editor.update(cx, |editor, cx| { + let (table, image) = (editor.blocks[0].key, editor.blocks[1].key); + editor.turn_into(table, BlockKind::Paragraph, window, cx); + editor.turn_into(image, BlockKind::Paragraph, window, cx); + editor.add_row(table, window, cx); + editor.add_column(table, window, cx); + editor.remove_line(table, 0, true, cx); + editor.merge_right(table, 0, cx); + editor.align(image, Align::Left, cx); + }) + }); + settle(cx); + let kinds: Vec = blocks(&editor, cx) + .into_iter() + .map(|(kind, _)| kind) + .collect(); + assert_eq!(kinds, [BlockKind::Paragraph, BlockKind::Paragraph]); +} + +#[gpui::test] +fn a_cell_whose_row_left_is_not_removed_or_merged_again(cx: &mut TestAppContext) { + let table = BlockKind::Table { + columns: 2, + merged: vec![], + }; + let cells = ["h1", "h2", "a", "b", "c", "d", "e", "f"]; + let (editor, cx) = open(vec![BlockData::new(table, cells)], cx); + cx.update(|_, cx| { + editor.update(cx, |editor, cx| { + let key = editor.blocks[0].key; + editor.remove_line(key, 6, false, cx); + editor.remove_line(key, 6, false, cx); + editor.merge_right(key, 6, cx); + }) + }); + settle(cx); + let texts: Vec = blocks(&editor, cx).remove(0).1; + assert_eq!(texts, ["h1", "h2", "a", "b", "c", "d"]); +} + +#[gpui::test] +fn a_drag_on_a_picture_turned_paragraph_leaves_undo_alone(cx: &mut TestAppContext) { + let image = BlockKind::Image(Media::new("missing.png")); + let (editor, cx) = open(vec![BlockData::new(image.clone(), ["caption"])], cx); + cx.update(|window, cx| { + editor.update(cx, |editor, cx| { + let key = editor.blocks[0].key; + editor.turn_into(key, BlockKind::Paragraph, window, cx); + editor.resize(key, 0.5, true, cx); + editor.undo(window, cx); + }) + }); + settle(cx); + assert_eq!(blocks(&editor, cx)[0].0, image); +} diff --git a/src/documents/blocks/view.rs b/src/documents/blocks/view.rs index a508e8e..0956030 100644 --- a/src/documents/blocks/view.rs +++ b/src/documents/blocks/view.rs @@ -54,7 +54,9 @@ impl BlockEditor { window: &mut Window, cx: &mut Context, ) { - let ix = self.index(key); + let Some(ix) = self.index(key) else { + return; + }; let empty = self.blocks[ix] .fields .first() @@ -216,7 +218,10 @@ impl Render for BlockEditor { .on_mouse_down( MouseButton::Left, cx.listener(move |editor, _, window, cx| { - let end = editor.blocks[editor.index(key)] + let Some(ix) = editor.index(key) else { + return; + }; + let end = editor.blocks[ix] .fields .first() .map(|field| field.read(cx).text().len()); diff --git a/src/documents/history.rs b/src/documents/history.rs index 4de1cca..18b1168 100644 --- a/src/documents/history.rs +++ b/src/documents/history.rs @@ -146,11 +146,12 @@ impl VersionHistory { selected: usize, ) -> Self { let versions: Vec = versions.into_iter().collect(); - assert!( - selected < versions.len(), - "version {selected} of {}", - versions.len() - ); + if selected >= versions.len() { + log::error!( + "version history: version {selected} of {}; none selected", + versions.len() + ); + } assert!( versions.windows(2).all(|pair| pair[0].at >= pair[1].at), "versions come newest first" @@ -181,8 +182,8 @@ impl RenderOnce for VersionHistory { let showing_changes = *changes.read(cx); let theme = cx.theme(); let colors = theme.colors.clone(); - let version = &self.versions[self.selected]; - let before = self.versions.get(self.selected + 1); + let version = self.versions.get(self.selected); + let before = version.and_then(|_| self.versions.get(self.selected + 1)); let list = self.versions.iter().enumerate().map(|(ix, version)| { let pick = self.on_select.clone(); let lit = ix == self.selected; @@ -248,7 +249,7 @@ impl RenderOnce for VersionHistory { let restore = self .on_restore .clone() - .filter(|_| self.selected > 0) + .filter(|_| self.selected > 0 && version.is_some()) .map(|restore| { let at = self.selected; Button::new((self.id.clone(), "restore"), "Restore this version") @@ -259,16 +260,20 @@ impl RenderOnce for VersionHistory { restore(at, window, cx) }) }); - let body = match (showing_changes, before) { - (true, Some(before)) => { + let body = match (version, showing_changes, before) { + (None, ..) => div() + .text_color(colors.fg_subtle) + .child("No version is selected.") + .into_any_element(), + (Some(version), true, Some(before)) => { PageHistoryDiff::new(before.markdown.clone(), version.markdown.clone()) .into_any_element() } - (true, None) => div() + (Some(_), true, None) => div() .text_color(colors.fg_subtle) .child("The first version; nothing came before it.") .into_any_element(), - (false, _) => MarkdownRenderer::new( + (Some(version), false, _) => MarkdownRenderer::new( (self.id.clone(), format!("text-{}", self.selected)), version.markdown.clone(), ) diff --git a/src/documents/modes.rs b/src/documents/modes.rs index 579d72f..9666328 100644 --- a/src/documents/modes.rs +++ b/src/documents/modes.rs @@ -49,8 +49,9 @@ impl MarkdownMode { type OnMode = Rc; -/// The block open as its source: where it came from, its own field, and whether it adds a block at the end. +/// The block open as its source: the document it opened in, where, its own field, and whether it adds a block at the end. struct Open { + document: String, range: Range, input: Entity, append: bool, @@ -85,7 +86,7 @@ fn joint(document: &str) -> &'static str { } } -/// Writes the open block back over its range as one undo step, and closes it; the range replaced and the bytes written. +/// Writes the open block back over its range as one undo step, and closes it; the range replaced and the bytes written, none when the text left the range. fn commit( state: &Entity, field: &Entity, @@ -97,6 +98,13 @@ fn commit( })?; let written = open.input.read(cx).text().to_string(); let document = field.read(cx).text().to_string(); + if document != open.document { + log::error!( + "markdown editor: the text changed under block {:?}; its edit is dropped", + open.range + ); + return None; + } let written = match open.append { true if written.is_empty() => return Some((open.range, 0)), true => format!("{}{written}", joint(&document)), @@ -122,7 +130,8 @@ fn open( window: &mut Window, cx: &mut App, ) { - let source = field.read(cx).text()[range.clone()].to_string(); + let document = field.read(cx).text().to_string(); + let source = document[range.clone()].to_string(); log::info!("markdown editor: block {range:?} opens"); let input = cx.new(|cx| { TextInput::new(window, cx) @@ -146,6 +155,7 @@ fn open( }; state.update(cx, |visual, cx| { visual.open = Some(Open { + document, range, input, append, @@ -262,7 +272,7 @@ fn visual( theme.text_size(TextSize::Xs), ) }; - let document = editor.field.read(cx).text().to_string(); + let document = SharedString::from(editor.field.read(cx).text().to_string()); let blocks = block_ranges(&document); let opened = state .read(cx) @@ -296,7 +306,12 @@ fn visual( rows.push(source(input, window, cx)); continue; } - let (state, field, pressed) = (state.clone(), field.clone(), range.clone()); + let (state, field, pressed, painted) = ( + state.clone(), + field.clone(), + range.clone(), + document.clone(), + ); rows.push( div() .id((editor.id.clone(), format!("block-{ix}"))) @@ -308,6 +323,12 @@ fn visual( .on_mouse_down(MouseButton::Left, move |_, window, cx| { window.prevent_default(); cx.stop_propagation(); + if field.read(cx).text() != painted.as_ref() { + log::error!( + "markdown editor: block {pressed:?} left with the text; press dropped" + ); + return; + } let range = match commit(&state, &field, cx) { Some((replaced, written)) => shifted(pressed.clone(), &replaced, written), None => pressed.clone(), @@ -376,7 +397,7 @@ fn visual( }; let (up, down) = (edges(false), edges(true)); let (escape_state, escape_field) = (state.clone(), field.clone()); - let text = SharedString::from(document); + let text = document; div() .id(editor.id.clone()) .flex() @@ -444,24 +465,4 @@ fn visual( } #[cfg(test)] -mod tests { - use super::{joint, shifted}; - - #[test] - fn later_blocks_move_by_what_an_edit_wrote() { - assert_eq!(shifted(10..14, &(2..6), 9), 15..19); - assert_eq!( - shifted(0..2, &(2..6), 9), - 0..2, - "a block before the edit stays" - ); - } - - #[test] - fn a_block_at_the_end_starts_a_paragraph() { - assert_eq!( - (joint(""), joint("a"), joint("a\n"), joint("a\n\n")), - ("", "\n\n", "\n", "") - ); - } -} +mod tests; diff --git a/src/documents/modes/tests.rs b/src/documents/modes/tests.rs new file mode 100644 index 0000000..6341def --- /dev/null +++ b/src/documents/modes/tests.rs @@ -0,0 +1,112 @@ +use gpui::{AppContext as _, Focusable as _}; + +use super::{MarkdownEditor, MarkdownMode, TextInput, Visual, commit, joint, open, shifted}; + +#[cfg(feature = "test-support")] +#[gpui::test] +fn an_open_block_of_a_replaced_text_writes_nothing(cx: &mut gpui::TestAppContext) { + cx.update(crate::theme::Theme::init); + cx.add_empty_window().update(|window, cx| { + let field = cx.new(|cx| TextInput::new(window, cx).multi_line(1, usize::MAX)); + field.update(cx, |field, cx| { + field.set_text( + "one + +two", cx, + ) + }); + let state = cx.new(|_| Visual::default()); + open(&state, &field, 0..3, false, true, window, cx); + field.update(cx, |field, cx| field.set_text("three", cx)); + assert_eq!(commit(&state, &field, cx), None); + assert_eq!(field.read(cx).text(), "three"); + }); +} + +#[test] +fn later_blocks_move_by_what_an_edit_wrote() { + assert_eq!(shifted(10..14, &(2..6), 9), 15..19); + assert_eq!( + shifted(0..2, &(2..6), 9), + 0..2, + "a block before the edit stays" + ); +} + +#[test] +fn a_block_at_the_end_starts_a_paragraph() { + assert_eq!( + (joint(""), joint("a"), joint("a\n"), joint("a\n\n")), + ("", "\n\n", "\n", "") + ); +} + +#[cfg(feature = "test-support")] +struct Visualized(gpui::Entity); + +#[cfg(feature = "test-support")] +impl gpui::Render for Visualized { + fn render( + &mut self, + _: &mut gpui::Window, + _: &mut gpui::Context, + ) -> impl gpui::IntoElement { + use gpui::{ParentElement, Styled}; + gpui::div() + .size_full() + .child(MarkdownEditor::new("visual", &self.0, MarkdownMode::Visual)) + } +} + +#[cfg(feature = "test-support")] +#[gpui::test] +fn a_press_on_a_block_of_a_replaced_text_opens_nothing(cx: &mut gpui::TestAppContext) { + use crate::theme::{ActiveTheme, ControlSize}; + cx.update(crate::theme::Theme::init); + let (view, cx) = cx.add_window_view(|window, cx| { + Visualized(cx.new(|cx| { + let mut field = TextInput::new(window, cx).multi_line(1, usize::MAX); + field.set_text("A long paragraph to edit.", cx); + field + })) + }); + cx.update(|window, _| window.activate_window()); + cx.run_until_parked(); + let field = view.read_with(cx, |view, _| view.0.clone()); + let at = cx.update(|window, cx| { + let bar = cx.theme().control_height(ControlSize::Lg); + gpui::point( + gpui::px(40.0), + bar.to_pixels(window.rem_size()) + gpui::px(20.0), + ) + }); + let source = |cx: &mut gpui::VisualTestContext| { + cx.update(|window, cx| { + window + .focused(cx) + .is_some_and(|f| f != field.focus_handle(cx)) + }) + }; + cx.simulate_mouse_down(at, gpui::MouseButton::Left, gpui::Modifiers::none()); + cx.run_until_parked(); + assert!(source(cx), "a press opens the block as its source"); + cx.update(|window, cx| window.focus(&field.focus_handle(cx), cx)); + cx.run_until_parked(); + cx.update(|window, cx| { + field.update(cx, |field, cx| field.set_text("x", cx)); + let press = gpui::MouseDownEvent { + position: at, + button: gpui::MouseButton::Left, + modifiers: gpui::Modifiers::none(), + click_count: 1, + first_mouse: false, + }; + window.dispatch_event(gpui::PlatformInput::MouseDown(press), cx); + }); + cx.run_until_parked(); + assert!(!source(cx), "a press on a replaced block opens nothing"); + assert_eq!( + field.read_with(cx, |field, _| field.text().to_string()), + "x" + ); +} diff --git a/src/documents/tests.rs b/src/documents/tests.rs index db9260a..ecf9368 100644 --- a/src/documents/tests.rs +++ b/src/documents/tests.rs @@ -253,3 +253,51 @@ fn escape_leaves_zen_turned_on_from_outside_and_hands_focus_back(cx: &mut TestAp let back = cx.update(|window, cx| zen.read(cx).outside.focus_handle(cx).is_focused(window)); assert!(back, "focus returns to where it was"); } + +#[gpui::test] +fn hits_past_the_pages_are_left_and_the_current_follows(cx: &mut TestAppContext) { + let (reader, _, cx) = open_with(400.0, None, cx); + reader.update(cx, |reader, cx| { + reader.hits = vec![hit(7), hit(2), hit(9), hit(1)]; + reader.current = Some(3); + cx.notify(); + }); + settle(cx); + reader.update(cx, |reader, cx| { + reader.current = Some(12); + cx.notify(); + }); + settle(cx); + let (_, _, cx) = open_with(400.0, Some(usize::MAX), cx); + settle(cx); +} + +/// Versions whose selected one left with a shorter history. +struct Versions(Vec, usize); + +impl Render for Versions { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + div().size_full().child(super::VersionHistory::new( + "history", + self.0.clone(), + self.1, + )) + } +} + +#[gpui::test] +fn a_selected_version_past_the_history_selects_none(cx: &mut TestAppContext) { + cx.update(Theme::init); + let version = super::Version { + at: jiff::Timestamp::UNIX_EPOCH, + author: "Ann".into(), + markdown: "# Plan".into(), + }; + let (view, cx) = cx.add_window_view(|_, _| Versions(vec![version], 4)); + settle(cx); + view.update(cx, |view, cx| { + *view = Versions(Vec::new(), usize::MAX); + cx.notify(); + }); + settle(cx); +} diff --git a/src/documents/tree.rs b/src/documents/tree.rs index aebad3b..bd6c1d0 100644 --- a/src/documents/tree.rs +++ b/src/documents/tree.rs @@ -63,7 +63,7 @@ struct Builder { inline: Inline, open: Vec<(usize, Style)>, link: Vec<(usize, String)>, - image: Option<(String, String)>, + image: Vec<(String, String)>, code: Option<(Option, String)>, head: Vec, row: Vec, @@ -189,10 +189,10 @@ pub fn parse(markdown: &str) -> Vec { Event::Rule => push(&mut built, &mut out, Node::Rule), Event::DisplayMath(math) => push(&mut built, &mut out, Node::Math(math.to_string())), Event::Start(Tag::Image { dest_url, .. }) => { - built.image = Some((String::new(), dest_url.to_string())) + built.image.push((String::new(), dest_url.to_string())) } Event::End(TagEnd::Image) => { - let (alt, url) = built.image.take().expect("an image was open"); + let (alt, url) = built.image.pop().expect("an image closes after it opens"); push(&mut built, &mut out, Node::Image { alt, url }); } Event::Start(Tag::Strong) => built.open.push((built.inline.text.len(), Style::Strong)), @@ -216,7 +216,7 @@ pub fn parse(markdown: &str) -> Vec { built.inline.styles.push((range.clone(), Style::Link)); built.inline.links.push((range, url)); } - Event::Text(text) => match (&mut built.code, &mut built.image) { + Event::Text(text) => match (&mut built.code, built.image.last_mut()) { (Some((_, code)), _) => code.push_str(&text), (None, Some((alt, _))) => alt.push_str(&text), (None, None) => built.inline.text.push_str(&text), @@ -296,6 +296,19 @@ mod tests { assert_eq!(nodes[5], Node::Rule); } + #[test] + fn an_image_inside_an_image_keeps_both() { + let nodes = parse("![outer ![inner](inner.png)](outer.png)"); + let urls: Vec<&str> = nodes + .iter() + .filter_map(|node| match node { + Node::Image { url, .. } => Some(url.as_str()), + _ => None, + }) + .collect(); + assert_eq!(urls, ["inner.png", "outer.png"], "{nodes:?}"); + } + #[test] fn display_math_stands_alone() { assert_eq!(parse("$$c = 1$$"), [Node::Math("c = 1".into())]); diff --git a/src/documents/viewer.rs b/src/documents/viewer.rs index f290015..1e4701c 100644 --- a/src/documents/viewer.rs +++ b/src/documents/viewer.rs @@ -141,15 +141,18 @@ impl DocumentViewer { /// The owner's find box, floating over the pages; the owner searches them, and `hits` light where they are, the current one scrolled into view. pub fn find(mut self, widget: FindWidget, hits: Vec, current: Option) -> Self { - assert!( - current.is_none_or(|current| current < hits.len()), - "hit {current:?} of {}", - hits.len() - ); - assert!( - hits.iter().all(|hit| hit.page < self.pages.len()), - "a hit lies past the last page" - ); + let pages = self.pages.len(); + let on_page = |hit: &PageHit| hit.page < pages; + if current.is_some_and(|current| current >= hits.len()) || !hits.iter().all(on_page) { + log::error!( + "document viewer: hit {current:?} of {}, some past {pages} pages; those left", + hits.len() + ); + } + let current = current + .filter(|current| hits.get(*current).is_some_and(on_page)) + .map(|current| hits[..current].iter().filter(|hit| on_page(hit)).count()); + let hits: Vec = hits.into_iter().filter(on_page).collect(); self.find = Some(Find { widget, hits, diff --git a/src/editor/find.rs b/src/editor/find.rs index f2a3564..70822e9 100644 --- a/src/editor/find.rs +++ b/src/editor/find.rs @@ -78,6 +78,10 @@ impl FindWidget { current: Option, total: usize, ) -> Self { + if current.is_some_and(|current| current >= total) { + log::error!("find: match {current:?} of {total}; none current"); + } + let current = current.filter(|current| *current < total); Self { id: id.into(), find: find.clone(), diff --git a/tasks/panics.md b/tasks/panics.md index c0f3bd9..0d076d8 100644 --- a/tasks/panics.md +++ b/tasks/panics.md @@ -35,7 +35,7 @@ Each item reads every site in its folders, fixes what the rules name, updates ca - [x] P18 Terminal — `src/terminal` - [x] P19 Git — `src/git` - [x] P20 Debug — `src/debug` -- [ ] P21 Documents — `src/documents` +- [x] P21 Documents — `src/documents` - [ ] P22 Collaboration — `src/collab` - [ ] P23 AI Chat — `src/chat` - [ ] P24 Agent — `src/agent` @@ -86,3 +86,4 @@ One line per item: what changed, what stayed and why, review rounds. - P18: row paints and the font advance log gpui faults; a pty side caps at what ConPTY and alacritty's pixel sizes hold; a default shell past the list keeps the one before. Kept: constant regexes, captured matches, keyed rows, non-empty shells. Review: 2 rounds. - P19: a blame that lags its code draws every line, bare where no owner fits; `git::resolve` returns `Option` for a conflict gone from the text. Kept: diff groups, merge regions, similar's pinned resolutions. Review: 2 rounds. - P20: a hex selection off the bytes and a stack frame past the stack select none; a flame focus keeps what the profile holds; the timeline's wheel reports only finite, positive ranges from finite factors; a dump's last byte must have an address; flame paints log. Kept: disassembly patterns, `zoomed`'s contract. Review: 3 rounds. +- P21: block editor listeners painted for a block, field, cell or kind since gone log and change nothing, a dropped picture drag leaves undo alone; a visual block press or edit over text the owner replaced is dropped; nested images parse; a version or find hit past its list selects none. Review: 4 rounds.