From e60a74e9cfc277e33451b8547c2d92d5e9259d16 Mon Sep 17 00:00:00 2001 From: Cyan Date: Sun, 18 Oct 2026 17:05:55 +0800 Subject: [PATCH 1/3] Clicking a row/column while in a formula now highlights that row/column --- crates/ui-egui/src/editor.rs | 44 ++++++++++++++----- crates/ui-egui/tests/formula_point_headers.rs | 4 ++ 2 files changed, 36 insertions(+), 12 deletions(-) diff --git a/crates/ui-egui/src/editor.rs b/crates/ui-egui/src/editor.rs index 4aada2953..f79169fce 100644 --- a/crates/ui-egui/src/editor.rs +++ b/crates/ui-egui/src/editor.rs @@ -208,20 +208,29 @@ pub fn formula_refs(text: &str) -> Vec<(usize, usize, RangeRef, Option, Tok::Sheet(a, _) => (Some(a.clone()), toks[i].start, i + 1), _ => (None, toks[i].start, i), }; - if let Some(Tok::Word(w)) = toks.get(j).map(|t| &t.tok) - && let Some(c) = CellRef::parse(w) - { - let mut end = toks[j].end; - let mut range = RangeRef::cell(c); - if let (Some(Tok::Colon | Tok::Trim(_)), Some(Tok::Word(w2))) = (toks.get(j + 1).map(|t| &t.tok), toks.get(j + 2).map(|t| &t.tok)) - && let Some(c2) = CellRef::parse(w2) - { - range = RangeRef::new(c, c2); - end = toks[j + 2].end; - i = j + 3; + if let Some(Tok::Word(w)) = toks.get(j).map(|t| &t.tok) { + let range_end = if matches!(toks.get(j + 1).map(|t| &t.tok), Some(Tok::Colon | Tok::Trim(_))) { + toks.get(j + 2).and_then(|token| { + let end = match &token.tok { + Tok::Word(w) => Some(w.clone()), + Tok::Number(n) if n.fract() == 0.0 && *n >= 1.0 => Some(format!("{n}")), + _ => None, + }?; + RangeRef::parse(&format!("{w}:{end}")).map(|range| (range, token.end)) + }) } else { + None + }; + let (range, end) = if let Some(range_end) = range_end { + i = j + 3; + range_end + } else if let Some(c) = CellRef::parse(w) { i = j + 1; - } + (RangeRef::cell(c), toks[j].end) + } else { + i += 1; + continue; + }; let key = body.get(start..end).unwrap_or("").replace('$', "").to_ascii_uppercase(); let idx = match seen.iter().position(|k| *k == key) { Some(k) => k, @@ -329,4 +338,15 @@ mod tests { assert_eq!(r[1].2.a1(), "B2:C3"); assert_eq!(r[3].3.as_deref(), Some("Sheet2")); } + + #[test] + fn whole_row_and_column_ref_spans() { + let refs = formula_refs("=SUM(C:C,4:6,$D:$E,Sheet2!7:8)"); + assert_eq!(refs.len(), 4); + assert_eq!(refs[0].2.a1(), "C:C"); + assert_eq!(refs[1].2.a1(), "4:6"); + assert_eq!(refs[2].2.a1(), "D:E"); + assert_eq!(refs[3].2.a1(), "7:8"); + assert_eq!(refs[3].3.as_deref(), Some("Sheet2")); + } } diff --git a/crates/ui-egui/tests/formula_point_headers.rs b/crates/ui-egui/tests/formula_point_headers.rs index 4313fb9f6..846fcb350 100644 --- a/crates/ui-egui/tests/formula_point_headers.rs +++ b/crates/ui-egui/tests/formula_point_headers.rs @@ -52,6 +52,8 @@ fn column_header_click_inserts_a_column_reference() { let app = h.state(); assert_eq!(app.editor.as_ref().map(|e| e.text.as_str()), Some("=SUM(C:C")); assert_eq!(app.session.active().unwrap().selection.current().a1(), "F8"); + let text = app.editor.as_ref().map_or("", |ed| ed.text.as_str()); + assert_eq!(gridcraft_ui_egui::editor::formula_refs(text).first().map(|r| r.2.a1()).as_deref(), Some("C:C")); } #[test] @@ -62,6 +64,8 @@ fn row_header_click_inserts_a_row_reference() { let app = h.state(); assert_eq!(app.editor.as_ref().map(|e| e.text.as_str()), Some("=SUM(4:4")); assert_eq!(app.session.active().unwrap().selection.current().a1(), "F8"); + let text = app.editor.as_ref().map_or("", |ed| ed.text.as_str()); + assert_eq!(gridcraft_ui_egui::editor::formula_refs(text).first().map(|r| r.2.a1()).as_deref(), Some("4:4")); } #[test] From ef0b243cd3d750af3ca16a85643a5317ba6b7231 Mon Sep 17 00:00:00 2001 From: Cyan Date: Sun, 18 Oct 2026 17:11:37 +0800 Subject: [PATCH 2/3] Allow multi-column/row selection inside formulas with shift --- crates/ui-egui/src/editor.rs | 28 +++++++++ crates/ui-egui/src/grid.rs | 35 ++++++++--- crates/ui-egui/tests/formula_point_headers.rs | 62 ++++++++++++++++++- 3 files changed, 115 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/editor.rs b/crates/ui-egui/src/editor.rs index f79169fce..7f8b60d29 100644 --- a/crates/ui-egui/src/editor.rs +++ b/crates/ui-egui/src/editor.rs @@ -107,6 +107,23 @@ impl EditState { self.sync_caret = true; } + /// Adds a reference to the current point-mode union, keeping earlier references intact. + pub fn insert_ref_union(&mut self, r: &str, separator: char) { + let caret_b = self.byte_caret(); + if !self.point.is_some_and(|(_, end)| end == caret_b) { + self.insert_ref(r); + return; + } + let (Some(before), Some(after)) = (self.text.get(..caret_b), self.text.get(caret_b..)) else { return }; + let start = caret_b + separator.len_utf8(); + let end = start + r.len(); + self.text = format!("{before}{separator}{r}{after}"); + self.point = Some((start, end)); + self.caret = self.text.get(..end).map(|s| s.chars().count()).unwrap_or(0); + self.caret_to_end = false; + self.sync_caret = true; + } + /// Updates function autocomplete for the word before the caret. pub fn update_autocomplete(&mut self, names: &[String]) { self.autocomplete.clear(); @@ -349,4 +366,15 @@ mod tests { assert_eq!(refs[3].2.a1(), "7:8"); assert_eq!(refs[3].3.as_deref(), Some("Sheet2")); } + + #[test] + fn inserting_a_union_keeps_each_reference_separately_pointed() { + let mut edit = EditState::new(0, CellRef::new(0, 0), "=SUM(".into(), true, false); + edit.insert_ref("C:C"); + edit.insert_ref_union("E:E", ','); + assert_eq!(edit.text, "=SUM(C:C,E:E"); + assert_eq!(formula_refs(&edit.text).iter().map(|r| r.2.a1()).collect::>(), ["C:C", "E:E"]); + assert_eq!(edit.point, Some((9, 12))); + assert!(edit.can_point()); + } } diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 90a39cb06..425ec6e93 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -2020,7 +2020,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & return; } let c = geo.col_at(sh, p.x); - if point_header(app, wb, sh, RangeRef::cols(c, c)) { + if point_header(app, wb, sh, RangeRef::cols(c, c), mods.shift, mods.ctrl) { return; } if resp.drag_started() @@ -2043,7 +2043,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & return; } let r = geo.row_at(sh, p.y); - if point_header(app, wb, sh, RangeRef::rows(r, r)) { + if point_header(app, wb, sh, RangeRef::rows(r, r), mods.shift, mods.ctrl) { return; } let range = if mods.shift { RangeRef::rows(sel.anchor.row.min(r), sel.anchor.row.max(r)) } else { RangeRef::rows(r, r) }; @@ -2334,12 +2334,33 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & keyboard(app, &ctx, resp, geo, sh); } -/// Point mode on a header: inserts the whole-column or whole-row reference instead of selecting. -fn point_header(app: &mut SheetApp, wb: &Workbook, sh: &Sheet, range: RangeRef) -> bool { +/// Point mode on a header: inserts a whole-row or whole-column reference instead of selecting. +fn point_header(app: &mut SheetApp, wb: &Workbook, sh: &Sheet, range: RangeRef, extend: bool, add: bool) -> bool { let Some(ed) = app.editor.as_mut().filter(|ed| ed.can_point()) else { return false }; - let r = range.a1(); - let r = if ed.sheet != wb.active_sheet { format!("{}!{r}", gridcraft_engine::formula::quote_sheet(&sh.name)) } else { r }; - ed.insert_ref(&r); + let range = if extend { + ed.point + .and_then(|point| crate::editor::formula_refs(&ed.text).into_iter().find(|(start, end, ..)| (*start, *end) == point)) + .and_then(|(_, _, previous, _, _)| { + if range.is_full_cols() && previous.is_full_cols() { + Some(RangeRef::cols(previous.start.col.min(range.start.col), previous.end.col.max(range.end.col))) + } else if range.is_full_rows() && previous.is_full_rows() { + Some(RangeRef::rows(previous.start.row.min(range.start.row), previous.end.row.max(range.end.row))) + } else { + None + } + }) + .unwrap_or(range) + } else { + range + }; + let reference = range.a1(); + let reference = if ed.sheet != wb.active_sheet { format!("{}!{reference}", gridcraft_engine::formula::quote_sheet(&sh.name)) } else { reference }; + if add { + ed.insert_ref_union(&reference, ed.locale.list_separator()); + } else { + ed.insert_ref(&reference); + } + ed.point_cell = Some(range.start); ed.request_focus = true; true } diff --git a/crates/ui-egui/tests/formula_point_headers.rs b/crates/ui-egui/tests/formula_point_headers.rs index 846fcb350..dd13bd9b6 100644 --- a/crates/ui-egui/tests/formula_point_headers.rs +++ b/crates/ui-egui/tests/formula_point_headers.rs @@ -35,13 +35,17 @@ fn header_center(h: &egui_kittest::Harness<'_, SheetApp>, rows: bool, index: u32 } fn click(h: &mut egui_kittest::Harness<'_, SheetApp>, pos: Pos2) { + click_with_modifiers(h, pos, Modifiers::NONE); +} + +fn click_with_modifiers(h: &mut egui_kittest::Harness<'_, SheetApp>, pos: Pos2, modifiers: Modifiers) { + h.input_mut().events.push(Event::ModifiersChanged(modifiers)); for pressed in [true, false] { - h.input_mut() - .events - .extend([Event::PointerMoved(pos), Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }]); + h.input_mut().events.extend([Event::PointerMoved(pos), Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers }]); h.step(); } h.run_steps(2); + h.input_mut().events.push(Event::ModifiersChanged(Modifiers::NONE)); } #[test] @@ -77,3 +81,55 @@ fn a_second_header_click_replaces_the_pointed_reference() { click(&mut h, second); assert_eq!(h.state().editor.as_ref().map(|e| e.text.as_str()), Some("=SUM(E:E")); } + +#[test] +fn shift_selects_a_contiguous_column_range_while_editing_a_formula() { + let mut h = harness(); + let first = header_center(&h, false, 2); + click(&mut h, first); + let second = header_center(&h, false, 4); + click_with_modifiers(&mut h, second, Modifiers::SHIFT); + let app = h.state(); + let text = app.editor.as_ref().map_or("", |ed| ed.text.as_str()); + assert_eq!(text, "=SUM(C:E"); + assert_eq!(gridcraft_ui_egui::editor::formula_refs(text).iter().map(|r| r.2.a1()).collect::>(), ["C:E"]); +} + +#[test] +fn ctrl_selects_discontiguous_columns_while_editing_a_formula() { + let mut h = harness(); + let first = header_center(&h, false, 2); + click(&mut h, first); + let second = header_center(&h, false, 4); + click_with_modifiers(&mut h, second, Modifiers::CTRL); + let app = h.state(); + let text = app.editor.as_ref().map_or("", |ed| ed.text.as_str()); + assert_eq!(text, "=SUM(C:C,E:E"); + assert_eq!(gridcraft_ui_egui::editor::formula_refs(text).iter().map(|r| r.2.a1()).collect::>(), ["C:C", "E:E"]); +} + +#[test] +fn shift_selects_a_contiguous_row_range_while_editing_a_formula() { + let mut h = harness(); + let first = header_center(&h, true, 3); + click(&mut h, first); + let second = header_center(&h, true, 5); + click_with_modifiers(&mut h, second, Modifiers::SHIFT); + let app = h.state(); + let text = app.editor.as_ref().map_or("", |ed| ed.text.as_str()); + assert_eq!(text, "=SUM(4:6"); + assert_eq!(gridcraft_ui_egui::editor::formula_refs(text).iter().map(|r| r.2.a1()).collect::>(), ["4:6"]); +} + +#[test] +fn ctrl_selects_discontiguous_rows_while_editing_a_formula() { + let mut h = harness(); + let first = header_center(&h, true, 3); + click(&mut h, first); + let second = header_center(&h, true, 5); + click_with_modifiers(&mut h, second, Modifiers::CTRL); + let app = h.state(); + let text = app.editor.as_ref().map_or("", |ed| ed.text.as_str()); + assert_eq!(text, "=SUM(4:4,6:6"); + assert_eq!(gridcraft_ui_egui::editor::formula_refs(text).iter().map(|r| r.2.a1()).collect::>(), ["4:4", "6:6"]); +} From ede2b435d53894880c253d1f920bb4249e267af3 Mon Sep 17 00:00:00 2001 From: Cyan Date: Sun, 18 Oct 2026 17:23:57 +0800 Subject: [PATCH 3/3] Fxied CodeRabbit flags MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixed: On macOS, Cmd-click sets mods.command, not mods.ctrl. Both calls therefore pass false for add, so point_header replaces the reference instead of adding a column or row union. Pass mods.command, as the adjacent selection path does. egui defines command as Cmd on macOS and Ctrl elsewhere. (docs.rs) & Fixed: Point mode remains active when the user changes sheets. The extension closure combines whole-column or whole-row references by coordinates only and discards the previous reference’s sheet. A prior Sheet2!C:C reference can therefore be combined with column E on Sheet3, then emitted as Sheet3!C:E. Compare the previous reference’s effective sheet with the active sheet before extending it. If they differ, use only the clicked range. --- crates/ui-egui/src/grid.rs | 12 +++-- crates/ui-egui/tests/formula_point_headers.rs | 48 +++++++++++++++++-- 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/crates/ui-egui/src/grid.rs b/crates/ui-egui/src/grid.rs index 425ec6e93..54bd63050 100644 --- a/crates/ui-egui/src/grid.rs +++ b/crates/ui-egui/src/grid.rs @@ -2020,7 +2020,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & return; } let c = geo.col_at(sh, p.x); - if point_header(app, wb, sh, RangeRef::cols(c, c), mods.shift, mods.ctrl) { + if point_header(app, wb, sh, RangeRef::cols(c, c), mods.shift, mods.command) { return; } if resp.drag_started() @@ -2043,7 +2043,7 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & return; } let r = geo.row_at(sh, p.y); - if point_header(app, wb, sh, RangeRef::rows(r, r), mods.shift, mods.ctrl) { + if point_header(app, wb, sh, RangeRef::rows(r, r), mods.shift, mods.command) { return; } let range = if mods.shift { RangeRef::rows(sel.anchor.row.min(r), sel.anchor.row.max(r)) } else { RangeRef::rows(r, r) }; @@ -2337,10 +2337,16 @@ fn interact(app: &mut SheetApp, ui: &mut egui::Ui, resp: &egui::Response, geo: & /// Point mode on a header: inserts a whole-row or whole-column reference instead of selecting. fn point_header(app: &mut SheetApp, wb: &Workbook, sh: &Sheet, range: RangeRef, extend: bool, add: bool) -> bool { let Some(ed) = app.editor.as_mut().filter(|ed| ed.can_point()) else { return false }; + let edit_sheet = ed.sheet; + let active_sheet = wb.active_sheet; let range = if extend { ed.point .and_then(|point| crate::editor::formula_refs(&ed.text).into_iter().find(|(start, end, ..)| (*start, *end) == point)) - .and_then(|(_, _, previous, _, _)| { + .and_then(|(_, _, previous, previous_sheet, _)| { + let previous_sheet = previous_sheet.as_deref().map(|name| wb.sheet_index(name)).unwrap_or(Some(edit_sheet)); + if previous_sheet != Some(active_sheet) { + return None; + } if range.is_full_cols() && previous.is_full_cols() { Some(RangeRef::cols(previous.start.col.min(range.start.col), previous.end.col.max(range.end.col))) } else if range.is_full_rows() && previous.is_full_rows() { diff --git a/crates/ui-egui/tests/formula_point_headers.rs b/crates/ui-egui/tests/formula_point_headers.rs index dd13bd9b6..97bd790d2 100644 --- a/crates/ui-egui/tests/formula_point_headers.rs +++ b/crates/ui-egui/tests/formula_point_headers.rs @@ -96,12 +96,12 @@ fn shift_selects_a_contiguous_column_range_while_editing_a_formula() { } #[test] -fn ctrl_selects_discontiguous_columns_while_editing_a_formula() { +fn command_selects_discontiguous_columns_while_editing_a_formula() { let mut h = harness(); let first = header_center(&h, false, 2); click(&mut h, first); let second = header_center(&h, false, 4); - click_with_modifiers(&mut h, second, Modifiers::CTRL); + click_with_modifiers(&mut h, second, Modifiers::COMMAND); let app = h.state(); let text = app.editor.as_ref().map_or("", |ed| ed.text.as_str()); assert_eq!(text, "=SUM(C:C,E:E"); @@ -122,14 +122,54 @@ fn shift_selects_a_contiguous_row_range_while_editing_a_formula() { } #[test] -fn ctrl_selects_discontiguous_rows_while_editing_a_formula() { +fn command_selects_discontiguous_rows_while_editing_a_formula() { let mut h = harness(); let first = header_center(&h, true, 3); click(&mut h, first); let second = header_center(&h, true, 5); - click_with_modifiers(&mut h, second, Modifiers::CTRL); + click_with_modifiers(&mut h, second, Modifiers::COMMAND); let app = h.state(); let text = app.editor.as_ref().map_or("", |ed| ed.text.as_str()); assert_eq!(text, "=SUM(4:4,6:6"); assert_eq!(gridcraft_ui_egui::editor::formula_refs(text).iter().map(|r| r.2.a1()).collect::>(), ["4:4", "6:6"]); } + +#[test] +fn shift_does_not_extend_a_column_reference_across_sheets() { + let mut h = harness(); + h.state_mut().session.execute("home.insertSheet", json!({"name": "Other sheet"})).unwrap(); + h.state_mut().session.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + h.run_steps(2); + let first = header_center(&h, false, 2); + click(&mut h, first); + h.state_mut().session.execute("sheet.activate", json!({"sheet": "Other sheet"})).unwrap(); + h.run_steps(2); + let second = header_center(&h, false, 4); + click_with_modifiers(&mut h, second, Modifiers::SHIFT); + let text = h.state().editor.as_ref().map_or("", |ed| ed.text.as_str()); + assert_eq!(text, "=SUM('Other sheet'!E:E"); + assert_eq!( + gridcraft_ui_egui::editor::formula_refs(text).iter().map(|r| (r.2.a1(), r.3.clone())).collect::>(), + [("E:E".into(), Some("Other sheet".into())),] + ); +} + +#[test] +fn shift_does_not_extend_a_row_reference_across_sheets() { + let mut h = harness(); + h.state_mut().session.execute("home.insertSheet", json!({"name": "Other sheet"})).unwrap(); + h.state_mut().session.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + h.run_steps(2); + let first = header_center(&h, true, 3); + click(&mut h, first); + h.state_mut().session.execute("sheet.activate", json!({"sheet": "Other sheet"})).unwrap(); + h.run_steps(2); + let second = header_center(&h, true, 5); + click_with_modifiers(&mut h, second, Modifiers::SHIFT); + let text = h.state().editor.as_ref().map_or("", |ed| ed.text.as_str()); + assert_eq!(text, "=SUM('Other sheet'!6:6"); + assert_eq!( + gridcraft_ui_egui::editor::formula_refs(text).iter().map(|r| (r.2.a1(), r.3.clone())).collect::>(), + [("6:6".into(), Some("Other sheet".into())),] + ); +}