From ede2b435d53894880c253d1f920bb4249e267af3 Mon Sep 17 00:00:00 2001 From: Cyan Date: Sun, 18 Oct 2026 17:23:57 +0800 Subject: [PATCH] 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())),] + ); +}