From edc328aadfaefb78f4b6b3e98b4f4780746a1d52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Mon, 12 Oct 2026 06:40:36 -0400 Subject: [PATCH] documents: table edits keep extra cells and quote marks and say why they refuse; preview takes no edits; lists go on only in list items; a match waits for its text Co-Authored-By: Claude Opus 5.5 (1M context) --- src/documents/document/finding.rs | 39 +++++- src/documents/document/lists.rs | 39 +++++- src/documents/document/mod.rs | 5 +- src/documents/document/tables.rs | 186 +++++++++++++++++++------- src/documents/document/tests/tools.rs | 58 +++++++- src/documents/document/visual.rs | 1 + src/documents/mod.rs | 4 +- 7 files changed, 273 insertions(+), 59 deletions(-) diff --git a/src/documents/document/finding.rs b/src/documents/document/finding.rs index 38cea91..54f4713 100644 --- a/src/documents/document/finding.rs +++ b/src/documents/document/finding.rs @@ -22,11 +22,13 @@ impl MarkdownDocument { } MarkdownMode::Preview => self.reveal(line(self, cx), window, cx), MarkdownMode::Visual => { - if self.commit(cx).is_some() { + if self.commit(cx).is_some() || !self.read_now(cx) { self.reparse(true, cx); } if !self.read_now(cx) { - return log::warn!("markdown document: a match shown while the text is read"); + // A long text: shown once read. + self.showing = Some(range); + return cx.notify(); } let ix = self.block_at(range.start); let held = self.parsed.blocks.get(ix).map(|block| block.range.clone()); @@ -48,10 +50,39 @@ impl MarkdownDocument { } } - /// `edits` to the text as one undo step. - pub fn replace(&mut self, edits: Vec<(Range, String)>, cx: &mut Context) { + /// A match asked before its text was read, now shown. + pub(super) fn show_waiting(&mut self, window: &mut Window, cx: &mut Context) { + if self.showing.is_none() || !self.read_now(cx) { + return; + } + let range = self.showing.take().expect("checked above"); + let text = self.editor.read(cx).text(); + let fits = range.end <= text.len() + && text.is_char_boundary(range.start) + && text.is_char_boundary(range.end); + if !fits { + return log::warn!("markdown document: a waiting match outlived its text"); + } + self.show_match(range, window, cx); + } + + /// `edits` as one undo step; false in preview. + pub fn replace(&mut self, edits: Vec<(Range, String)>, cx: &mut Context) -> bool { + if self.mode == MarkdownMode::Preview { + return false; + } self.commit(cx); + let text = self.editor.read(cx).text(); + if edits + .iter() + .all(|(range, with)| text[range.clone()] == *with) + { + return true; + } log::info!("markdown document: {} replaced", edits.len()); + // Its own change: an open block outlives it. + self.writes += 1; self.editor.update(cx, |editor, cx| editor.edit(edits, cx)); + true } } diff --git a/src/documents/document/lists.rs b/src/documents/document/lists.rs index 1b463a7..480312d 100644 --- a/src/documents/document/lists.rs +++ b/src/documents/document/lists.rs @@ -3,8 +3,10 @@ use std::ops::Range; use gpui::{Context, Window}; +use markdown::mdast; use super::MarkdownDocument; +use crate::documents::markdown::{span, tree, walk}; /// Spaces one indent step adds. const STEP: usize = 4; @@ -39,6 +41,23 @@ pub(super) fn next(line: &str) -> Option { Some(Next::Item(format!("{}{mark}{task}", " ".repeat(indent)))) } +/// Whether `at` sits in a list item, not code. +fn listed(source: &str, at: usize) -> bool { + let root = tree(source); + let mut nodes = Vec::new(); + walk(&root, &mut nodes); + let held: Vec<&mdast::Node> = nodes + .into_iter() + .filter(|node| span(node).start <= at && at <= span(node).end) + .collect(); + let item = held + .iter() + .any(|node| matches!(node, mdast::Node::ListItem(_))); + item && !held + .iter() + .any(|node| matches!(node, mdast::Node::Code(_) | mdast::Node::Math(_))) +} + /// A bullet marker, and where its text starts. fn bullet(rest: &str) -> Option<(String, usize)> { let mark = rest.chars().next().filter(|ch| "-*+".contains(*ch))?; @@ -117,7 +136,25 @@ impl MarkdownDocument { if editor.selections().len() != 1 || !caret.is_empty() { return false; } + if !self.read_now(cx) { + self.reparse(true, cx); + } + if !self.read_now(cx) { + log::info!("markdown document: Enter while a long text is read"); + return false; + } + let editor = self.editor.read(cx); let text = editor.text(); + let ix = self.block_at(caret.start); + let block = self.parsed.blocks.get(ix).map(|block| block.range.clone()); + let Some(block) = + block.filter(|block| block.start <= caret.start && caret.start <= block.end) + else { + return false; + }; + if !listed(&text[block.clone()], caret.start - block.start) { + return false; + } let start = text[..caret.start].rfind('\n').map_or(0, |at| at + 1); let end = text[caret.start..] .find('\n') @@ -149,7 +186,7 @@ impl MarkdownDocument { let input = input.read(cx); (input.text().to_string(), input.selection()) }; - if !caret.is_empty() { + if !caret.is_empty() || !listed(&text, caret.start) { return false; } let start = text[..caret.start].rfind('\n').map_or(0, |at| at + 1); diff --git a/src/documents/document/mod.rs b/src/documents/document/mod.rs index 69d01fb..4f640bb 100644 --- a/src/documents/document/mod.rs +++ b/src/documents/document/mod.rs @@ -19,7 +19,7 @@ use gpui::{ pub use parse::DocumentHeading; use parse::{Parsed, alike, parsed}; -pub use tables::TableEdit; +pub use tables::{TableEdit, TableRefusal}; use super::{MarkdownMode, markdown::markdown_highlights, modes::joint, render::OnLink}; use crate::{ @@ -79,6 +79,8 @@ pub struct MarkdownDocument { asks: u64, /// Its own write-backs whose changes are still to come. writes: usize, + /// A match asked for before its text was read. + showing: Option>, focus: FocusHandle, on_link: Option, _subscriptions: Vec, @@ -131,6 +133,7 @@ impl MarkdownDocument { top: None, asks: 0, writes: 0, + showing: None, focus, on_link: None, _subscriptions: subscriptions, diff --git a/src/documents/document/tables.rs b/src/documents/document/tables.rs index 63e4dc6..aef523f 100644 --- a/src/documents/document/tables.rs +++ b/src/documents/document/tables.rs @@ -6,7 +6,10 @@ use gpui::Context; use markdown::mdast::{self, AlignKind}; use super::MarkdownDocument; -use crate::documents::markdown::{span, tree, walk}; +use crate::documents::{ + MarkdownMode, + markdown::{span, tree, walk}, +}; /// What a table edit does at the caret. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -22,21 +25,47 @@ pub enum TableEdit { /// A new table: three columns, a head and two rows. const NEW: &str = "| | | |\n| --- | --- | --- |\n| | | |\n| | | |"; +/// Why a table edit did not happen. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum TableRefusal { + /// No table at the caret. + Outside, + /// The head row and the last column stay. + Kept, + /// The caret is in a cell past the head's columns. + Beyond, + /// Its lines carry different quote or list marks. + Mixed, + /// Preview takes no edits. + Preview, +} + /// The table around `at` after `edit`: its range, text, caret. pub(super) fn edit_table( text: &str, at: usize, edit: TableEdit, -) -> Option<(Range, String, usize)> { +) -> Result<(Range, String, usize), TableRefusal> { let root = tree(text); let mut nodes = Vec::new(); walk(&root, &mut nodes); - let (range, table) = nodes.into_iter().find_map(|node| match node { - mdast::Node::Table(table) if (span(node).start..=span(node).end).contains(&at) => { - Some((span(node), table)) - } - _ => None, - })?; + let (range, table) = nodes + .into_iter() + .find_map(|node| match node { + mdast::Node::Table(table) if (span(node).start..=span(node).end).contains(&at) => { + Some((span(node), table)) + } + _ => None, + }) + .ok_or(TableRefusal::Outside)?; + let prefix = &text[text[..range.start].rfind('\n').map_or(0, |found| found + 1)..range.start]; + if text[range.clone()] + .split('\n') + .skip(1) + .any(|line| !line.starts_with(prefix)) + { + return Err(TableRefusal::Mixed); + } let rows: Vec<&mdast::Node> = table.children.iter().collect(); let row = rows .iter() @@ -48,6 +77,7 @@ pub(super) fn edit_table( .rposition(|cell| span(cell).start <= at) .unwrap_or(0); let width = table.align.len(); + // Cells past the head stay as written. let mut grid: Vec> = rows .iter() .map(|row| { @@ -55,21 +85,30 @@ pub(super) fn edit_table( .iter() .map(|cell| written(text, cell)) .collect(); - cells.resize(width, String::new()); + if cells.len() < width { + cells.resize(width, String::new()); + } cells }) .collect(); let mut aligns = table.align.clone(); + let column_edit = matches!( + edit, + TableEdit::ColumnLeft | TableEdit::ColumnRight | TableEdit::DeleteColumn + ); + if column_edit && column >= width { + return Err(TableRefusal::Beyond); + } let (row, column) = match edit { TableEdit::RowAbove | TableEdit::RowBelow => { let below = edit == TableEdit::RowBelow || row == 0; let at = if below { row + 1 } else { row }; grid.insert(at, vec![String::new(); width]); - (at, column) + (at, column.min(width - 1)) } TableEdit::DeleteRow if row > 0 => { grid.remove(row); - (row.min(grid.len() - 1), column) + (row.min(grid.len() - 1), column.min(width - 1)) } TableEdit::ColumnLeft | TableEdit::ColumnRight => { let at = column + usize::from(edit == TableEdit::ColumnRight); @@ -85,10 +124,10 @@ pub(super) fn edit_table( aligns.remove(column); (row, column.min(width - 2)) } - _ => return None, + _ => return Err(TableRefusal::Kept), }; - let (out, caret) = table_text(&grid, &aligns, (row, column)); - Some((range, out, caret)) + let (out, caret) = table_text(&grid, &aligns, (row, column), prefix); + Ok((range, out, caret)) } /// A cell's source between its pipes. @@ -100,8 +139,13 @@ fn written(text: &str, cell: &mdast::Node) -> String { } } -/// The table's text, and where cell `at`'s text starts. -fn table_text(grid: &[Vec], aligns: &[AlignKind], at: (usize, usize)) -> (String, usize) { +/// The table's text, each line after the first under `prefix`. +fn table_text( + grid: &[Vec], + aligns: &[AlignKind], + at: (usize, usize), + prefix: &str, +) -> (String, usize) { let mut out = String::new(); let mut caret = 0; for (row, cells) in grid.iter().enumerate() { @@ -116,6 +160,7 @@ fn table_text(grid: &[Vec], aligns: &[AlignKind], at: (usize, usize)) -> out.push('|'); if row == 0 { out.push('\n'); + out.push_str(prefix); for align in aligns { out.push_str(match align { AlignKind::Left => "| :-- ", @@ -128,32 +173,42 @@ fn table_text(grid: &[Vec], aligns: &[AlignKind], at: (usize, usize)) -> } if row + 1 < grid.len() { out.push('\n'); + out.push_str(prefix); } } (out, caret) } impl MarkdownDocument { - /// `edit` on the table at the caret; false outside one. - pub fn edit_table(&mut self, edit: TableEdit, cx: &mut Context) -> bool { + /// `edit` on the table at the caret, or why not. + pub fn edit_table( + &mut self, + edit: TableEdit, + cx: &mut Context, + ) -> Result<(), TableRefusal> { + if self.mode == MarkdownMode::Preview { + return Err(TableRefusal::Preview); + } self.commit(cx); let at = self.editor.read(cx).primary().range().start; let text = self.editor.read(cx).text().to_string(); - let Some((range, table, caret)) = edit_table(&text, at, edit) else { - log::info!("markdown document: no table edit {edit:?} at the caret"); - return false; - }; + let (range, table, caret) = edit_table(&text, at, edit).inspect_err(|why| { + log::info!("markdown document: table edit {edit:?} refused: {why:?}"); + })?; log::info!("markdown document: table edit {edit:?}"); let caret = range.start + caret; self.editor.update(cx, |editor, cx| { editor.edit([(range, table)], cx); editor.select(std::iter::once(caret..caret), cx); }); - true + Ok(()) } - /// A new table after the caret's line. - pub fn insert_table(&mut self, cx: &mut Context) { + /// A new table after the caret's line, unless in preview. + pub fn insert_table(&mut self, cx: &mut Context) -> Result<(), TableRefusal> { + if self.mode == MarkdownMode::Preview { + return Err(TableRefusal::Preview); + } self.commit(cx); let text = self.editor.read(cx).text().to_string(); let at = self.editor.read(cx).primary().range().start; @@ -176,62 +231,93 @@ impl MarkdownDocument { editor.edit([(end..end, written)], cx); editor.select(std::iter::once(caret..caret), cx); }); + Ok(()) } } #[cfg(test)] mod tests { - use super::{TableEdit, edit_table}; + use super::{TableEdit, TableRefusal, edit_table}; const TABLE: &str = "Intro\n\n| A | B |\n|:--|--:|\n| 1 | 2 |\n| 3 | 4 |\n\nAfter\n"; - fn edited(at: &str, edit: TableEdit) -> Option { - let at = TABLE.find(at).expect("in the table"); - let (range, table, _) = edit_table(TABLE, at, edit)?; - Some(format!( + fn edited_in(text: &str, at: &str, edit: TableEdit) -> Result { + let at = text.find(at).expect("in the table"); + let (range, table, _) = edit_table(text, at, edit)?; + Ok(format!( "{}{table}{}", - &TABLE[..range.start], - &TABLE[range.end..] + &text[..range.start], + &text[range.end..] )) } + fn edited(at: &str, edit: TableEdit) -> Result { + edited_in(TABLE, at, edit) + } + #[test] fn rows_and_columns_come_and_go() { - let around = |table: &str| format!("Intro\n\n{table}\n\nAfter\n"); + let around = |table: &str| Ok(format!("Intro\n\n{table}\n\nAfter\n")); assert_eq!( - edited("3", TableEdit::RowAbove).as_deref(), - Some(around("| A | B |\n| :-- | --: |\n| 1 | 2 |\n| | |\n| 3 | 4 |").as_str()) + edited("3", TableEdit::RowAbove), + around("| A | B |\n| :-- | --: |\n| 1 | 2 |\n| | |\n| 3 | 4 |") ); assert_eq!( - edited("A", TableEdit::RowBelow).as_deref(), - Some(around("| A | B |\n| :-- | --: |\n| | |\n| 1 | 2 |\n| 3 | 4 |").as_str()) + edited("A", TableEdit::RowBelow), + around("| A | B |\n| :-- | --: |\n| | |\n| 1 | 2 |\n| 3 | 4 |") ); assert_eq!( - edited("2", TableEdit::ColumnLeft).as_deref(), - Some(around("| A | | B |\n| :-- | --- | --: |\n| 1 | | 2 |\n| 3 | | 4 |").as_str()) + edited("2", TableEdit::ColumnLeft), + around("| A | | B |\n| :-- | --- | --: |\n| 1 | | 2 |\n| 3 | | 4 |") ); assert_eq!( - edited("1", TableEdit::DeleteRow).as_deref(), - Some(around("| A | B |\n| :-- | --: |\n| 3 | 4 |").as_str()) + edited("1", TableEdit::DeleteRow), + around("| A | B |\n| :-- | --: |\n| 3 | 4 |") ); assert_eq!( - edited("B", TableEdit::DeleteColumn).as_deref(), - Some(around("| A |\n| :-- |\n| 1 |\n| 3 |").as_str()) + edited("B", TableEdit::DeleteColumn), + around("| A |\n| :-- |\n| 1 |\n| 3 |") ); } #[test] fn the_head_stays_and_outside_is_no_table() { + assert_eq!(edited("A", TableEdit::DeleteRow), Err(TableRefusal::Kept)); assert_eq!( - edited("A", TableEdit::DeleteRow), - None, - "the head row stays" + edit_table(TABLE, 1, TableEdit::RowBelow).err(), + Some(TableRefusal::Outside) ); - assert!(edit_table(TABLE, 1, TableEdit::RowBelow).is_none()); let single = "| A |\n| - |\n| 1 |\n"; - assert!( - edit_table(single, 2, TableEdit::DeleteColumn).is_none(), - "the last column stays" + assert_eq!( + edit_table(single, 2, TableEdit::DeleteColumn).err(), + Some(TableRefusal::Kept) + ); + } + + #[test] + fn cells_past_the_head_stay_and_take_no_column_edit() { + let text = "| A | B |\n| - | - |\n| 1 | 2 | 3 |\n"; + assert_eq!( + edited_in(text, "3", TableEdit::DeleteColumn), + Err(TableRefusal::Beyond) + ); + assert_eq!( + edited_in(text, "1", TableEdit::RowBelow), + Ok("| A | B |\n| --- | --- |\n| 1 | 2 | 3 |\n| | |\n".into()) + ); + } + + #[test] + fn a_quoted_table_keeps_its_marks() { + let text = "> | A | B |\n> | - | - |\n> | 1 | 2 |\n"; + assert_eq!( + edited_in(text, "1", TableEdit::RowBelow), + Ok("> | A | B |\n> | --- | --- |\n> | 1 | 2 |\n> | | |\n".into()) + ); + let mixed = "- | A |\n | - |\n | 1 |\n"; + assert_eq!( + edited_in(mixed, "1", TableEdit::RowBelow), + Err(TableRefusal::Mixed) ); } } diff --git a/src/documents/document/tests/tools.rs b/src/documents/document/tests/tools.rs index 77efe8b..544cd53 100644 --- a/src/documents/document/tests/tools.rs +++ b/src/documents/document/tests/tools.rs @@ -46,7 +46,7 @@ fn a_table_edit_is_one_undo_step_at_the_caret(cx: &mut TestAppContext) { let edited = document.update(cx, |document, cx| { document.edit_table(crate::documents::TableEdit::RowBelow, cx) }); - assert!(edited); + assert_eq!(edited, Ok(())); assert_eq!(text(&document, cx), "| A |\n| --- |\n| 1 |\n| |\n"); let editor = document.read_with(cx, |document, _| document.editor().clone()); editor.update(cx, |editor, cx| editor.undo(cx)); @@ -56,7 +56,9 @@ fn a_table_edit_is_one_undo_step_at_the_caret(cx: &mut TestAppContext) { #[gpui::test] fn a_new_table_goes_after_the_caret_line(cx: &mut TestAppContext) { let (document, cx) = open("Intro\nNext\n", MarkdownMode::Visual, cx); - document.update(cx, |document, cx| document.insert_table(cx)); + document.update(cx, |document, cx| { + document.insert_table(cx).expect("inserted") + }); assert_eq!( text(&document, cx), "Intro\n\n| | | |\n| --- | --- | --- |\n| | | |\n| | | |\n\nNext\n" @@ -93,3 +95,55 @@ fn every_replacement_is_one_undo_step(cx: &mut TestAppContext) { editor.update(cx, |editor, cx| editor.undo(cx)); assert_eq!(text(&document, cx), "a b a"); } + +const FENCED: &str = "```text\n- a\n```\n"; + +#[gpui::test] +fn enter_in_source_code_is_a_plain_line_break(cx: &mut TestAppContext) { + let (document, cx) = open(FENCED, MarkdownMode::Source, cx); + document.update(cx, |document, cx| document.set_caret(1, 3, cx)); + let editor = document.read_with(cx, |document, _| document.editor().clone()); + let focus = editor.read_with(cx, gpui::Focusable::focus_handle); + cx.update(|window, cx| window.focus(&focus, cx)); + cx.simulate_keystrokes("enter"); + assert_eq!(text(&document, cx), "```text\n- a\n\n```\n"); +} + +#[gpui::test] +fn enter_in_an_open_code_block_is_a_plain_line_break(cx: &mut TestAppContext) { + let (document, cx) = open(FENCED, MarkdownMode::Visual, cx); + press(0, Modifiers::none(), cx); + document.update(cx, |document, cx| { + let input = document.open.as_ref().expect("open").input.clone(); + input.update(cx, |input, cx| input.select(11..11, cx)); + }); + cx.simulate_keystrokes("enter"); + assert_eq!(opened(&document, cx), "```text\n- a\n\n```"); +} + +#[gpui::test] +fn a_match_shown_after_a_replacement_opens_selected(cx: &mut TestAppContext) { + let (document, cx) = open("a b a\n", MarkdownMode::Visual, cx); + document.update_in(cx, |document, window, cx| { + document.replace(vec![(0..1, "x".into())], cx); + document.show_match(4..5, window, cx); + }); + cx.run_until_parked(); + cx.simulate_input("z"); + assert_eq!(text(&document, cx), "x b z\n"); +} + +#[gpui::test] +fn preview_takes_no_edits(cx: &mut TestAppContext) { + let (document, cx) = open("| A |\n| - |\n| 1 |\n", MarkdownMode::Preview, cx); + let refused = document.update(cx, |document, cx| { + ( + document.replace(vec![(2..3, "B".into())], cx), + document.edit_table(crate::documents::TableEdit::RowBelow, cx), + document.insert_table(cx), + ) + }); + let preview = Err(crate::documents::TableRefusal::Preview); + assert_eq!(refused, (false, preview, preview)); + assert_eq!(text(&document, cx), "| A |\n| - |\n| 1 |\n"); +} diff --git a/src/documents/document/visual.rs b/src/documents/document/visual.rs index 836f86f..e46c0e2 100644 --- a/src/documents/document/visual.rs +++ b/src/documents/document/visual.rs @@ -139,6 +139,7 @@ impl Render for MarkdownDocument { fn render(&mut self, window: &mut Window, cx: &mut Context) -> impl IntoElement { // A top line asked before its text was read. self.place_top(window, cx); + self.show_waiting(window, cx); let colors = cx.theme().colors.clone(); let root = div() .size_full() diff --git a/src/documents/mod.rs b/src/documents/mod.rs index 394fd8c..af4ebe5 100644 --- a/src/documents/mod.rs +++ b/src/documents/mod.rs @@ -34,7 +34,9 @@ pub use blocks::{Align, BlockData, BlockEditor, BlockEvent, BlockKind, Media}; pub use contents::outline; pub use counts::{ReadingTime, WordCount}; pub use diagram::Diagram; -pub use document::{DocumentEvent, DocumentHeading, LARGEST_READ, MarkdownDocument, TableEdit}; +pub use document::{ + DocumentEvent, DocumentHeading, LARGEST_READ, MarkdownDocument, TableEdit, TableRefusal, +}; pub use epub::{Chapter, EpubReader}; pub use format::{Format, format}; pub(crate) use history::Change;