From 7980970ed68a071d72716f3fce1f8b2d374649ae Mon Sep 17 00:00:00 2001 From: sedax90 <5001801+sedax90@users.noreply.github.com> Date: Thu, 29 Oct 2026 14:46:40 +0100 Subject: [PATCH 01/11] feat(linux-grid): sum, average and count the selected rows' focused column --- linux/Cargo.lock | 1 + linux/crates/app/Cargo.toml | 2 + linux/crates/app/src/services/grid/mod.rs | 2 + .../src/services/grid/selection_summary.rs | 441 ++++++++++++++++ linux/crates/app/src/ui/browse_tab/imp.rs | 2 + linux/crates/app/src/ui/browse_tab/layout.rs | 2 + .../app/src/ui/data_grid/grid_actions.rs | 1 + linux/crates/app/src/ui/data_grid/imp.rs | 2 + linux/crates/app/src/ui/data_grid/mod.rs | 1 + .../app/src/ui/data_grid/selection_summary.rs | 474 ++++++++++++++++++ linux/crates/app/src/ui/format/mod.rs | 2 +- linux/crates/app/src/ui/format/thousands.rs | 32 ++ linux/crates/app/src/ui/result_pane/mod.rs | 5 +- linux/data/resources/ui/browse-tab.ui | 2 +- linux/po/POTFILES.in | 2 + 15 files changed, 968 insertions(+), 3 deletions(-) create mode 100644 linux/crates/app/src/services/grid/selection_summary.rs create mode 100644 linux/crates/app/src/ui/data_grid/selection_summary.rs diff --git a/linux/Cargo.lock b/linux/Cargo.lock index 1714d8d671..5fbd303eb5 100644 --- a/linux/Cargo.lock +++ b/linux/Cargo.lock @@ -5551,6 +5551,7 @@ dependencies = [ "icu_locale_core", "libadwaita", "log", + "num-bigint", "proptest", "pulldown-cmark", "relm4", diff --git a/linux/crates/app/Cargo.toml b/linux/crates/app/Cargo.toml index 414dd20bde..5187a3365d 100644 --- a/linux/crates/app/Cargo.toml +++ b/linux/crates/app/Cargo.toml @@ -100,6 +100,8 @@ gettext-rs = { version = "0.8", features = ["gettext-system"] } # Counts grouped the way the user's locale groups digits. icu_decimal = { version = "2.3", features = ["alloc"] } icu_locale_core = { version = "2.3", features = ["alloc"] } +# A selection's sum and average, exact past what i128 holds. +num-bigint.workspace = true # The assistant's replies are Markdown; the HTML writer stays off. pulldown-cmark = { version = "0.13", default-features = false } sqlformat = "0.5" diff --git a/linux/crates/app/src/services/grid/mod.rs b/linux/crates/app/src/services/grid/mod.rs index 69b283298e..7ba15fab7f 100644 --- a/linux/crates/app/src/services/grid/mod.rs +++ b/linux/crates/app/src/services/grid/mod.rs @@ -19,6 +19,7 @@ mod grid_column; mod grid_row_id; mod row_key_builder; mod row_snapshot; +mod selection_summary; #[cfg(test)] mod test_columns; pub mod value_order; @@ -43,3 +44,4 @@ pub use self::grid_column::{CellDisplay, GridColumn, grid_columns}; pub use self::grid_row_id::GridRowId; pub use self::row_key_builder::RowKeyBuilder; pub use self::row_snapshot::RowSnapshot; +pub use self::selection_summary::{NumberFigures, SelectionSummary, SummaryRule, SummaryTally}; diff --git a/linux/crates/app/src/services/grid/selection_summary.rs b/linux/crates/app/src/services/grid/selection_summary.rs new file mode 100644 index 0000000000..207a886fa5 --- /dev/null +++ b/linux/crates/app/src/services/grid/selection_summary.rs @@ -0,0 +1,441 @@ +use std::cmp::Ordering; + +use num_bigint::BigInt; +use tablepro_core::Value; + +use super::{CellDisplay, GridColumn}; + +/// An exponent past this reads as text, not a number: `1e999999` would +/// otherwise take megabytes of digits to add up. +const MAX_EXPONENT: i64 = 400; +/// The most digits a value may write out and still add up as a number, +/// PostgreSQL's widest numeric. A longer text cell of an untyped column +/// is text: adding a million-digit fraction would hold the main thread. +const MAX_DIGITS: usize = 1_000; +/// The average carries this many digits past the column's own, and drops +/// the zeros it does not need. +const AVERAGE_EXTRA_DIGITS: u32 = 4; + +/// How a column's values add up. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SummaryRule { + /// A number type: a value that is not a number is counted as one. + Numeric, + /// No declared type, so text that reads as a number is one. Typed + /// text holds identifiers like ZIP codes and is never summed. + NumericIfParsable, + CountOnly, +} + +impl SummaryRule { + pub fn for_column(column: &GridColumn) -> Self { + if column.column_type.kind.is_untyped() { + return Self::NumericIfParsable; + } + match column.display() { + CellDisplay::Number => Self::Numeric, + _ => Self::CountOnly, + } + } +} + +/// A decimal number held exactly, as `mantissa × 10^-scale`, so a sum of +/// cents stays in cents and `0.1 + 0.2` is `0.3`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ExactNumber { + mantissa: BigInt, + scale: u32, +} + +impl ExactNumber { + /// Plain decimal text, an optional sign and exponent included, with + /// ASCII blanks around it. `None` for anything else, `NaN` and + /// `Infinity` among them. + pub fn parse(text: &str) -> Option { + let text = text.trim_ascii(); + let (negative, rest) = match text.as_bytes().first()? { + b'-' => (true, &text[1..]), + b'+' => (false, &text[1..]), + _ => (false, text), + }; + let (number, exponent) = match rest.find(['e', 'E']) { + Some(at) => (&rest[..at], rest[at + 1..].parse::().ok()?), + None => (rest, 0), + }; + if !(-MAX_EXPONENT..=MAX_EXPONENT).contains(&exponent) { + return None; + } + let (whole, fraction) = number.split_once('.').unwrap_or((number, "")); + if whole.is_empty() && fraction.is_empty() { + return None; + } + if whole.len() + fraction.len() > MAX_DIGITS + || !whole.bytes().chain(fraction.bytes()).all(|byte| byte.is_ascii_digit()) + { + return None; + } + let digits = format!("{whole}{fraction}"); + let mut mantissa: BigInt = digits.parse().ok()?; + if negative { + mantissa = -mantissa; + } + let scale = i64::try_from(fraction.len()).ok()? - exponent; + Some(match u32::try_from(scale) { + Ok(scale) => Self { mantissa, scale }, + Err(_) => Self { + mantissa: mantissa * pow10(u32::try_from(-scale).ok()?), + scale: 0, + }, + }) + } + + /// The number of a value that is one, by its exact text. + pub fn of_value(value: &Value) -> Option { + match value { + Value::Int(n) => Some(Self::integer(BigInt::from(*n))), + Value::UInt(n) => Some(Self::integer(BigInt::from(*n))), + Value::WideInt(n) => Self::parse(&n.to_string()), + Value::Decimal(n) => Self::parse(&n.to_string()), + // The shortest text that reads back as the same float, so a + // 0.1 stored as a double adds as the 0.1 the cell shows. + Value::Float32(n) if n.is_finite() => Self::parse(&n.to_string()), + Value::Float64(n) if n.is_finite() => Self::parse(&n.to_string()), + Value::Text(text) => Self::parse(text), + _ => None, + } + } + + fn integer(mantissa: BigInt) -> Self { + Self { mantissa, scale: 0 } + } + + pub fn scale(&self) -> u32 { + self.scale + } + + fn rescaled(&self, scale: u32) -> BigInt { + match scale.checked_sub(self.scale) { + Some(0) | None => self.mantissa.clone(), + Some(more) => &self.mantissa * pow10(more), + } + } + + fn add(&self, other: &Self) -> Self { + let scale = self.scale.max(other.scale); + Self { + mantissa: self.rescaled(scale) + other.rescaled(scale), + scale, + } + } + + fn cmp_value(&self, other: &Self) -> Ordering { + let scale = self.scale.max(other.scale); + self.rescaled(scale).cmp(&other.rescaled(scale)) + } + + /// `self ÷ count` to `scale` digits, half away from zero. + fn divided(&self, count: u64, scale: u32) -> Self { + let numerator = self.rescaled(scale); + let denominator = BigInt::from(count); + let quotient: BigInt = &numerator / &denominator; + let remainder: BigInt = &numerator % &denominator; + let doubled_remainder = remainder.magnitude() * 2u32; + let rounded = match doubled_remainder >= *denominator.magnitude() { + true if self.mantissa < BigInt::ZERO => quotient - 1, + true => quotient + 1, + false => quotient, + }; + Self { + mantissa: rounded, + scale, + } + } + + /// The same number with no trailing fraction zeros, counted from its + /// digits and divided out at once. + fn trimmed(&self) -> Self { + if self.mantissa == BigInt::ZERO { + return Self::integer(BigInt::ZERO); + } + let digits = self.mantissa.magnitude().to_string(); + let zeros = digits.bytes().rev().take_while(|digit| *digit == b'0').count(); + let drop = u32::try_from(zeros).unwrap_or(u32::MAX).min(self.scale); + Self { + mantissa: &self.mantissa / pow10(drop), + scale: self.scale - drop, + } + } + + /// The number written out with at least `scale` fraction digits and + /// no grouping, as SQL and a spreadsheet read it in any locale. + pub fn plain_text(&self, scale: u32) -> String { + let scale = scale.max(self.scale); + let mantissa = self.rescaled(scale); + let digits = mantissa.magnitude().to_string(); + let sign = if mantissa < BigInt::ZERO { "-" } else { "" }; + let scale = scale as usize; + if scale == 0 { + return format!("{sign}{digits}"); + } + let digits = format!("{digits:0>width$}", width = scale + 1); + let (whole, fraction) = digits.split_at(digits.len() - scale); + format!("{sign}{whole}.{fraction}") + } +} + +fn pow10(exponent: u32) -> BigInt { + BigInt::from(10).pow(exponent) +} + +/// What the numbers among the summed values come to. Sum, minimum and +/// maximum keep the widest scale among them, so cents stay cents. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct NumberFigures { + pub count: u64, + pub sum: ExactNumber, + pub minimum: ExactNumber, + pub maximum: ExactNumber, + pub average: ExactNumber, + pub scale: u32, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SelectionSummary { + /// The values that are not empty. + pub value_count: u64, + /// NULLs, and a new row's cells left to the server's default. + pub empty_count: u64, + /// Values of a number column that are not numbers, such as `NaN`. + pub not_a_number_count: u64, + pub numbers: Option, +} + +/// Adds the values of the summarized cells one at a time. +#[derive(Debug, Default)] +pub struct SummaryTally { + value_count: u64, + empty_count: u64, + not_a_number_count: u64, + count: u64, + sum: Option, + minimum: Option, + maximum: Option, +} + +impl SummaryTally { + /// `default` is a new row's cell left to the server, which holds no + /// value yet. + pub fn add(&mut self, value: &Value, default: bool, rule: SummaryRule) { + if default || value.is_null() { + self.empty_count += 1; + return; + } + self.value_count += 1; + let number = match rule { + SummaryRule::CountOnly => return, + SummaryRule::Numeric | SummaryRule::NumericIfParsable => ExactNumber::of_value(value), + }; + match number { + Some(number) => self.add_number(number), + None if rule == SummaryRule::Numeric => self.not_a_number_count += 1, + None => {} + } + } + + fn add_number(&mut self, number: ExactNumber) { + self.count += 1; + self.sum = Some(match &self.sum { + Some(sum) => sum.add(&number), + None => number.clone(), + }); + if self + .minimum + .as_ref() + .is_none_or(|minimum| number.cmp_value(minimum) == Ordering::Less) + { + self.minimum = Some(number.clone()); + } + if self + .maximum + .as_ref() + .is_none_or(|maximum| number.cmp_value(maximum) == Ordering::Greater) + { + self.maximum = Some(number); + } + } + + pub fn summary(self) -> SelectionSummary { + let numbers = match (self.sum, self.minimum, self.maximum) { + (Some(sum), Some(minimum), Some(maximum)) => { + let scale = sum.scale(); + let average = sum.divided(self.count, scale + AVERAGE_EXTRA_DIGITS).trimmed(); + Some(NumberFigures { + count: self.count, + sum, + minimum, + maximum, + average, + scale, + }) + } + _ => None, + }; + SelectionSummary { + value_count: self.value_count, + empty_count: self.empty_count, + not_a_number_count: self.not_a_number_count, + numbers, + } + } +} + +#[cfg(test)] +mod tests { + use tablepro_core::Value; + use tablepro_core::column::{ColumnKind, IntegerKind, IntegerWidth, TextLength}; + use tablepro_core::value::SqlDecimal; + + use super::{ExactNumber, SummaryRule, SummaryTally}; + use crate::services::grid::test_columns::result_only; + + fn summed(values: &[Value], rule: SummaryRule) -> super::SelectionSummary { + let mut tally = SummaryTally::default(); + for value in values { + tally.add(value, false, rule); + } + tally.summary() + } + + fn plain(number: &ExactNumber, scale: u32) -> String { + number.plain_text(scale) + } + + #[test] + fn decimals_add_exactly_and_keep_their_scale() { + let cents = |text: &str| Value::Decimal(text.parse::().expect("a decimal")); + let summary = summed(&[cents("0.10"), cents("0.20"), cents("19.99")], SummaryRule::Numeric); + let numbers = summary.numbers.expect("numbers"); + + assert_eq!(numbers.count, 3); + assert_eq!(plain(&numbers.sum, numbers.scale), "20.29"); + assert_eq!(plain(&numbers.minimum, numbers.scale), "0.10"); + assert_eq!(plain(&numbers.maximum, numbers.scale), "19.99"); + assert_eq!(plain(&numbers.average, 0), "6.763333", "four digits past the cents"); + } + + #[test] + fn floats_add_as_the_text_the_cell_shows() { + let summary = summed(&[Value::Float64(0.1), Value::Float64(0.2)], SummaryRule::Numeric); + let numbers = summary.numbers.expect("numbers"); + + assert_eq!(plain(&numbers.sum, numbers.scale), "0.3"); + assert_eq!(plain(&numbers.average, 0), "0.15"); + } + + #[test] + fn an_average_with_no_fraction_drops_its_zeros() { + let summary = summed(&[Value::Int(2), Value::Int(4), Value::Int(-9)], SummaryRule::Numeric); + let numbers = summary.numbers.expect("numbers"); + + assert_eq!(plain(&numbers.sum, numbers.scale), "-3"); + assert_eq!(plain(&numbers.average, 0), "-1"); + assert_eq!(plain(&numbers.minimum, 0), "-9"); + let summary = summed(&[Value::Int(-1), Value::Int(-2)], SummaryRule::Numeric); + assert_eq!(plain(&summary.numbers.expect("numbers").average, 0), "-1.5"); + let summary = summed(&[Value::Int(-2), Value::Int(-1), Value::Int(0)], SummaryRule::Numeric); + assert_eq!(plain(&summary.numbers.expect("numbers").average, 0), "-1"); + let summary = summed(&[Value::Int(1), Value::Int(1), Value::Int(0)], SummaryRule::Numeric); + assert_eq!( + plain(&summary.numbers.expect("numbers").average, 0), + "0.6667", + "half away from zero at the fourth digit" + ); + } + + #[test] + fn nulls_are_empty_and_a_nan_is_not_a_number() { + let nan = Value::Decimal(SqlDecimal::nan()); + let mut tally = SummaryTally::default(); + tally.add(&Value::Int(5), false, SummaryRule::Numeric); + tally.add(&Value::Null, false, SummaryRule::Numeric); + tally.add(&Value::Null, true, SummaryRule::Numeric); + tally.add(&nan, false, SummaryRule::Numeric); + tally.add(&Value::Float64(f64::INFINITY), false, SummaryRule::Numeric); + let summary = tally.summary(); + + assert_eq!(summary.value_count, 3); + assert_eq!(summary.empty_count, 2); + assert_eq!(summary.not_a_number_count, 2); + assert_eq!(summary.numbers.expect("numbers").count, 1); + } + + #[test] + fn text_is_summed_only_in_a_column_of_no_declared_type() { + let values = [ + Value::Text("12".into()), + Value::Text(" 3.5 ".into()), + Value::Text("n/a".into()), + ]; + + let untyped = summed(&values, SummaryRule::NumericIfParsable); + assert_eq!(untyped.value_count, 3); + assert_eq!( + untyped.not_a_number_count, 0, + "free text in an untyped column is just text" + ); + let numbers = untyped.numbers.expect("numbers"); + assert_eq!(plain(&numbers.sum, numbers.scale), "15.5"); + + let typed = summed(&values, SummaryRule::CountOnly); + assert_eq!(typed.value_count, 3); + assert!(typed.numbers.is_none()); + } + + #[test] + fn the_rule_follows_the_column_type() { + let integer = ColumnKind::Integer(IntegerKind { + width: IntegerWidth::W64, + signed: true, + }); + let zip = ColumnKind::Text { + length: TextLength::Varying(5), + }; + + assert_eq!( + SummaryRule::for_column(&result_only("n", integer)), + SummaryRule::Numeric + ); + assert_eq!( + SummaryRule::for_column(&result_only("zip", zip)), + SummaryRule::CountOnly + ); + let text = ColumnKind::Text { + length: TextLength::Unbounded, + }; + assert_eq!(SummaryRule::for_column(&result_only("t", text)), SummaryRule::CountOnly); + assert_eq!( + SummaryRule::for_column(&result_only("x", ColumnKind::Other)), + SummaryRule::NumericIfParsable + ); + } + + #[test] + fn number_text_parses_exactly() { + let parsed = |text: &str| ExactNumber::parse(text).map(|number| number.plain_text(0)); + + assert_eq!(parsed("1e3").as_deref(), Some("1000")); + assert_eq!(parsed("-2.5E-2").as_deref(), Some("-0.025")); + assert_eq!(parsed(".5").as_deref(), Some("0.5")); + assert_eq!(parsed("7.").as_deref(), Some("7")); + assert_eq!(parsed("1e999999"), None); + assert_eq!( + parsed(&format!("0.{}1", "0".repeat(1_000))), + None, + "past the widest numeric" + ); + assert_eq!(parsed(&"9".repeat(1_000)).map(|text| text.len()), Some(1_000)); + for text in ["", "-", ".", "NaN", "inf", "1,5", "0x10", "1e"] { + assert_eq!(parsed(text), None, "{text:?}"); + } + } +} diff --git a/linux/crates/app/src/ui/browse_tab/imp.rs b/linux/crates/app/src/ui/browse_tab/imp.rs index 0cbacd6ce8..b6c8158638 100644 --- a/linux/crates/app/src/ui/browse_tab/imp.rs +++ b/linux/crates/app/src/ui/browse_tab/imp.rs @@ -46,6 +46,8 @@ pub struct BrowseTab { #[template_child] pub selection_label: TemplateChild, #[template_child] + pub action_bar: TemplateChild, + #[template_child] pub more_button: TemplateChild, #[template_child] pub export_button: TemplateChild, diff --git a/linux/crates/app/src/ui/browse_tab/layout.rs b/linux/crates/app/src/ui/browse_tab/layout.rs index 20a97fe76a..81699c7048 100644 --- a/linux/crates/app/src/ui/browse_tab/layout.rs +++ b/linux/crates/app/src/ui/browse_tab/layout.rs @@ -152,6 +152,8 @@ pub(super) fn grid(tab: &BrowseTab, init: GridInit) -> DataGrid { grid.set_hexpand(true); let imp = tab.imp(); imp.grid_holder.append(&grid); + // After the selected-row count, which it sums. + imp.action_bar.pack_start(grid.selection_summary().widget()); let _ = imp.grid.set(grid.clone()); connect_grid(tab, &grid); grid diff --git a/linux/crates/app/src/ui/data_grid/grid_actions.rs b/linux/crates/app/src/ui/data_grid/grid_actions.rs index 201f64c4a2..71cd586a8b 100644 --- a/linux/crates/app/src/ui/data_grid/grid_actions.rs +++ b/linux/crates/app/src/ui/data_grid/grid_actions.rs @@ -176,6 +176,7 @@ impl DataGrid { self.edit().refresh_actions(self, target.as_ref()); copy::refresh_actions(self, target.as_ref()); references::refresh_actions(self, target.as_ref()); + self.refresh_summary(); find_bar::refresh_actions(self); } } diff --git a/linux/crates/app/src/ui/data_grid/imp.rs b/linux/crates/app/src/ui/data_grid/imp.rs index 28a482d82a..0dec7923ab 100644 --- a/linux/crates/app/src/ui/data_grid/imp.rs +++ b/linux/crates/app/src/ui/data_grid/imp.rs @@ -64,6 +64,8 @@ pub struct DataGrid { pub columns: RefCell, pub actions: gio::SimpleActionGroup, pub menus: OnceCell, + /// Made when a host first places it in a bar. + pub summary: OnceCell, pub edit: EditController, pub find: FindState, /// What a menu was opened on, until it closes. diff --git a/linux/crates/app/src/ui/data_grid/mod.rs b/linux/crates/app/src/ui/data_grid/mod.rs index eba768f692..a575970ef6 100644 --- a/linux/crates/app/src/ui/data_grid/mod.rs +++ b/linux/crates/app/src/ui/data_grid/mod.rs @@ -24,6 +24,7 @@ mod row_detail; mod row_json_dialog; mod row_number_column; mod selection; +mod selection_summary; mod server_sorter; mod table_keys; #[cfg(test)] diff --git a/linux/crates/app/src/ui/data_grid/selection_summary.rs b/linux/crates/app/src/ui/data_grid/selection_summary.rs new file mode 100644 index 0000000000..46c686a3c4 --- /dev/null +++ b/linux/crates/app/src/ui/data_grid/selection_summary.rs @@ -0,0 +1,474 @@ +use std::cell::RefCell; +use std::rc::Rc; + +use gtk4::prelude::*; +use gtk4::subclass::prelude::*; +use gtk4::{self as gtk, glib}; + +use super::DataGrid; +use super::tracked_values::RowPendingState; +use crate::i18n::{gettext, gettext_f}; +use crate::services::grid::{NumberFigures, SelectionSummary, SummaryRule, SummaryTally}; +use crate::ui::format::{format_decimal, format_thousands}; + +/// A summary of one row says no more than the cell, so it starts at two. +const MIN_ROWS: usize = 2; + +/// The figures of the focused column over the selected rows, as a +/// spreadsheet's status bar shows them, with every figure and a Copy +/// button for each in its popover. +#[derive(Clone)] +pub struct SummaryReadout { + button: gtk::MenuButton, + label: SummaryLadder, + heading: gtk::Label, + figures: gtk::Grid, + shown: Rc>>, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +struct Shown { + column: String, + summary: SelectionSummary, +} + +/// One line of the popover: what it is, the value as the locale writes +/// it, and the text Copy puts on the clipboard. +struct Figure { + title: String, + copy_title: String, + shown: String, + copied: String, +} + +impl SummaryReadout { + fn new(grid: &DataGrid) -> Self { + let label = SummaryLadder::new(); + let heading = gtk::Label::builder() + .xalign(0.0) + .ellipsize(gtk::pango::EllipsizeMode::End) + .build(); + heading.add_css_class("heading"); + let figures = gtk::Grid::builder().row_spacing(4).column_spacing(12).build(); + let content = gtk::Box::builder() + .orientation(gtk::Orientation::Vertical) + .spacing(8) + .margin_top(6) + .margin_bottom(6) + .margin_start(6) + .margin_end(6) + .build(); + content.append(&heading); + content.append(&figures); + let popover = gtk::Popover::builder().child(&content).build(); + let button = gtk::MenuButton::builder() + .child(&label) + .popover(&popover) + .visible(false) + .build(); + button.add_css_class("flat"); + button.update_property(&[gtk::accessible::Property::Description(&gettext( + "Shows every figure with a Copy button", + ))]); + let readout = Self { + button, + label, + heading, + figures, + shown: Rc::default(), + }; + let weak = grid.downgrade(); + let filled = readout.clone(); + popover.connect_show(move |_| { + if let Some(grid) = weak.upgrade() { + filled.fill_figures(&grid); + } + }); + readout + } + + pub(crate) fn widget(&self) -> >k::Widget { + self.button.upcast_ref() + } + + fn show(&self, grid: &DataGrid, shown: Option) { + if *self.shown.borrow() == shown { + return; + } + match &shown { + Some(shown) => { + let rungs = label_rungs(&shown.summary); + self.button.set_tooltip_text(Some(&rungs[0])); + self.button + .update_property(&[gtk::accessible::Property::Label(&rungs[0])]); + self.label.set_rungs(&rungs); + } + None => self.button.popdown(), + } + self.button.set_visible(shown.is_some()); + self.shown.replace(shown); + if self.button.popover().is_some_and(|popover| popover.is_visible()) { + self.fill_figures(grid); + } + } + + fn fill_figures(&self, grid: &DataGrid) { + while let Some(child) = self.figures.first_child() { + self.figures.remove(&child); + } + let Some(shown) = self.shown.borrow().clone() else { + return; + }; + self.heading.set_label(&shown.column); + let (counts, numbers) = figures(&shown.summary); + let mut line = 0; + for figure in counts { + self.attach_figure(grid, &figure, line); + line += 1; + } + if !numbers.is_empty() { + self.figures + .attach(>k::Separator::new(gtk::Orientation::Horizontal), 0, line, 3, 1); + line += 1; + } + for figure in numbers { + self.attach_figure(grid, &figure, line); + line += 1; + } + } + + fn attach_figure(&self, grid: &DataGrid, figure: &Figure, line: i32) { + let title = gtk::Label::builder().label(&figure.title).xalign(0.0).build(); + title.add_css_class("dimmed"); + let value = gtk::Label::builder() + .label(&figure.shown) + .xalign(1.0) + .hexpand(true) + .selectable(true) + .build(); + value.add_css_class("numeric"); + value.update_relation(&[gtk::accessible::Relation::LabelledBy(&[title.upcast_ref()])]); + let copy = gtk::Button::builder() + .icon_name("edit-copy-symbolic") + .tooltip_text(&figure.copy_title) + .build(); + copy.add_css_class("flat"); + copy.update_property(&[gtk::accessible::Property::Label(&figure.copy_title)]); + let text = figure.copied.clone(); + let weak = grid.downgrade(); + copy.connect_clicked(move |_| { + if let Some(grid) = weak.upgrade() { + crate::ui::clipboard::copy_text(&grid, text.clone(), |message| grid.toast(message)); + } + }); + self.figures.attach(&title, 0, line, 1, 1); + self.figures.attach(&value, 1, line, 1, 1); + self.figures.attach(©, 2, line, 1, 1); + } +} + +/// The bar's text from the widest to the narrowest. Digits are never cut: +/// a narrower bar drops figures instead, down to a sign that keeps the +/// popover in reach at every width. +fn label_rungs(summary: &SelectionSummary) -> [String; 3] { + let count = format_thousands(summary.value_count); + // Translators: the summation sign, the bar's sum when it has room for nothing else. + let sign = gettext("Σ"); + match &summary.numbers { + Some(numbers) => { + let sum = format_decimal(&numbers.sum.plain_text(numbers.scale)); + [ + gettext_f( + "Sum {sum}, Average {average}, Count {count}", + &[ + ("sum", &sum), + ("average", &format_decimal(&numbers.average.plain_text(0))), + ("count", &count), + ], + ), + gettext_f("Sum {sum}", &[("sum", &sum)]), + sign, + ] + } + None => { + let count = gettext_f("Count {count}", &[("count", &count)]); + [count.clone(), count, sign] + } + } +} + +mod ladder { + use std::cell::RefCell; + + use gtk4::prelude::*; + use gtk4::subclass::prelude::*; + use gtk4::{self as gtk, glib}; + + /// Shows the first of its labels that fits its width, the last one + /// when none does, so a long one never shows cut short. + #[derive(Default)] + pub struct SummaryLadder { + pub rungs: RefCell>, + } + + #[glib::object_subclass] + impl ObjectSubclass for SummaryLadder { + const NAME: &'static str = "TableProSummaryLadder"; + type Type = super::SummaryLadder; + type ParentType = gtk::Widget; + } + + impl ObjectImpl for SummaryLadder { + fn dispose(&self) { + for rung in self.rungs.take() { + rung.unparent(); + } + } + } + + impl WidgetImpl for SummaryLadder { + fn measure(&self, orientation: gtk::Orientation, _for_size: i32) -> (i32, i32, i32, i32) { + let rungs = self.rungs.borrow(); + let natural = |rung: >k::Label| rung.measure(orientation, -1).1; + match orientation { + gtk::Orientation::Horizontal => { + let widest = rungs.first().map_or(0, natural); + let narrowest = rungs.last().map_or(0, natural); + (narrowest, widest.max(narrowest), -1, -1) + } + _ => { + let tallest = rungs.iter().map(natural).max().unwrap_or(0); + (tallest, tallest, -1, -1) + } + } + } + + fn size_allocate(&self, width: i32, height: i32, baseline: i32) { + let rungs = self.rungs.borrow(); + let last = rungs.len().saturating_sub(1); + let chosen = rungs + .iter() + .position(|rung| rung.measure(gtk::Orientation::Horizontal, -1).1 <= width) + .unwrap_or(last); + for (index, rung) in rungs.iter().enumerate() { + rung.set_child_visible(index == chosen); + if index == chosen { + rung.allocate(width, height, baseline, None); + } + } + } + } +} + +glib::wrapper! { + pub struct SummaryLadder(ObjectSubclass) + @extends gtk::Widget, + @implements gtk::Accessible, gtk::Buildable, gtk::ConstraintTarget; +} + +impl SummaryLadder { + fn new() -> Self { + glib::Object::new() + } + + fn set_rungs(&self, texts: &[String]) { + let mut rungs = self.imp().rungs.borrow_mut(); + while rungs.len() < texts.len() { + // The button carries the whole text for a screen reader. + let rung = gtk::Label::builder() + .accessible_role(gtk::AccessibleRole::Presentation) + .build(); + rung.add_css_class("caption"); + rung.add_css_class("numeric"); + rung.set_parent(self); + rungs.push(rung); + } + for (rung, text) in rungs.iter().zip(texts) { + rung.set_label(text); + } + drop(rungs); + self.queue_resize(); + } + + /// The text it shows when nothing is short of room. + #[cfg(test)] + fn widest(&self) -> String { + self.imp() + .rungs + .borrow() + .first() + .map(|rung| rung.label().to_string()) + .unwrap_or_default() + } +} + +fn count_figure(title: String, copy_title: String, n: u64) -> Figure { + Figure { + title, + copy_title, + shown: format_thousands(n), + copied: n.to_string(), + } +} + +fn number_figure(title: String, copy_title: String, plain: String) -> Figure { + Figure { + title, + copy_title, + shown: format_decimal(&plain), + copied: plain, + } +} + +/// The counts, then the figures of the numbers among the values. +fn figures(summary: &SelectionSummary) -> (Vec
, Vec
) { + let mut counts = vec![ + count_figure(gettext("Count"), gettext("Copy Count"), summary.value_count), + count_figure( + // Translators: how many of the summed cells hold a number. + gettext("Numbers"), + gettext("Copy Number Count"), + summary.numbers.as_ref().map_or(0, |numbers| numbers.count), + ), + count_figure(gettext("Empty"), gettext("Copy Empty Count"), summary.empty_count), + ]; + if summary.not_a_number_count > 0 { + counts.push(count_figure( + gettext("Not a Number"), + gettext("Copy Not a Number Count"), + summary.not_a_number_count, + )); + } + let numbers = summary.numbers.as_ref().map_or_else(Vec::new, number_figures); + (counts, numbers) +} + +fn number_figures(numbers: &NumberFigures) -> Vec
{ + vec![ + number_figure( + gettext("Sum"), + gettext("Copy Sum"), + numbers.sum.plain_text(numbers.scale), + ), + number_figure( + gettext("Average"), + gettext("Copy Average"), + numbers.average.plain_text(0), + ), + number_figure( + gettext("Minimum"), + gettext("Copy Minimum"), + numbers.minimum.plain_text(numbers.scale), + ), + number_figure( + gettext("Maximum"), + gettext("Copy Maximum"), + numbers.maximum.plain_text(numbers.scale), + ), + ] +} + +impl DataGrid { + /// The readout for a host to place in its status bar. It shows only + /// once placed, so a grid no bar holds sums nothing. + pub(crate) fn selection_summary(&self) -> SummaryReadout { + let readout = self.imp().summary.get_or_init(|| SummaryReadout::new(self)).clone(); + self.refresh_summary(); + readout + } + + pub(super) fn refresh_summary(&self) { + if let Some(readout) = self.imp().summary.get() { + readout.show(self, self.summarize()); + } + } + + /// The focused column over the selected rows. Rows marked for + /// deletion are left out, and pending edits count as the cell shows + /// them. + fn summarize(&self) -> Option { + let rows = self.selected_rows(); + if rows.len() < MIN_ROWS { + return None; + } + let column = self.focus_target().resolve()?.cell?.1; + let columns = self.grid_columns(); + let grid_column = columns.iter().find(|candidate| candidate.index == column)?; + let rule = SummaryRule::for_column(grid_column); + let tracked = self.tracked(); + let mut tally = SummaryTally::default(); + for row in rows { + if tracked.row_state(&row) == RowPendingState::MarkedForDeletion { + continue; + } + tracked.with_value(&row, column, |value, default| tally.add(value, default, rule)); + } + Some(Shown { + column: grid_column.name.clone(), + summary: tally.summary(), + }) + } +} + +#[cfg(test)] +mod tests { + use gtk4::prelude::*; + use tablepro_core::Value; + + use crate::ui::data_grid::test_grid::{Grid, named}; + + fn numbers_and_names() -> Vec> { + let text = |name: &str| Value::Text(name.to_owned()); + vec![ + vec![Value::Int(10), text("a")], + vec![Value::Int(32), text("b")], + vec![Value::Null, text("c")], + ] + } + + #[gtk4::test] + fn summary_sums_the_focused_column_over_the_selected_rows() { + let grid = Grid::open( + vec![named("amount", "integer", false), named("name", "text", true)], + numbers_and_names(), + ); + let readout = grid.grid.selection_summary(); + assert!(!readout.widget().is_visible(), "nothing is selected yet"); + + grid.focus(0, 0); + grid.grid.selection().select_all(); + + assert!(readout.widget().is_visible()); + assert_eq!(readout.label.widest(), "Sum 42, Average 21, Count 2"); + } + + #[gtk4::test] + fn summary_only_counts_a_text_column() { + let grid = Grid::open( + vec![named("amount", "integer", false), named("name", "text", true)], + numbers_and_names(), + ); + let readout = grid.grid.selection_summary(); + + grid.focus(0, 1); + grid.grid.selection().select_all(); + + assert_eq!(readout.label.widest(), "Count 3"); + } + + #[gtk4::test] + fn summary_hides_for_a_single_row() { + let grid = Grid::open( + vec![named("amount", "integer", false), named("name", "text", true)], + numbers_and_names(), + ); + let readout = grid.grid.selection_summary(); + grid.focus(0, 0); + grid.grid.selection().select_all(); + assert!(readout.widget().is_visible()); + + grid.grid.selection().select_item(1, true); + + assert!(!readout.widget().is_visible()); + } +} diff --git a/linux/crates/app/src/ui/format/mod.rs b/linux/crates/app/src/ui/format/mod.rs index ade92482ee..b05034fe7f 100644 --- a/linux/crates/app/src/ui/format/mod.rs +++ b/linux/crates/app/src/ui/format/mod.rs @@ -4,7 +4,7 @@ mod thousands; #[cfg(test)] pub(crate) use self::thousands::Thousands; -pub(crate) use self::thousands::format_thousands; +pub(crate) use self::thousands::{format_decimal, format_thousands}; /// The count gettext picks a plural form by. It takes a u32, and a /// result past that is rare enough for the largest u32 to stand in. diff --git a/linux/crates/app/src/ui/format/thousands.rs b/linux/crates/app/src/ui/format/thousands.rs index b3395e50df..a77c58fd6d 100644 --- a/linux/crates/app/src/ui/format/thousands.rs +++ b/linux/crates/app/src/ui/format/thousands.rs @@ -36,6 +36,17 @@ impl Thousands { None => n.to_string(), } } + + /// `plain`, a decimal number written with a `.` and no grouping, + /// grouped and with the locale's decimal separator. Every fraction + /// digit stays, trailing zeros included, so cents read as cents. + pub(crate) fn format_decimal(&self, plain: &str) -> String { + let parsed = Decimal::try_from_str(plain); + match (&self.formatter, parsed) { + (Some(formatter), Ok(decimal)) => formatter.format_to_string(&decimal), + _ => plain.to_owned(), + } + } } // A cache, not state: every window formats with the process's locale, @@ -57,6 +68,11 @@ pub(crate) fn format_thousands(n: u64) -> String { USER.with(|thousands| thousands.format(n)) } +/// `plain` decimal text as the user's locale writes numbers. +pub(crate) fn format_decimal(plain: &str) -> String { + USER.with(|thousands| thousands.format_decimal(plain)) +} + #[cfg(test)] mod tests { use super::*; @@ -97,4 +113,20 @@ mod tests { assert_eq!(Thousands::for_posix_locale("").format(1_000), "1,000"); assert_eq!(Thousands::for_posix_locale("not a locale").format(1_000), "1,000"); } + + #[test] + fn a_decimal_keeps_its_fraction_digits_and_the_locale_separator() { + let english = Thousands::for_posix_locale("en_US.UTF-8"); + let italian = Thousands::for_posix_locale("it_IT.UTF-8"); + + assert_eq!(english.format_decimal("1234567.50"), "1,234,567.50"); + assert_eq!(english.format_decimal("-0.025"), "-0.025"); + assert_eq!(italian.format_decimal("12345.5"), "12.345,5"); + assert_eq!( + italian.format_decimal("1234.5"), + "1234,5", + "four digits stay together in Italian" + ); + assert_eq!(english.format_decimal("not a number"), "not a number"); + } } diff --git a/linux/crates/app/src/ui/result_pane/mod.rs b/linux/crates/app/src/ui/result_pane/mod.rs index fa6a62603d..85b4fd2409 100644 --- a/linux/crates/app/src/ui/result_pane/mod.rs +++ b/linux/crates/app/src/ui/result_pane/mod.rs @@ -69,7 +69,10 @@ pub(crate) fn result_set_page( let page = gtk::Box::builder().orientation(gtk::Orientation::Vertical).build(); page.append(banner.widget()); page.append(&grid); - page.append(&row_count_footer(view.row_count() as u64)); + let footer = gtk::Box::builder().spacing(6).build(); + footer.append(&row_count_footer(view.row_count() as u64)); + footer.append(grid.selection_summary().widget()); + page.append(&footer); (page.upcast(), grid) } diff --git a/linux/data/resources/ui/browse-tab.ui b/linux/data/resources/ui/browse-tab.ui index a997e126c5..53d43a0eb7 100644 --- a/linux/data/resources/ui/browse-tab.ui +++ b/linux/data/resources/ui/browse-tab.ui @@ -95,7 +95,7 @@ - + list-add-symbolic diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index af8dd18f8f..cf371f6f92 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -91,6 +91,7 @@ crates/app/src/services/grid/grid_row_id.rs crates/app/src/services/grid/mod.rs crates/app/src/services/grid/row_key_builder.rs crates/app/src/services/grid/row_snapshot.rs +crates/app/src/services/grid/selection_summary.rs crates/app/src/services/grid/test_columns.rs crates/app/src/services/grid/value_order.rs crates/app/src/services/history_availability.rs @@ -268,6 +269,7 @@ crates/app/src/ui/data_grid/row_detail/value_editor.rs crates/app/src/ui/data_grid/row_json_dialog.rs crates/app/src/ui/data_grid/row_number_column.rs crates/app/src/ui/data_grid/selection.rs +crates/app/src/ui/data_grid/selection_summary.rs crates/app/src/ui/data_grid/server_sorter.rs crates/app/src/ui/data_grid/table_keys.rs crates/app/src/ui/data_grid/test_grid.rs From 651a32d2841388fc2910f0e8ebece6f872780b85 Mon Sep 17 00:00:00 2001 From: sedax90 <5001801+sedax90@users.noreply.github.com> Date: Thu, 29 Oct 2026 14:47:44 +0100 Subject: [PATCH 02/11] docs(linux-grid): add the changelog entry --- linux/changelog.d/3877.added.md | 1 + 1 file changed, 1 insertion(+) create mode 100644 linux/changelog.d/3877.added.md diff --git a/linux/changelog.d/3877.added.md b/linux/changelog.d/3877.added.md new file mode 100644 index 0000000000..480bbe9b6e --- /dev/null +++ b/linux/changelog.d/3877.added.md @@ -0,0 +1 @@ +Selecting rows shows the sum, average and count of the focused column, with every figure ready to copy. From 7260bd371423ec3cee6dffd9cf5b387c8029018d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 29 Oct 2026 22:11:03 +0700 Subject: [PATCH 03/11] refactor(linux-grid): sum the selection off the GTK thread with stock widgets --- linux/crates/app/src/services/grid/mod.rs | 2 +- .../src/services/grid/selection_summary.rs | 528 +++++++------- .../app/src/services/grid/value_order.rs | 2 +- .../app/src/ui/browse_tab/grid_handlers.rs | 1 + linux/crates/app/src/ui/browse_tab/layout.rs | 3 +- .../app/src/ui/browse_tab/tests/load.rs | 29 + linux/crates/app/src/ui/data_grid/columns.rs | 1 + linux/crates/app/src/ui/data_grid/imp.rs | 4 +- linux/crates/app/src/ui/data_grid/mod.rs | 1 + .../app/src/ui/data_grid/selection_summary.rs | 661 ++++++++---------- .../crates/app/src/ui/data_grid/tests/mod.rs | 1 + .../app/src/ui/data_grid/tests/summary.rs | 255 +++++++ linux/crates/app/src/ui/key_tab/mod.rs | 1 + linux/crates/app/src/ui/key_tab/tests.rs | 35 + linux/crates/app/src/ui/result_pane/mod.rs | 4 +- linux/crates/core/src/value/decimal.rs | 25 + linux/po/POTFILES.in | 1 + 17 files changed, 900 insertions(+), 654 deletions(-) create mode 100644 linux/crates/app/src/ui/data_grid/tests/summary.rs diff --git a/linux/crates/app/src/services/grid/mod.rs b/linux/crates/app/src/services/grid/mod.rs index 7ba15fab7f..2252854d55 100644 --- a/linux/crates/app/src/services/grid/mod.rs +++ b/linux/crates/app/src/services/grid/mod.rs @@ -44,4 +44,4 @@ pub use self::grid_column::{CellDisplay, GridColumn, grid_columns}; pub use self::grid_row_id::GridRowId; pub use self::row_key_builder::RowKeyBuilder; pub use self::row_snapshot::RowSnapshot; -pub use self::selection_summary::{NumberFigures, SelectionSummary, SummaryRule, SummaryTally}; +pub use self::selection_summary::{SelectionSummary, SummaryRule, summarize}; diff --git a/linux/crates/app/src/services/grid/selection_summary.rs b/linux/crates/app/src/services/grid/selection_summary.rs index 207a886fa5..699ed1e1a3 100644 --- a/linux/crates/app/src/services/grid/selection_summary.rs +++ b/linux/crates/app/src/services/grid/selection_summary.rs @@ -1,118 +1,181 @@ +use std::borrow::Cow; use std::cmp::Ordering; use num_bigint::BigInt; use tablepro_core::Value; +use tablepro_core::value::SqlDecimal; +use super::value_order::decimal_of; use super::{CellDisplay, GridColumn}; -/// An exponent past this reads as text, not a number: `1e999999` would -/// otherwise take megabytes of digits to add up. -const MAX_EXPONENT: i64 = 400; -/// The most digits a value may write out and still add up as a number, -/// PostgreSQL's widest numeric. A longer text cell of an untyped column -/// is text: adding a million-digit fraction would hold the main thread. -const MAX_DIGITS: usize = 1_000; -/// The average carries this many digits past the column's own, and drops -/// the zeros it does not need. const AVERAGE_EXTRA_DIGITS: u32 = 4; -/// How a column's values add up. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum SummaryRule { - /// A number type: a value that is not a number is counted as one. - Numeric, - /// No declared type, so text that reads as a number is one. Typed - /// text holds identifiers like ZIP codes and is never summed. + /// `scale` is the column's declared one, which its cells are padded to. + /// A value that is not a number, such as NaN, is counted as one. + Numeric { scale: u32 }, + /// No declared type, so text that reads as a number is one. Typed text + /// holds identifiers like ZIP codes and is never summed. NumericIfParsable, CountOnly, } impl SummaryRule { pub fn for_column(column: &GridColumn) -> Self { - if column.column_type.kind.is_untyped() { + let kind = &column.column_type.kind; + if kind.is_untyped() { return Self::NumericIfParsable; } match column.display() { - CellDisplay::Number => Self::Numeric, + CellDisplay::Number => Self::Numeric { + scale: kind.decimal_scale().unwrap_or(0), + }, _ => Self::CountOnly, } } } -/// A decimal number held exactly, as `mantissa × 10^-scale`, so a sum of -/// cents stays in cents and `0.1 + 0.2` is `0.3`. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct ExactNumber { - mantissa: BigInt, - scale: u32, +pub struct SelectionSummary { + pub value_count: u64, + pub empty_count: u64, + pub not_a_number_count: u64, + pub numbers: Option, } -impl ExactNumber { - /// Plain decimal text, an optional sign and exponent included, with - /// ASCII blanks around it. `None` for anything else, `NaN` and - /// `Infinity` among them. - pub fn parse(text: &str) -> Option { - let text = text.trim_ascii(); - let (negative, rest) = match text.as_bytes().first()? { - b'-' => (true, &text[1..]), - b'+' => (false, &text[1..]), - _ => (false, text), - }; - let (number, exponent) = match rest.find(['e', 'E']) { - Some(at) => (&rest[..at], rest[at + 1..].parse::().ok()?), - None => (rest, 0), - }; - if !(-MAX_EXPONENT..=MAX_EXPONENT).contains(&exponent) { +/// Plain decimal text with no grouping, as SQL and a spreadsheet read it +/// in any locale. Sum, minimum and maximum keep the widest scale, so cents +/// stay cents; the average carries four more digits. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct NumberFigures { + pub count: u64, + pub sum: String, + pub average: String, + pub minimum: String, + pub maximum: String, +} + +/// `values` as their cells show them, with a cell left to its default as +/// NULL. `None` once `cancelled` says so. It asks before every value: one +/// of 100,000 digits takes milliseconds alone. +pub fn summarize(values: &[Value], rule: SummaryRule, cancelled: &dyn Fn() -> bool) -> Option { + let mut tally = Tally::default(); + for value in values { + if cancelled() { return None; } - let (whole, fraction) = number.split_once('.').unwrap_or((number, "")); - if whole.is_empty() && fraction.is_empty() { - return None; + tally.add(value, rule); + } + Some(tally.finish(rule)) +} + +#[derive(Default)] +struct Tally { + value_count: u64, + empty_count: u64, + not_a_number_count: u64, + count: u64, + sum: Exact, + minimum: Option, + maximum: Option, +} + +impl Tally { + fn add(&mut self, value: &Value, rule: SummaryRule) { + if value.is_null() { + self.empty_count += 1; + return; } - if whole.len() + fraction.len() > MAX_DIGITS - || !whole.bytes().chain(fraction.bytes()).all(|byte| byte.is_ascii_digit()) + self.value_count += 1; + if rule == SummaryRule::CountOnly { + return; + } + match number_of(value) { + Some(number) => self.add_number(number), + None if rule != SummaryRule::NumericIfParsable => self.not_a_number_count += 1, + None => {} + } + } + + fn add_number(&mut self, number: Exact) { + self.count += 1; + self.sum = self.sum.plus(&number); + if self + .minimum + .as_ref() + .is_none_or(|minimum| number.compare(minimum) == Ordering::Less) { - return None; + self.minimum = Some(number.clone()); } - let digits = format!("{whole}{fraction}"); - let mut mantissa: BigInt = digits.parse().ok()?; - if negative { - mantissa = -mantissa; + if self + .maximum + .as_ref() + .is_none_or(|maximum| number.compare(maximum) == Ordering::Greater) + { + self.maximum = Some(number); } - let scale = i64::try_from(fraction.len()).ok()? - exponent; - Some(match u32::try_from(scale) { - Ok(scale) => Self { mantissa, scale }, - Err(_) => Self { - mantissa: mantissa * pow10(u32::try_from(-scale).ok()?), - scale: 0, - }, - }) } - /// The number of a value that is one, by its exact text. - pub fn of_value(value: &Value) -> Option { - match value { - Value::Int(n) => Some(Self::integer(BigInt::from(*n))), - Value::UInt(n) => Some(Self::integer(BigInt::from(*n))), - Value::WideInt(n) => Self::parse(&n.to_string()), - Value::Decimal(n) => Self::parse(&n.to_string()), - // The shortest text that reads back as the same float, so a - // 0.1 stored as a double adds as the 0.1 the cell shows. - Value::Float32(n) if n.is_finite() => Self::parse(&n.to_string()), - Value::Float64(n) if n.is_finite() => Self::parse(&n.to_string()), - Value::Text(text) => Self::parse(text), + fn finish(self, rule: SummaryRule) -> SelectionSummary { + let declared = match rule { + SummaryRule::Numeric { scale } => scale, + SummaryRule::NumericIfParsable | SummaryRule::CountOnly => 0, + }; + let numbers = match (self.minimum, self.maximum) { + (Some(minimum), Some(maximum)) => { + let scale = self.sum.scale.max(declared); + let average = self.sum.divided(self.count, scale + AVERAGE_EXTRA_DIGITS); + Some(NumberFigures { + count: self.count, + sum: self.sum.text(scale), + average: average.trimmed().text(0), + minimum: minimum.text(scale), + maximum: maximum.text(scale), + }) + } _ => None, + }; + SelectionSummary { + value_count: self.value_count, + empty_count: self.empty_count, + not_a_number_count: self.not_a_number_count, + numbers, } } +} + +/// A number type's value, or text that reads as a number, at its exact +/// decimal. A float is the decimal its shortest text spells, which is +/// what its cell shows, so `0.1 + 0.2` is `0.3`. +fn number_of(value: &Value) -> Option { + let decimal = match value { + Value::Int(n) => return Some(Exact::integer(BigInt::from(*n))), + Value::UInt(n) => return Some(Exact::integer(BigInt::from(*n))), + Value::Text(text) => Cow::Owned(text.parse::().ok()?), + Value::Other(other) => Cow::Owned(other.text.parse::().ok()?), + _ => decimal_of(value)?, + }; + let (negative, digits, scale) = decimal.parts()?; + let magnitude = BigInt::parse_bytes(digits.as_bytes(), 10)?; + Some(Exact { + mantissa: if negative { -magnitude } else { magnitude }, + scale, + }) +} +/// `mantissa × 10^-scale`, with the arithmetic `SqlDecimal` leaves out. +#[derive(Debug, Clone, Default)] +struct Exact { + mantissa: BigInt, + scale: u32, +} + +impl Exact { fn integer(mantissa: BigInt) -> Self { Self { mantissa, scale: 0 } } - pub fn scale(&self) -> u32 { - self.scale - } - fn rescaled(&self, scale: u32) -> BigInt { match scale.checked_sub(self.scale) { Some(0) | None => self.mantissa.clone(), @@ -120,7 +183,7 @@ impl ExactNumber { } } - fn add(&self, other: &Self) -> Self { + fn plus(&self, other: &Self) -> Self { let scale = self.scale.max(other.scale); Self { mantissa: self.rescaled(scale) + other.rescaled(scale), @@ -128,7 +191,7 @@ impl ExactNumber { } } - fn cmp_value(&self, other: &Self) -> Ordering { + fn compare(&self, other: &Self) -> Ordering { let scale = self.scale.max(other.scale); self.rescaled(scale).cmp(&other.rescaled(scale)) } @@ -137,26 +200,18 @@ impl ExactNumber { fn divided(&self, count: u64, scale: u32) -> Self { let numerator = self.rescaled(scale); let denominator = BigInt::from(count); - let quotient: BigInt = &numerator / &denominator; - let remainder: BigInt = &numerator % &denominator; - let doubled_remainder = remainder.magnitude() * 2u32; - let rounded = match doubled_remainder >= *denominator.magnitude() { - true if self.mantissa < BigInt::ZERO => quotient - 1, + let quotient = &numerator / &denominator; + let remainder = &numerator % &denominator; + let rounds_away = remainder.magnitude() * 2u32 >= *denominator.magnitude(); + let mantissa = match rounds_away { + true if numerator < BigInt::ZERO => quotient - 1, true => quotient + 1, false => quotient, }; - Self { - mantissa: rounded, - scale, - } + Self { mantissa, scale } } - /// The same number with no trailing fraction zeros, counted from its - /// digits and divided out at once. fn trimmed(&self) -> Self { - if self.mantissa == BigInt::ZERO { - return Self::integer(BigInt::ZERO); - } let digits = self.mantissa.magnitude().to_string(); let zeros = digits.bytes().rev().take_while(|digit| *digit == b'0').count(); let drop = u32::try_from(zeros).unwrap_or(u32::MAX).min(self.scale); @@ -166,9 +221,8 @@ impl ExactNumber { } } - /// The number written out with at least `scale` fraction digits and - /// no grouping, as SQL and a spreadsheet read it in any locale. - pub fn plain_text(&self, scale: u32) -> String { + /// The number with at least `scale` fraction digits. + fn text(&self, scale: u32) -> String { let scale = scale.max(self.scale); let mantissa = self.rescaled(scale); let digits = mantissa.magnitude().to_string(); @@ -184,184 +238,114 @@ impl ExactNumber { } fn pow10(exponent: u32) -> BigInt { - BigInt::from(10).pow(exponent) + BigInt::from(10u32).pow(exponent) } -/// What the numbers among the summed values come to. Sum, minimum and -/// maximum keep the widest scale among them, so cents stay cents. -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct NumberFigures { - pub count: u64, - pub sum: ExactNumber, - pub minimum: ExactNumber, - pub maximum: ExactNumber, - pub average: ExactNumber, - pub scale: u32, -} +#[cfg(test)] +mod tests { + use std::cell::Cell; -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct SelectionSummary { - /// The values that are not empty. - pub value_count: u64, - /// NULLs, and a new row's cells left to the server's default. - pub empty_count: u64, - /// Values of a number column that are not numbers, such as `NaN`. - pub not_a_number_count: u64, - pub numbers: Option, -} + use tablepro_core::column::{ColumnKind, IntegerKind, IntegerWidth, TextLength}; + use tablepro_core::value::{OtherValue, SqlDecimal}; + use tablepro_core::Value; -/// Adds the values of the summarized cells one at a time. -#[derive(Debug, Default)] -pub struct SummaryTally { - value_count: u64, - empty_count: u64, - not_a_number_count: u64, - count: u64, - sum: Option, - minimum: Option, - maximum: Option, -} + use super::{NumberFigures, SelectionSummary, SummaryRule, summarize}; + use crate::services::grid::test_columns::result_only; -impl SummaryTally { - /// `default` is a new row's cell left to the server, which holds no - /// value yet. - pub fn add(&mut self, value: &Value, default: bool, rule: SummaryRule) { - if default || value.is_null() { - self.empty_count += 1; - return; - } - self.value_count += 1; - let number = match rule { - SummaryRule::CountOnly => return, - SummaryRule::Numeric | SummaryRule::NumericIfParsable => ExactNumber::of_value(value), - }; - match number { - Some(number) => self.add_number(number), - None if rule == SummaryRule::Numeric => self.not_a_number_count += 1, - None => {} - } + fn summed(values: &[Value], rule: SummaryRule) -> SelectionSummary { + summarize(values, rule, &|| false).expect("a run nobody cancelled") } - fn add_number(&mut self, number: ExactNumber) { - self.count += 1; - self.sum = Some(match &self.sum { - Some(sum) => sum.add(&number), - None => number.clone(), - }); - if self - .minimum - .as_ref() - .is_none_or(|minimum| number.cmp_value(minimum) == Ordering::Less) - { - self.minimum = Some(number.clone()); - } - if self - .maximum - .as_ref() - .is_none_or(|maximum| number.cmp_value(maximum) == Ordering::Greater) - { - self.maximum = Some(number); - } + fn numbers(values: &[Value], rule: SummaryRule) -> NumberFigures { + summed(values, rule).numbers.expect("numbers") } - pub fn summary(self) -> SelectionSummary { - let numbers = match (self.sum, self.minimum, self.maximum) { - (Some(sum), Some(minimum), Some(maximum)) => { - let scale = sum.scale(); - let average = sum.divided(self.count, scale + AVERAGE_EXTRA_DIGITS).trimmed(); - Some(NumberFigures { - count: self.count, - sum, - minimum, - maximum, - average, - scale, - }) - } - _ => None, - }; - SelectionSummary { - value_count: self.value_count, - empty_count: self.empty_count, - not_a_number_count: self.not_a_number_count, - numbers, - } + fn decimal(text: &str) -> Value { + Value::Decimal(text.parse::().expect("a decimal")) } -} -#[cfg(test)] -mod tests { - use tablepro_core::Value; - use tablepro_core::column::{ColumnKind, IntegerKind, IntegerWidth, TextLength}; - use tablepro_core::value::SqlDecimal; + const NUMERIC: SummaryRule = SummaryRule::Numeric { scale: 0 }; - use super::{ExactNumber, SummaryRule, SummaryTally}; - use crate::services::grid::test_columns::result_only; + #[test] + fn decimals_add_exactly_and_keep_their_scale() { + let figures = numbers(&[decimal("0.10"), decimal("0.20"), decimal("19.99")], NUMERIC); - fn summed(values: &[Value], rule: SummaryRule) -> super::SelectionSummary { - let mut tally = SummaryTally::default(); - for value in values { - tally.add(value, false, rule); - } - tally.summary() + assert_eq!(figures.count, 3); + assert_eq!(figures.sum, "20.29"); + assert_eq!(figures.minimum, "0.10"); + assert_eq!(figures.maximum, "19.99"); + assert_eq!(figures.average, "6.763333", "four digits past the cents"); } - fn plain(number: &ExactNumber, scale: u32) -> String { - number.plain_text(scale) + #[test] + fn the_declared_scale_pads_the_figures_as_it_pads_the_cells() { + let figures = numbers(&[decimal("5"), decimal("6")], SummaryRule::Numeric { scale: 2 }); + + assert_eq!(figures.sum, "11.00"); + assert_eq!(figures.minimum, "5.00"); + assert_eq!(figures.average, "5.5"); } #[test] - fn decimals_add_exactly_and_keep_their_scale() { - let cents = |text: &str| Value::Decimal(text.parse::().expect("a decimal")); - let summary = summed(&[cents("0.10"), cents("0.20"), cents("19.99")], SummaryRule::Numeric); - let numbers = summary.numbers.expect("numbers"); - - assert_eq!(numbers.count, 3); - assert_eq!(plain(&numbers.sum, numbers.scale), "20.29"); - assert_eq!(plain(&numbers.minimum, numbers.scale), "0.10"); - assert_eq!(plain(&numbers.maximum, numbers.scale), "19.99"); - assert_eq!(plain(&numbers.average, 0), "6.763333", "four digits past the cents"); + fn floats_add_as_the_text_the_cell_shows() { + let figures = numbers(&[Value::Float64(0.1), Value::Float64(0.2)], NUMERIC); + assert_eq!(figures.sum, "0.3"); + assert_eq!(figures.average, "0.15"); + + let figures = numbers(&[Value::Float32(0.1), Value::Float32(0.2)], NUMERIC); + assert_eq!(figures.sum, "0.3", "a float32 at its own width, not widened"); } #[test] - fn floats_add_as_the_text_the_cell_shows() { - let summary = summed(&[Value::Float64(0.1), Value::Float64(0.2)], SummaryRule::Numeric); - let numbers = summary.numbers.expect("numbers"); + fn the_average_rounds_half_away_from_zero_and_drops_its_zeros() { + assert_eq!( + numbers(&[Value::Int(2), Value::Int(4), Value::Int(-9)], NUMERIC).average, + "-1" + ); + assert_eq!(numbers(&[Value::Int(-1), Value::Int(-2)], NUMERIC).average, "-1.5"); + assert_eq!( + numbers(&[Value::Int(1), Value::Int(1), Value::Int(0)], NUMERIC).average, + "0.6667" + ); + assert_eq!( + numbers(&[Value::Int(-1), Value::Int(-1), Value::Int(0)], NUMERIC).average, + "-0.6667" + ); + } - assert_eq!(plain(&numbers.sum, numbers.scale), "0.3"); - assert_eq!(plain(&numbers.average, 0), "0.15"); + #[test] + fn integers_of_any_width_add_past_i128() { + let wide = "170141183460469231731687303715884105727" + .parse() + .map(Value::WideInt) + .expect("an i128::MAX"); + let figures = numbers(&[wide.clone(), wide, Value::UInt(u64::MAX)], NUMERIC); + + assert_eq!(figures.sum, "340282366920938463481821351505477763069"); } #[test] - fn an_average_with_no_fraction_drops_its_zeros() { - let summary = summed(&[Value::Int(2), Value::Int(4), Value::Int(-9)], SummaryRule::Numeric); - let numbers = summary.numbers.expect("numbers"); - - assert_eq!(plain(&numbers.sum, numbers.scale), "-3"); - assert_eq!(plain(&numbers.average, 0), "-1"); - assert_eq!(plain(&numbers.minimum, 0), "-9"); - let summary = summed(&[Value::Int(-1), Value::Int(-2)], SummaryRule::Numeric); - assert_eq!(plain(&summary.numbers.expect("numbers").average, 0), "-1.5"); - let summary = summed(&[Value::Int(-2), Value::Int(-1), Value::Int(0)], SummaryRule::Numeric); - assert_eq!(plain(&summary.numbers.expect("numbers").average, 0), "-1"); - let summary = summed(&[Value::Int(1), Value::Int(1), Value::Int(0)], SummaryRule::Numeric); + fn a_decimal_wider_than_a_thousand_digits_is_still_a_number() { + let wide = format!("1{}", "0".repeat(1_500)); + let summary = summed(&[decimal(&wide), Value::Int(1)], NUMERIC); + + assert_eq!(summary.not_a_number_count, 0); assert_eq!( - plain(&summary.numbers.expect("numbers").average, 0), - "0.6667", - "half away from zero at the fourth digit" + summary.numbers.expect("numbers").sum, + format!("1{}1", "0".repeat(1_499)) ); } #[test] fn nulls_are_empty_and_a_nan_is_not_a_number() { - let nan = Value::Decimal(SqlDecimal::nan()); - let mut tally = SummaryTally::default(); - tally.add(&Value::Int(5), false, SummaryRule::Numeric); - tally.add(&Value::Null, false, SummaryRule::Numeric); - tally.add(&Value::Null, true, SummaryRule::Numeric); - tally.add(&nan, false, SummaryRule::Numeric); - tally.add(&Value::Float64(f64::INFINITY), false, SummaryRule::Numeric); - let summary = tally.summary(); + let values = [ + Value::Int(5), + Value::Null, + Value::Null, + Value::Decimal(SqlDecimal::nan()), + Value::Float64(f64::INFINITY), + ]; + let summary = summed(&values, NUMERIC); assert_eq!(summary.value_count, 3); assert_eq!(summary.empty_count, 2); @@ -371,48 +355,58 @@ mod tests { #[test] fn text_is_summed_only_in_a_column_of_no_declared_type() { + let other = Value::Other(Box::new(OtherValue { + type_name: "Variant(UInt32, String)".to_owned(), + text: "4".to_owned(), + })); let values = [ Value::Text("12".into()), - Value::Text(" 3.5 ".into()), + Value::Text("\u{a0}3.5 ".into()), Value::Text("n/a".into()), + other, ]; let untyped = summed(&values, SummaryRule::NumericIfParsable); - assert_eq!(untyped.value_count, 3); - assert_eq!( - untyped.not_a_number_count, 0, - "free text in an untyped column is just text" - ); - let numbers = untyped.numbers.expect("numbers"); - assert_eq!(plain(&numbers.sum, numbers.scale), "15.5"); + assert_eq!(untyped.value_count, 4); + assert_eq!(untyped.not_a_number_count, 0, "free text in an untyped column is just text"); + assert_eq!(untyped.numbers.expect("numbers").sum, "19.5"); let typed = summed(&values, SummaryRule::CountOnly); - assert_eq!(typed.value_count, 3); + assert_eq!(typed.value_count, 4); assert!(typed.numbers.is_none()); } + #[test] + fn a_column_of_text_only_counts() { + let summary = summed(&[Value::Text("a".into()), Value::Null], SummaryRule::NumericIfParsable); + + assert_eq!((summary.value_count, summary.empty_count), (1, 1)); + assert!(summary.numbers.is_none()); + } + #[test] fn the_rule_follows_the_column_type() { let integer = ColumnKind::Integer(IntegerKind { width: IntegerWidth::W64, signed: true, }); + let cents = ColumnKind::Decimal { + precision: Some(10), + scale: Some(2), + }; let zip = ColumnKind::Text { length: TextLength::Varying(5), }; assert_eq!( SummaryRule::for_column(&result_only("n", integer)), - SummaryRule::Numeric + SummaryRule::Numeric { scale: 0 } ); assert_eq!( - SummaryRule::for_column(&result_only("zip", zip)), - SummaryRule::CountOnly + SummaryRule::for_column(&result_only("price", cents)), + SummaryRule::Numeric { scale: 2 } ); - let text = ColumnKind::Text { - length: TextLength::Unbounded, - }; - assert_eq!(SummaryRule::for_column(&result_only("t", text)), SummaryRule::CountOnly); + assert_eq!(SummaryRule::for_column(&result_only("zip", zip)), SummaryRule::CountOnly); assert_eq!( SummaryRule::for_column(&result_only("x", ColumnKind::Other)), SummaryRule::NumericIfParsable @@ -420,22 +414,16 @@ mod tests { } #[test] - fn number_text_parses_exactly() { - let parsed = |text: &str| ExactNumber::parse(text).map(|number| number.plain_text(0)); - - assert_eq!(parsed("1e3").as_deref(), Some("1000")); - assert_eq!(parsed("-2.5E-2").as_deref(), Some("-0.025")); - assert_eq!(parsed(".5").as_deref(), Some("0.5")); - assert_eq!(parsed("7.").as_deref(), Some("7")); - assert_eq!(parsed("1e999999"), None); - assert_eq!( - parsed(&format!("0.{}1", "0".repeat(1_000))), - None, - "past the widest numeric" - ); - assert_eq!(parsed(&"9".repeat(1_000)).map(|text| text.len()), Some(1_000)); - for text in ["", "-", ".", "NaN", "inf", "1,5", "0x10", "1e"] { - assert_eq!(parsed(text), None, "{text:?}"); - } + fn a_cancelled_run_answers_nothing() { + let asked = Cell::new(0); + let values = vec![Value::Int(1); 10]; + + let answer = summarize(&values, NUMERIC, &|| { + asked.set(asked.get() + 1); + asked.get() > 3 + }); + + assert_eq!(answer, None); + assert_eq!(asked.get(), 4, "it stops before the next value"); } } diff --git a/linux/crates/app/src/services/grid/value_order.rs b/linux/crates/app/src/services/grid/value_order.rs index 94c3b88c3d..d466aeb52b 100644 --- a/linux/crates/app/src/services/grid/value_order.rs +++ b/linux/crates/app/src/services/grid/value_order.rs @@ -215,7 +215,7 @@ fn integer_of(value: &Value) -> Option { /// The value as an exact decimal, for comparing numbers of different /// kinds. A float is taken at the decimal its shortest form spells. A /// decimal is borrowed, since a sort compares each one many times. -fn decimal_of(value: &Value) -> Option> { +pub(crate) fn decimal_of(value: &Value) -> Option> { let owned = match value { Value::Decimal(decimal) => return Some(Cow::Borrowed(decimal)), Value::Float32(_) | Value::Float64(_) => { diff --git a/linux/crates/app/src/ui/browse_tab/grid_handlers.rs b/linux/crates/app/src/ui/browse_tab/grid_handlers.rs index c6179d3bf3..1709abcab2 100644 --- a/linux/crates/app/src/ui/browse_tab/grid_handlers.rs +++ b/linux/crates/app/src/ui/browse_tab/grid_handlers.rs @@ -72,6 +72,7 @@ impl BrowseModel { let label = &self.widgets.selection_label; label.set_label(hint.as_deref().unwrap_or_default()); label.set_visible(hint.is_some()); + self.grid.selection_summary().set_visible(self.shown_page().is_some()); let ids: Vec = rows.iter().map(GridRow::id).collect(); let blocked = match delete_availability(&ids, self.read_only) { DeleteAvailability::Unavailable(block) => block.explanation(), diff --git a/linux/crates/app/src/ui/browse_tab/layout.rs b/linux/crates/app/src/ui/browse_tab/layout.rs index 81699c7048..46fb075649 100644 --- a/linux/crates/app/src/ui/browse_tab/layout.rs +++ b/linux/crates/app/src/ui/browse_tab/layout.rs @@ -152,8 +152,7 @@ pub(super) fn grid(tab: &BrowseTab, init: GridInit) -> DataGrid { grid.set_hexpand(true); let imp = tab.imp(); imp.grid_holder.append(&grid); - // After the selected-row count, which it sums. - imp.action_bar.pack_start(grid.selection_summary().widget()); + imp.action_bar.pack_start(&grid.selection_summary()); let _ = imp.grid.set(grid.clone()); connect_grid(tab, &grid); grid diff --git a/linux/crates/app/src/ui/browse_tab/tests/load.rs b/linux/crates/app/src/ui/browse_tab/tests/load.rs index ab04ce809b..e281dfc373 100644 --- a/linux/crates/app/src/ui/browse_tab/tests/load.rs +++ b/linux/crates/app/src/ui/browse_tab/tests/load.rs @@ -250,6 +250,35 @@ fn a_failed_page_turn_hands_the_focus_to_try_again() { assert_eq!(focus().as_ref(), Some(retry.upcast_ref())); } +/// The figures are of rows the error page hides, so the readout goes with +/// the selected-row count. +#[gtk4::test] +fn the_selection_summary_hides_behind_the_error_page() { + let mut script = super::paging(full_page(0), rows(&["unused"])); + script.query_bound = Default::default(); + script + .query_bound + .once(FakeReply::ok(full_page(0))) + .always(FakeReply::err(DriverError::server("denied"))); + let harness = Harness::open(script, []); + wait_until(WAIT, || harness.shown().len() == 100).expect("the first page"); + let _shown = Shown::new(harness.tab()); + let readout = harness + .grid() + .selection_summary() + .downcast::() + .expect("the readout"); + wait_until(WAIT, || harness.grid().focus_cell(0, 0).is_some()).expect("a cell takes the focus"); + harness.select(0..3); + wait_until(WAIT, || readout.reveals_child()).expect("the readout shows the selection"); + assert!(readout.is_visible()); + + harness.tab().send(PagingInput::NextPage); + + wait_until(WAIT, || harness.visible_page().as_deref() == Some("error")).expect("the error page"); + assert!(!readout.is_visible(), "the readout summed rows the error page hides"); +} + fn error_text(harness: &Harness) -> String { harness .tab() diff --git a/linux/crates/app/src/ui/data_grid/columns.rs b/linux/crates/app/src/ui/data_grid/columns.rs index f341c6a432..ee0b7f5043 100644 --- a/linux/crates/app/src/ui/data_grid/columns.rs +++ b/linux/crates/app/src/ui/data_grid/columns.rs @@ -497,6 +497,7 @@ impl DataGrid { if let Some(column) = self.data_view_column(index) { column.set_visible(visible); } + self.refresh_summary(); } /// Every column back where the result has it, at its opening width, diff --git a/linux/crates/app/src/ui/data_grid/imp.rs b/linux/crates/app/src/ui/data_grid/imp.rs index 0dec7923ab..ca64c78d80 100644 --- a/linux/crates/app/src/ui/data_grid/imp.rs +++ b/linux/crates/app/src/ui/data_grid/imp.rs @@ -16,6 +16,7 @@ use super::references::Armed; use super::reversed_list::ReversedList; use super::row_detail::RowDetailPane; use super::selection::Target; +use super::selection_summary::SummaryState; use super::{GridConfig, GridStore}; /// The grid's selection model, made with the grid and given its rows once @@ -64,8 +65,7 @@ pub struct DataGrid { pub columns: RefCell, pub actions: gio::SimpleActionGroup, pub menus: OnceCell, - /// Made when a host first places it in a bar. - pub summary: OnceCell, + pub summary: SummaryState, pub edit: EditController, pub find: FindState, /// What a menu was opened on, until it closes. diff --git a/linux/crates/app/src/ui/data_grid/mod.rs b/linux/crates/app/src/ui/data_grid/mod.rs index a575970ef6..47e2ca9bde 100644 --- a/linux/crates/app/src/ui/data_grid/mod.rs +++ b/linux/crates/app/src/ui/data_grid/mod.rs @@ -180,6 +180,7 @@ impl DataGrid { pub fn set_columns(&self, columns: Arc<[GridColumn]>, sorting: GridSorting, rules: Option) { self.commit_editors(); self.store().clear(); + self.imp().summary.forget_column(); self.edit().set_rules(self, rules); self.rebuild_columns(columns, sorting); self.show_rows_or_empty(); diff --git a/linux/crates/app/src/ui/data_grid/selection_summary.rs b/linux/crates/app/src/ui/data_grid/selection_summary.rs index 46c686a3c4..7246c0e8ba 100644 --- a/linux/crates/app/src/ui/data_grid/selection_summary.rs +++ b/linux/crates/app/src/ui/data_grid/selection_summary.rs @@ -1,29 +1,38 @@ -use std::cell::RefCell; +use std::cell::{Cell, OnceCell, RefCell}; use std::rc::Rc; +use std::sync::Arc; +use std::sync::atomic::{AtomicBool, Ordering}; use gtk4::prelude::*; use gtk4::subclass::prelude::*; use gtk4::{self as gtk, glib}; +use tablepro_core::Value; use super::DataGrid; -use super::tracked_values::RowPendingState; use crate::i18n::{gettext, gettext_f}; -use crate::services::grid::{NumberFigures, SelectionSummary, SummaryRule, SummaryTally}; +use crate::services::grid::{SelectionSummary, SummaryRule, summarize}; use crate::ui::format::{format_decimal, format_thousands}; +use crate::ui::icons; -/// A summary of one row says no more than the cell, so it starts at two. +/// A summary of one row says no more than its cell. const MIN_ROWS: usize = 2; -/// The figures of the focused column over the selected rows, as a -/// spreadsheet's status bar shows them, with every figure and a Copy -/// button for each in its popover. -#[derive(Clone)] -pub struct SummaryReadout { - button: gtk::MenuButton, - label: SummaryLadder, - heading: gtk::Label, - figures: gtk::Grid, - shown: Rc>>, +#[derive(Default)] +pub struct SummaryState { + readout: OnceCell, + /// The last data column a cell had the focus in. It stays while the + /// focus is in the readout's popover or the detail pane. + column: Cell>, + queued: Cell, + /// Counts runs, so an answer to an older one is dropped. + runs: Cell, + running: RefCell>>, +} + +impl SummaryState { + pub(super) fn forget_column(&self) { + self.column.set(None); + } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -32,24 +41,41 @@ struct Shown { summary: SelectionSummary, } -/// One line of the popover: what it is, the value as the locale writes -/// it, and the text Copy puts on the clipboard. -struct Figure { - title: String, - copy_title: String, - shown: String, - copied: String, +/// A menu button in the host's bottom bar whose popover lists every +/// figure with a Copy button. The revealer keeps the bar's height while +/// it is hidden. +struct Readout { + revealer: gtk::Revealer, + button: gtk::MenuButton, + heading: gtk::Label, + separator: gtk::Separator, + figures: Vec, + shown: RefCell>, } -impl SummaryReadout { +impl Readout { fn new(grid: &DataGrid) -> Self { - let label = SummaryLadder::new(); let heading = gtk::Label::builder() .xalign(0.0) .ellipsize(gtk::pango::EllipsizeMode::End) .build(); heading.add_css_class("heading"); - let figures = gtk::Grid::builder().row_spacing(4).column_spacing(12).build(); + let table = gtk::Grid::builder().row_spacing(4).column_spacing(12).build(); + let separator = gtk::Separator::new(gtk::Orientation::Horizontal); + let mut figures = Vec::with_capacity(Figure::ALL.len()); + let mut line = 0; + for figure in Figure::ALL { + if figure == Figure::Sum { + table.attach(&separator, 0, line, 3, 1); + line += 1; + } + let row = FigureRow::new(grid, figure); + table.attach(&row.title, 0, line, 1, 1); + table.attach(&row.value, 1, line, 1, 1); + table.attach(&row.copy, 2, line, 1, 1); + figures.push(row); + line += 1; + } let content = gtk::Box::builder() .orientation(gtk::Orientation::Vertical) .spacing(8) @@ -59,416 +85,299 @@ impl SummaryReadout { .margin_end(6) .build(); content.append(&heading); - content.append(&figures); - let popover = gtk::Popover::builder().child(&content).build(); + content.append(&table); let button = gtk::MenuButton::builder() - .child(&label) - .popover(&popover) - .visible(false) + .popover(>k::Popover::builder().child(&content).build()) + .direction(gtk::ArrowType::Up) + .can_shrink(true) .build(); button.add_css_class("flat"); - button.update_property(&[gtk::accessible::Property::Description(&gettext( - "Shows every figure with a Copy button", - ))]); - let readout = Self { + button.add_css_class("caption"); + button.add_css_class("numeric"); + let revealer = gtk::Revealer::builder() + .transition_type(gtk::RevealerTransitionType::SlideRight) + .child(&button) + .build(); + Self { + revealer, button, - label, heading, + separator, figures, - shown: Rc::default(), - }; - let weak = grid.downgrade(); - let filled = readout.clone(); - popover.connect_show(move |_| { - if let Some(grid) = weak.upgrade() { - filled.fill_figures(&grid); - } - }); - readout - } - - pub(crate) fn widget(&self) -> >k::Widget { - self.button.upcast_ref() + shown: RefCell::default(), + } } - fn show(&self, grid: &DataGrid, shown: Option) { + fn show(&self, shown: Option) { if *self.shown.borrow() == shown { return; } match &shown { Some(shown) => { - let rungs = label_rungs(&shown.summary); - self.button.set_tooltip_text(Some(&rungs[0])); - self.button - .update_property(&[gtk::accessible::Property::Label(&rungs[0])]); - self.label.set_rungs(&rungs); + let sentence = sentence(&shown.summary); + self.button.set_label(&sentence); + // The label ellipsizes in a narrow bar; the tooltip keeps every digit. + self.button.set_tooltip_text(Some(&sentence)); + self.heading.set_label(&shown.column); + for row in &self.figures { + row.show(row.figure.of(&shown.summary)); + } + self.separator.set_visible(shown.summary.numbers.is_some()); } None => self.button.popdown(), } - self.button.set_visible(shown.is_some()); + self.revealer.set_reveal_child(shown.is_some()); self.shown.replace(shown); - if self.button.popover().is_some_and(|popover| popover.is_visible()) { - self.fill_figures(grid); - } } +} - fn fill_figures(&self, grid: &DataGrid) { - while let Some(child) = self.figures.first_child() { - self.figures.remove(&child); - } - let Some(shown) = self.shown.borrow().clone() else { - return; - }; - self.heading.set_label(&shown.column); - let (counts, numbers) = figures(&shown.summary); - let mut line = 0; - for figure in counts { - self.attach_figure(grid, &figure, line); - line += 1; +fn sentence(summary: &SelectionSummary) -> String { + let count = format_thousands(summary.value_count); + match &summary.numbers { + Some(numbers) => gettext_f( + "Sum {sum}, Average {average}, Count {count}", + &[ + ("sum", &format_decimal(&numbers.sum)), + ("average", &format_decimal(&numbers.average)), + ("count", &count), + ], + ), + None => gettext_f("Count {count}", &[("count", &count)]), + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum Figure { + Count, + Numbers, + Empty, + NotANumber, + Sum, + Average, + Minimum, + Maximum, +} + +impl Figure { + const ALL: [Self; 8] = [ + Self::Count, + Self::Numbers, + Self::Empty, + Self::NotANumber, + Self::Sum, + Self::Average, + Self::Minimum, + Self::Maximum, + ]; + + fn title(self) -> String { + match self { + Self::Count => gettext("Count"), + // Translators: how many of the summed cells hold a number. + Self::Numbers => gettext("Numbers"), + Self::Empty => gettext("Empty"), + Self::NotANumber => gettext("Not a Number"), + Self::Sum => gettext("Sum"), + Self::Average => gettext("Average"), + Self::Minimum => gettext("Minimum"), + Self::Maximum => gettext("Maximum"), } - if !numbers.is_empty() { - self.figures - .attach(>k::Separator::new(gtk::Orientation::Horizontal), 0, line, 3, 1); - line += 1; + } + + fn copy_title(self) -> String { + match self { + Self::Count => gettext("Copy Count"), + Self::Numbers => gettext("Copy Number Count"), + Self::Empty => gettext("Copy Empty Count"), + Self::NotANumber => gettext("Copy Not a Number Count"), + Self::Sum => gettext("Copy Sum"), + Self::Average => gettext("Copy Average"), + Self::Minimum => gettext("Copy Minimum"), + Self::Maximum => gettext("Copy Maximum"), } - for figure in numbers { - self.attach_figure(grid, &figure, line); - line += 1; + } + + /// The figure as the locale writes it and as Copy puts it on the + /// clipboard, when the summary has it. + fn of(self, summary: &SelectionSummary) -> Option<(String, String)> { + let count = |n: u64| (format_thousands(n), n.to_string()); + let number = |plain: &String| (format_decimal(plain), plain.clone()); + let numbers = summary.numbers.as_ref(); + match self { + Self::Count => Some(count(summary.value_count)), + Self::Numbers => Some(count(numbers.map_or(0, |numbers| numbers.count))), + Self::Empty => Some(count(summary.empty_count)), + Self::NotANumber => (summary.not_a_number_count > 0).then(|| count(summary.not_a_number_count)), + Self::Sum => numbers.map(|numbers| number(&numbers.sum)), + Self::Average => numbers.map(|numbers| number(&numbers.average)), + Self::Minimum => numbers.map(|numbers| number(&numbers.minimum)), + Self::Maximum => numbers.map(|numbers| number(&numbers.maximum)), } } +} + +struct FigureRow { + figure: Figure, + title: gtk::Label, + value: gtk::Label, + copy: gtk::Button, + copied: Rc>, +} - fn attach_figure(&self, grid: &DataGrid, figure: &Figure, line: i32) { - let title = gtk::Label::builder().label(&figure.title).xalign(0.0).build(); +impl FigureRow { + fn new(grid: &DataGrid, figure: Figure) -> Self { + let title = gtk::Label::builder().label(figure.title()).xalign(0.0).build(); title.add_css_class("dimmed"); + // A sum can run to thousands of digits; the popover stays narrow. let value = gtk::Label::builder() - .label(&figure.shown) .xalign(1.0) .hexpand(true) - .selectable(true) + .wrap(true) + .wrap_mode(gtk::pango::WrapMode::Char) + .max_width_chars(32) .build(); value.add_css_class("numeric"); value.update_relation(&[gtk::accessible::Relation::LabelledBy(&[title.upcast_ref()])]); + let copy_title = figure.copy_title(); let copy = gtk::Button::builder() - .icon_name("edit-copy-symbolic") - .tooltip_text(&figure.copy_title) + .icon_name(icons::EDIT_COPY) + .tooltip_text(©_title) + .valign(gtk::Align::Center) .build(); copy.add_css_class("flat"); - copy.update_property(&[gtk::accessible::Property::Label(&figure.copy_title)]); - let text = figure.copied.clone(); - let weak = grid.downgrade(); + copy.update_property(&[gtk::accessible::Property::Label(©_title)]); + let copied: Rc> = Rc::default(); + let text = copied.clone(); + let grid = grid.downgrade(); copy.connect_clicked(move |_| { - if let Some(grid) = weak.upgrade() { - crate::ui::clipboard::copy_text(&grid, text.clone(), |message| grid.toast(message)); + if let Some(grid) = grid.upgrade() { + crate::ui::clipboard::copy_text(&grid, text.borrow().clone(), |message| grid.toast(message)); } }); - self.figures.attach(&title, 0, line, 1, 1); - self.figures.attach(&value, 1, line, 1, 1); - self.figures.attach(©, 2, line, 1, 1); - } -} - -/// The bar's text from the widest to the narrowest. Digits are never cut: -/// a narrower bar drops figures instead, down to a sign that keeps the -/// popover in reach at every width. -fn label_rungs(summary: &SelectionSummary) -> [String; 3] { - let count = format_thousands(summary.value_count); - // Translators: the summation sign, the bar's sum when it has room for nothing else. - let sign = gettext("Σ"); - match &summary.numbers { - Some(numbers) => { - let sum = format_decimal(&numbers.sum.plain_text(numbers.scale)); - [ - gettext_f( - "Sum {sum}, Average {average}, Count {count}", - &[ - ("sum", &sum), - ("average", &format_decimal(&numbers.average.plain_text(0))), - ("count", &count), - ], - ), - gettext_f("Sum {sum}", &[("sum", &sum)]), - sign, - ] + Self { + figure, + title, + value, + copy, + copied, } - None => { - let count = gettext_f("Count {count}", &[("count", &count)]); - [count.clone(), count, sign] - } - } -} - -mod ladder { - use std::cell::RefCell; - - use gtk4::prelude::*; - use gtk4::subclass::prelude::*; - use gtk4::{self as gtk, glib}; - - /// Shows the first of its labels that fits its width, the last one - /// when none does, so a long one never shows cut short. - #[derive(Default)] - pub struct SummaryLadder { - pub rungs: RefCell>, - } - - #[glib::object_subclass] - impl ObjectSubclass for SummaryLadder { - const NAME: &'static str = "TableProSummaryLadder"; - type Type = super::SummaryLadder; - type ParentType = gtk::Widget; } - impl ObjectImpl for SummaryLadder { - fn dispose(&self) { - for rung in self.rungs.take() { - rung.unparent(); - } - } - } - - impl WidgetImpl for SummaryLadder { - fn measure(&self, orientation: gtk::Orientation, _for_size: i32) -> (i32, i32, i32, i32) { - let rungs = self.rungs.borrow(); - let natural = |rung: >k::Label| rung.measure(orientation, -1).1; - match orientation { - gtk::Orientation::Horizontal => { - let widest = rungs.first().map_or(0, natural); - let narrowest = rungs.last().map_or(0, natural); - (narrowest, widest.max(narrowest), -1, -1) - } - _ => { - let tallest = rungs.iter().map(natural).max().unwrap_or(0); - (tallest, tallest, -1, -1) - } - } - } - - fn size_allocate(&self, width: i32, height: i32, baseline: i32) { - let rungs = self.rungs.borrow(); - let last = rungs.len().saturating_sub(1); - let chosen = rungs - .iter() - .position(|rung| rung.measure(gtk::Orientation::Horizontal, -1).1 <= width) - .unwrap_or(last); - for (index, rung) in rungs.iter().enumerate() { - rung.set_child_visible(index == chosen); - if index == chosen { - rung.allocate(width, height, baseline, None); - } - } + fn show(&self, figure: Option<(String, String)>) { + let shown = figure.is_some(); + if let Some((text, copied)) = figure { + self.value.set_label(&text); + self.copied.replace(copied); } + self.title.set_visible(shown); + self.value.set_visible(shown); + self.copy.set_visible(shown); } } -glib::wrapper! { - pub struct SummaryLadder(ObjectSubclass) - @extends gtk::Widget, - @implements gtk::Accessible, gtk::Buildable, gtk::ConstraintTarget; -} - -impl SummaryLadder { - fn new() -> Self { - glib::Object::new() +impl DataGrid { + /// The readout for a host to place in its bottom bar. A grid sums + /// nothing until one is placed. + pub(crate) fn selection_summary(&self) -> gtk::Widget { + let state = &self.imp().summary; + let placed = state.readout.get().is_some(); + let widget = state + .readout + .get_or_init(|| Readout::new(self)) + .revealer + .clone() + .upcast(); + if !placed { + self.refresh_summary(); + } + widget } - fn set_rungs(&self, texts: &[String]) { - let mut rungs = self.imp().rungs.borrow_mut(); - while rungs.len() < texts.len() { - // The button carries the whole text for a screen reader. - let rung = gtk::Label::builder() - .accessible_role(gtk::AccessibleRole::Presentation) - .build(); - rung.add_css_class("caption"); - rung.add_css_class("numeric"); - rung.set_parent(self); - rungs.push(rung); + /// Sum again once the main loop is idle, so a key press that moves + /// both the focus and the selection sums once. + pub(super) fn refresh_summary(&self) { + let state = &self.imp().summary; + if let Some(cell) = self.focused_cell() { + state.column.set(Some(cell.column())); } - for (rung, text) in rungs.iter().zip(texts) { - rung.set_label(text); + if state.readout.get().is_none() || state.queued.replace(true) { + return; } - drop(rungs); - self.queue_resize(); - } - - /// The text it shows when nothing is short of room. - #[cfg(test)] - fn widest(&self) -> String { - self.imp() - .rungs - .borrow() - .first() - .map(|rung| rung.label().to_string()) - .unwrap_or_default() - } -} - -fn count_figure(title: String, copy_title: String, n: u64) -> Figure { - Figure { - title, - copy_title, - shown: format_thousands(n), - copied: n.to_string(), - } -} - -fn number_figure(title: String, copy_title: String, plain: String) -> Figure { - Figure { - title, - copy_title, - shown: format_decimal(&plain), - copied: plain, - } -} - -/// The counts, then the figures of the numbers among the values. -fn figures(summary: &SelectionSummary) -> (Vec
, Vec
) { - let mut counts = vec![ - count_figure(gettext("Count"), gettext("Copy Count"), summary.value_count), - count_figure( - // Translators: how many of the summed cells hold a number. - gettext("Numbers"), - gettext("Copy Number Count"), - summary.numbers.as_ref().map_or(0, |numbers| numbers.count), - ), - count_figure(gettext("Empty"), gettext("Copy Empty Count"), summary.empty_count), - ]; - if summary.not_a_number_count > 0 { - counts.push(count_figure( - gettext("Not a Number"), - gettext("Copy Not a Number Count"), - summary.not_a_number_count, - )); - } - let numbers = summary.numbers.as_ref().map_or_else(Vec::new, number_figures); - (counts, numbers) -} - -fn number_figures(numbers: &NumberFigures) -> Vec
{ - vec![ - number_figure( - gettext("Sum"), - gettext("Copy Sum"), - numbers.sum.plain_text(numbers.scale), - ), - number_figure( - gettext("Average"), - gettext("Copy Average"), - numbers.average.plain_text(0), - ), - number_figure( - gettext("Minimum"), - gettext("Copy Minimum"), - numbers.minimum.plain_text(numbers.scale), - ), - number_figure( - gettext("Maximum"), - gettext("Copy Maximum"), - numbers.maximum.plain_text(numbers.scale), - ), - ] -} - -impl DataGrid { - /// The readout for a host to place in its status bar. It shows only - /// once placed, so a grid no bar holds sums nothing. - pub(crate) fn selection_summary(&self) -> SummaryReadout { - let readout = self.imp().summary.get_or_init(|| SummaryReadout::new(self)).clone(); - self.refresh_summary(); - readout + let grid = self.downgrade(); + glib::idle_add_local_once(move || { + if let Some(grid) = grid.upgrade() { + grid.imp().summary.queued.set(false); + grid.start_summary(); + } + }); } - pub(super) fn refresh_summary(&self) { - if let Some(readout) = self.imp().summary.get() { - readout.show(self, self.summarize()); + /// Sum the values off the GTK thread: a page holds up to 100,000 rows. + /// A newer run stops this one. + fn start_summary(&self) { + let state = &self.imp().summary; + let Some(readout) = state.readout.get() else { + return; + }; + if let Some(running) = state.running.take() { + running.store(true, Ordering::Relaxed); } + let run = state.runs.get().wrapping_add(1); + state.runs.set(run); + let Some((column, rule, values)) = self.summary_input() else { + readout.show(None); + return; + }; + let Some(tasks) = self.config().map(|config| config.tasks.clone()) else { + return; + }; + let cancelled = Arc::new(AtomicBool::new(false)); + state.running.replace(Some(cancelled.clone())); + let grid = self.downgrade(); + glib::spawn_future_local(async move { + let answer = tasks + .spawn_blocking_task(move || summarize(&values, rule, &|| cancelled.load(Ordering::Relaxed))) + .await; + let Some(grid) = grid.upgrade() else { + return; + }; + let state = &grid.imp().summary; + if state.runs.get() != run { + return; + } + state.running.take(); + match answer { + Ok(Some(summary)) => { + if let Some(readout) = state.readout.get() { + readout.show(Some(Shown { column, summary })); + } + } + Ok(None) => {} + Err(failure) => tracing::warn!(%failure, "the selection could not be summed"), + } + }); } - /// The focused column over the selected rows. Rows marked for - /// deletion are left out, and pending edits count as the cell shows - /// them. - fn summarize(&self) -> Option { + /// The summed column's name, its rule and its values in the selected + /// rows, as the cells show them. Rows marked for deletion are left out, + /// also once a save failed on them. + fn summary_input(&self) -> Option<(String, SummaryRule, Vec)> { let rows = self.selected_rows(); if rows.len() < MIN_ROWS { return None; } - let column = self.focus_target().resolve()?.cell?.1; + let index = self.imp().summary.column.get()?; + if !self.shown_columns().contains(&index) { + return None; + } let columns = self.grid_columns(); - let grid_column = columns.iter().find(|candidate| candidate.index == column)?; - let rule = SummaryRule::for_column(grid_column); + let column = columns.iter().find(|column| column.index == index)?; let tracked = self.tracked(); - let mut tally = SummaryTally::default(); - for row in rows { - if tracked.row_state(&row) == RowPendingState::MarkedForDeletion { - continue; - } - tracked.with_value(&row, column, |value, default| tally.add(value, default, rule)); - } - Some(Shown { - column: grid_column.name.clone(), - summary: tally.summary(), - }) - } -} - -#[cfg(test)] -mod tests { - use gtk4::prelude::*; - use tablepro_core::Value; - - use crate::ui::data_grid::test_grid::{Grid, named}; - - fn numbers_and_names() -> Vec> { - let text = |name: &str| Value::Text(name.to_owned()); - vec![ - vec![Value::Int(10), text("a")], - vec![Value::Int(32), text("b")], - vec![Value::Null, text("c")], - ] - } - - #[gtk4::test] - fn summary_sums_the_focused_column_over_the_selected_rows() { - let grid = Grid::open( - vec![named("amount", "integer", false), named("name", "text", true)], - numbers_and_names(), - ); - let readout = grid.grid.selection_summary(); - assert!(!readout.widget().is_visible(), "nothing is selected yet"); - - grid.focus(0, 0); - grid.grid.selection().select_all(); - - assert!(readout.widget().is_visible()); - assert_eq!(readout.label.widest(), "Sum 42, Average 21, Count 2"); - } - - #[gtk4::test] - fn summary_only_counts_a_text_column() { - let grid = Grid::open( - vec![named("amount", "integer", false), named("name", "text", true)], - numbers_and_names(), - ); - let readout = grid.grid.selection_summary(); - - grid.focus(0, 1); - grid.grid.selection().select_all(); - - assert_eq!(readout.label.widest(), "Count 3"); - } - - #[gtk4::test] - fn summary_hides_for_a_single_row() { - let grid = Grid::open( - vec![named("amount", "integer", false), named("name", "text", true)], - numbers_and_names(), - ); - let readout = grid.grid.selection_summary(); - grid.focus(0, 0); - grid.grid.selection().select_all(); - assert!(readout.widget().is_visible()); - - grid.grid.selection().select_item(1, true); - - assert!(!readout.widget().is_visible()); + let values = rows + .iter() + .filter(|row| !tracked.mark(row, index).row_deleted) + .map(|row| tracked.with_value(row, index, |value, default| if default { Value::Null } else { value.clone() })) + .collect(); + Some((column.name.clone(), SummaryRule::for_column(column), values)) } } diff --git a/linux/crates/app/src/ui/data_grid/tests/mod.rs b/linux/crates/app/src/ui/data_grid/tests/mod.rs index d0d46eec1f..c218bade72 100644 --- a/linux/crates/app/src/ui/data_grid/tests/mod.rs +++ b/linux/crates/app/src/ui/data_grid/tests/mod.rs @@ -6,6 +6,7 @@ mod layout; mod menus; mod selection; mod sorting; +mod summary; mod values; mod view; diff --git a/linux/crates/app/src/ui/data_grid/tests/summary.rs b/linux/crates/app/src/ui/data_grid/tests/summary.rs new file mode 100644 index 0000000000..b1e9164053 --- /dev/null +++ b/linux/crates/app/src/ui/data_grid/tests/summary.rs @@ -0,0 +1,255 @@ +use std::cell::RefCell; +use std::rc::Rc; + +use gtk4 as gtk; +use gtk4::prelude::*; +use tablepro_core::Value; +use tablepro_core::value::SqlDecimal; + +use super::super::GridRequest; +use super::super::test_grid::{Grid, WAIT, key, named}; +use crate::services::tracker_edit::TrackerEdit; +use crate::test_support::{descendants, drain_main_context, finish_on_main, wait_until}; + +fn text(value: &str) -> Value { + Value::Text(value.to_owned()) +} + +/// Amounts 10, 32 and NULL, keyed 1 to 3. +fn amounts() -> Grid { + Grid::open( + vec![ + named("id", "integer", true), + named("amount", "integer", false), + named("name", "text", false), + ], + vec![ + vec![Value::Int(1), Value::Int(10), text("a")], + vec![Value::Int(2), Value::Int(32), text("b")], + vec![Value::Int(3), Value::Null, text("c")], + ], + ) +} + +fn readout(grid: &Grid) -> (gtk::Revealer, gtk::MenuButton) { + let revealer = grid + .grid + .selection_summary() + .downcast::() + .expect("the readout is a revealer"); + let button = revealer + .child() + .and_downcast::() + .expect("the readout's menu button"); + (revealer, button) +} + +fn wait_for_label(button: >k::MenuButton, expected: &str) { + wait_until(WAIT, || button.label().as_deref() == Some(expected)) + .unwrap_or_else(|_| panic!("the readout said {:?}, not {expected:?}", button.label())); +} + +fn wait_hidden(revealer: >k::Revealer) { + wait_until(WAIT, || !revealer.reveals_child()).expect("the readout hides"); +} + +#[gtk4::test] +fn sums_the_focused_column_over_the_selected_rows() { + let grid = amounts(); + let (revealer, button) = readout(&grid); + drain_main_context(); + assert!(!revealer.reveals_child(), "nothing is selected yet"); + + grid.focus(0, 1); + grid.grid.selection().select_all(); + + wait_for_label(&button, "Sum 42, Average 21, Count 2"); + assert!(revealer.reveals_child()); + assert_eq!(button.tooltip_text().as_deref(), Some("Sum 42, Average 21, Count 2")); +} + +#[gtk4::test] +fn a_text_column_only_counts() { + let grid = amounts(); + let (_, button) = readout(&grid); + + grid.focus(0, 2); + grid.grid.selection().select_all(); + + wait_for_label(&button, "Count 3"); +} + +#[gtk4::test] +fn a_single_row_shows_no_summary() { + let grid = amounts(); + let (revealer, button) = readout(&grid); + grid.focus(0, 1); + grid.grid.selection().select_all(); + wait_for_label(&button, "Sum 42, Average 21, Count 2"); + + grid.grid.selection().select_item(1, true); + + wait_hidden(&revealer); +} + +#[gtk4::test] +fn an_edit_counts_as_the_cell_shows_it_while_the_focus_is_elsewhere() { + let grid = amounts(); + let (revealer, button) = readout(&grid); + grid.focus(0, 1); + grid.grid.selection().select_all(); + wait_for_label(&button, "Sum 42, Average 21, Count 2"); + + gtk::prelude::GtkWindowExt::set_focus(&grid.window, None::<>k::Widget>); + grid.grid.apply_delta(&grid.tracker.apply(TrackerEdit::Cell { + row_key: key(1), + col: 1, + original: Value::Int(10), + value: Value::Int(20), + })); + + wait_for_label(&button, "Sum 52, Average 26, Count 2"); + assert!(revealer.reveals_child(), "the focus leaving the cells hid the readout"); +} + +#[gtk4::test] +fn rows_marked_for_deletion_are_left_out_also_after_a_failed_save() { + let grid = amounts(); + let (_, button) = readout(&grid); + grid.focus(0, 1); + grid.grid.selection().select_all(); + wait_for_label(&button, "Sum 42, Average 21, Count 2"); + + grid.grid.apply_delta(&grid.tracker.apply(TrackerEdit::DeleteRows { + persisted: vec![(key(2), vec![Value::Int(2), Value::Int(32), text("b")].into())], + drafts: Vec::new(), + })); + wait_for_label(&button, "Sum 10, Average 10, Count 1"); + + grid.grid.apply_delta(&grid.tracker.mark_error_row(key(2))); + grid.grid.apply_delta(&grid.tracker.apply(TrackerEdit::Cell { + row_key: key(1), + col: 1, + original: Value::Int(10), + value: Value::Int(11), + })); + wait_for_label(&button, "Sum 11, Average 11, Count 1"); +} + +#[gtk4::test] +fn a_hidden_column_is_not_summed() { + let grid = amounts(); + let (revealer, button) = readout(&grid); + grid.focus(0, 1); + grid.grid.selection().select_all(); + wait_for_label(&button, "Sum 42, Average 21, Count 2"); + + grid.grid.set_column_visible(1, false); + + // GTK may move the focus to a cell still shown, whose column is summed then. + wait_until(WAIT, || { + !revealer.reveals_child() || button.label().as_deref() != Some("Sum 42, Average 21, Count 2") + }) + .expect("the hidden column is still summed"); +} + +#[gtk4::test] +fn the_readout_keeps_its_height_while_hidden() { + let grid = amounts(); + let (revealer, button) = readout(&grid); + let footer = gtk::Box::new(gtk::Orientation::Horizontal, 6); + footer.append(>k::Label::new(Some("3 rows"))); + footer.append(&revealer); + let hidden = footer.measure(gtk::Orientation::Vertical, -1).1; + + grid.focus(0, 1); + grid.grid.selection().select_all(); + wait_for_label(&button, "Sum 42, Average 21, Count 2"); + + assert_eq!(footer.measure(gtk::Orientation::Vertical, -1).1, hidden); +} + +#[gtk4::test] +fn the_popover_lists_each_figure_and_copies_it_without_grouping() { + let decimal = |text: &str| Value::Decimal(text.parse::().expect("a decimal")); + let grid = Grid::open( + vec![named("id", "integer", true), named("price", "numeric(10,2)", false)], + vec![ + vec![Value::Int(1), decimal("1234.5")], + vec![Value::Int(2), decimal("0.25")], + vec![Value::Int(3), decimal("NaN")], + ], + ); + let toasts: Rc>> = Rc::default(); + let log = toasts.clone(); + grid.grid.connect_request(move |_, request| { + if let GridRequest::Toast(text) = request { + log.borrow_mut().push(text); + } + }); + let (_, button) = readout(&grid); + grid.focus(0, 1); + grid.grid.selection().select_all(); + // CI runs in the C locale, which groups as English does. + wait_for_label(&button, "Sum 1,234.75, Average 617.375, Count 3"); + + let popover = button.popover().expect("the readout's popover"); + let labels: Vec = descendants(&popover) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + .filter(|label| label.is_visible()) + .map(|label| label.label().to_string()) + .collect(); + assert_eq!( + labels, + [ + "price", + "Count", + "3", + "Numbers", + "2", + "Empty", + "0", + "Not a Number", + "1", + "Sum", + "1,234.75", + "Average", + "617.375", + "Minimum", + "0.25", + "Maximum", + "1,234.50", + ] + ); + + let copy_maximum = descendants(&popover) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + .find(|copy| copy.tooltip_text().as_deref() == Some("Copy Maximum")) + .expect("Copy Maximum"); + copy_maximum.emit_clicked(); + wait_until(WAIT, || !toasts.borrow().is_empty()).expect("the copy lands"); + let clipboard = grid.grid.clipboard(); + let copied = finish_on_main(WAIT, async move { clipboard.read_text_future().await }) + .expect("the clipboard to answer") + .expect("text on the clipboard"); + assert_eq!(copied.as_deref(), Some("1234.50")); +} + +#[gtk4::test] +fn the_readout_goes_with_its_grid() { + let grid = amounts(); + let (revealer, button) = readout(&grid); + grid.focus(0, 1); + grid.grid.selection().select_all(); + wait_for_label(&button, "Sum 42, Average 21, Count 2"); + let (revealer_ref, button_ref) = (revealer.downgrade(), button.downgrade()); + drop((revealer, button)); + + drop(grid); + drain_main_context(); + + assert!(revealer_ref.upgrade().is_none(), "the readout outlived its grid"); + assert!(button_ref.upgrade().is_none(), "the menu button outlived its grid"); +} diff --git a/linux/crates/app/src/ui/key_tab/mod.rs b/linux/crates/app/src/ui/key_tab/mod.rs index c30341e809..441d877497 100644 --- a/linux/crates/app/src/ui/key_tab/mod.rs +++ b/linux/crates/app/src/ui/key_tab/mod.rs @@ -344,6 +344,7 @@ fn content_page(grid: &DataGrid) -> (gtk::Box, ContentParts) { .build(); more.add_css_class("flat"); footer_bar.append(&footer); + footer_bar.append(&grid.selection_summary()); footer_bar.append(&more); page.append(&header); page.append(>k::Separator::new(gtk::Orientation::Horizontal)); diff --git a/linux/crates/app/src/ui/key_tab/tests.rs b/linux/crates/app/src/ui/key_tab/tests.rs index c6afe185a1..4db0e79740 100644 --- a/linux/crates/app/src/ui/key_tab/tests.rs +++ b/linux/crates/app/src/ui/key_tab/tests.rs @@ -198,6 +198,41 @@ fn a_refresh_keeps_the_grid_and_its_sort() { assert_eq!(shown_elements(&tab), ["element2", "element1", "element0"]); } +#[gtk4::test] +fn the_footer_sums_the_selected_entries() { + let mut script = ConnectionScript::default(); + script.read_key.always(FakeReply::ok(Some(list_page(0, 3, None)))); + let (tab, _fake, _runtime) = opened(script); + let window = crate::test_support::OwnedWindow::presented(gtk::Window::builder().child(tab.widget()).build()); + wait_until(WAIT, || tab.rows() == 3 && tab.grid().width() > 0).expect("the key, laid out"); + let readout = tab + .grid() + .selection_summary() + .downcast::() + .expect("the readout"); + assert!( + readout.is_ancestor(tab.widget()), + "the readout is not in the key's footer" + ); + + wait_until(WAIT, || tab.grid().focus_cell(0, 0).is_some()).expect("a cell takes the focus"); + tab.grid().selection().select_all(); + + let button = readout.child().and_downcast::().expect("the menu button"); + wait_until(WAIT, || button.label().as_deref() == Some("Sum 3, Average 1, Count 3")).expect("the index summed"); + + button.popup(); + + // The figures are in place before the popover opens and takes the focus. + let first_copy = || { + gtk::prelude::GtkWindowExt::focus(&*window) + .and_downcast::() + .and_then(|focused| focused.tooltip_text()) + .is_some_and(|tooltip| tooltip == "Copy Count") + }; + wait_until(WAIT, first_copy).expect("the popover's first Copy button takes the focus"); +} + /// The header's Row Details toggle reaches the tab's grid. #[gtk4::test] fn row_details_reaches_the_key_s_grid() { diff --git a/linux/crates/app/src/ui/result_pane/mod.rs b/linux/crates/app/src/ui/result_pane/mod.rs index 85b4fd2409..d0046c16a0 100644 --- a/linux/crates/app/src/ui/result_pane/mod.rs +++ b/linux/crates/app/src/ui/result_pane/mod.rs @@ -71,7 +71,7 @@ pub(crate) fn result_set_page( page.append(&grid); let footer = gtk::Box::builder().spacing(6).build(); footer.append(&row_count_footer(view.row_count() as u64)); - footer.append(grid.selection_summary().widget()); + footer.append(&grid.selection_summary()); page.append(&footer); (page.upcast(), grid) } @@ -90,7 +90,7 @@ fn row_count_footer(shown: u64) -> gtk::Label { .margin_top(4) .margin_bottom(4) .build(); - footer.add_css_class("dim-label"); + footer.add_css_class("dimmed"); footer.add_css_class("caption"); footer } diff --git a/linux/crates/core/src/value/decimal.rs b/linux/crates/core/src/value/decimal.rs index afb4306746..e00e8bd20e 100644 --- a/linux/crates/core/src/value/decimal.rs +++ b/linux/crates/core/src/value/decimal.rs @@ -132,6 +132,18 @@ impl SqlDecimal { Some((if *negative { -magnitude } else { magnitude }, *scale)) } + /// What `from_parts` takes: the sign, the digits and the scale. + pub fn parts(&self) -> Option<(bool, &str, u32)> { + match &self.0 { + DecimalRepr::Finite { + negative, + coefficient, + scale, + } => Some((*negative, &**coefficient, *scale)), + _ => None, + } + } + pub fn fits(&self, precision: u32, scale: u32) -> bool { let DecimalRepr::Finite { coefficient, @@ -442,6 +454,19 @@ mod tests { assert_eq!(SqlDecimal::from_scaled_i128(-125, 2).to_string(), "-1.25"); } + #[test] + fn parts_round_trip_through_from_parts() { + let wide = format!("-{}.5", "9".repeat(60)); + let digits = format!("{}5", "9".repeat(60)); + assert_eq!(parse(&wide).parts(), Some((true, digits.as_str(), 1))); + assert_eq!(parse("0.10").parts(), Some((false, "10", 2))); + let cents = parse("-12.30"); + let (negative, coefficient, scale) = cents.parts().unwrap(); + assert_eq!(SqlDecimal::from_parts(negative, coefficient, scale), Ok(cents.clone())); + assert_eq!(SqlDecimal::nan().parts(), None); + assert_eq!(SqlDecimal::infinity(true).parts(), None); + } + #[test] fn cmp_numeric_orders_across_scales_and_specials() { let ordered = [ diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index cf371f6f92..13a299aff0 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -282,6 +282,7 @@ crates/app/src/ui/data_grid/tests/menus.rs crates/app/src/ui/data_grid/tests/mod.rs crates/app/src/ui/data_grid/tests/selection.rs crates/app/src/ui/data_grid/tests/sorting.rs +crates/app/src/ui/data_grid/tests/summary.rs crates/app/src/ui/data_grid/tests/values.rs crates/app/src/ui/data_grid/tests/view.rs crates/app/src/ui/data_grid/tracked_values.rs From a073a956eb7fd56e02c917809bd82d12faa24082 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 29 Oct 2026 23:31:43 +0700 Subject: [PATCH 04/11] fix(linux-grid): share row snapshots with the summing thread and hold Copy while a run is out --- .../src/services/grid/selection_summary.rs | 6 +- .../app/src/ui/data_grid/selection_summary.rs | 69 +++++++++++++------ .../app/src/ui/data_grid/tests/summary.rs | 3 +- 3 files changed, 56 insertions(+), 22 deletions(-) diff --git a/linux/crates/app/src/services/grid/selection_summary.rs b/linux/crates/app/src/services/grid/selection_summary.rs index 699ed1e1a3..d64f8c4557 100644 --- a/linux/crates/app/src/services/grid/selection_summary.rs +++ b/linux/crates/app/src/services/grid/selection_summary.rs @@ -59,7 +59,11 @@ pub struct NumberFigures { /// `values` as their cells show them, with a cell left to its default as /// NULL. `None` once `cancelled` says so. It asks before every value: one /// of 100,000 digits takes milliseconds alone. -pub fn summarize(values: &[Value], rule: SummaryRule, cancelled: &dyn Fn() -> bool) -> Option { +pub fn summarize<'a>( + values: impl IntoIterator, + rule: SummaryRule, + cancelled: &dyn Fn() -> bool, +) -> Option { let mut tally = Tally::default(); for value in values { if cancelled() { diff --git a/linux/crates/app/src/ui/data_grid/selection_summary.rs b/linux/crates/app/src/ui/data_grid/selection_summary.rs index 7246c0e8ba..b6075af35b 100644 --- a/linux/crates/app/src/ui/data_grid/selection_summary.rs +++ b/linux/crates/app/src/ui/data_grid/selection_summary.rs @@ -6,11 +6,10 @@ use std::sync::atomic::{AtomicBool, Ordering}; use gtk4::prelude::*; use gtk4::subclass::prelude::*; use gtk4::{self as gtk, glib}; -use tablepro_core::Value; use super::DataGrid; use crate::i18n::{gettext, gettext_f}; -use crate::services::grid::{SelectionSummary, SummaryRule, summarize}; +use crate::services::grid::{RowSnapshot, SelectionSummary, SummaryRule, summarize}; use crate::ui::format::{format_decimal, format_thousands}; use crate::ui::icons; @@ -108,7 +107,16 @@ impl Readout { } } + /// While a run is out, Copy would take figures of rows no longer + /// selected. + fn set_current(&self, current: bool) { + for row in &self.figures { + row.copy.set_sensitive(current); + } + } + fn show(&self, shown: Option) { + self.set_current(true); if *self.shown.borrow() == shown { return; } @@ -324,19 +332,26 @@ impl DataGrid { } let run = state.runs.get().wrapping_add(1); state.runs.set(run); - let Some((column, rule, values)) = self.summary_input() else { + let (Some(tasks), Some(input)) = (self.config().map(|config| config.tasks.clone()), self.summary_input()) else { readout.show(None); return; }; - let Some(tasks) = self.config().map(|config| config.tasks.clone()) else { - return; - }; + let SummaryInput { + column, + index, + rule, + rows, + } = input; let cancelled = Arc::new(AtomicBool::new(false)); state.running.replace(Some(cancelled.clone())); + readout.set_current(false); let grid = self.downgrade(); glib::spawn_future_local(async move { let answer = tasks - .spawn_blocking_task(move || summarize(&values, rule, &|| cancelled.load(Ordering::Relaxed))) + .spawn_blocking_task(move || { + let values = rows.iter().map(|row| row.value(index)); + summarize(values, rule, &|| cancelled.load(Ordering::Relaxed)) + }) .await; let Some(grid) = grid.upgrade() else { return; @@ -346,22 +361,24 @@ impl DataGrid { return; } state.running.take(); + let Some(readout) = state.readout.get() else { + return; + }; match answer { - Ok(Some(summary)) => { - if let Some(readout) = state.readout.get() { - readout.show(Some(Shown { column, summary })); - } - } + Ok(Some(summary)) => readout.show(Some(Shown { column, summary })), Ok(None) => {} - Err(failure) => tracing::warn!(%failure, "the selection could not be summed"), + Err(failure) => { + tracing::warn!(%failure, "the selection could not be summed"); + readout.show(None); + } } }); } - /// The summed column's name, its rule and its values in the selected - /// rows, as the cells show them. Rows marked for deletion are left out, - /// also once a save failed on them. - fn summary_input(&self) -> Option<(String, SummaryRule, Vec)> { + /// The selected rows as the cells show them, sharing the cells the page + /// read. Rows marked for deletion are left out, also once a save failed + /// on them. + fn summary_input(&self) -> Option { let rows = self.selected_rows(); if rows.len() < MIN_ROWS { return None; @@ -373,11 +390,23 @@ impl DataGrid { let columns = self.grid_columns(); let column = columns.iter().find(|column| column.index == index)?; let tracked = self.tracked(); - let values = rows + let rows = rows .iter() .filter(|row| !tracked.mark(row, index).row_deleted) - .map(|row| tracked.with_value(row, index, |value, default| if default { Value::Null } else { value.clone() })) + .map(|row| tracked.snapshot(row)) .collect(); - Some((column.name.clone(), SummaryRule::for_column(column), values)) + Some(SummaryInput { + column: column.name.clone(), + index, + rule: SummaryRule::for_column(column), + rows, + }) } } + +struct SummaryInput { + column: String, + index: usize, + rule: SummaryRule, + rows: Vec, +} diff --git a/linux/crates/app/src/ui/data_grid/tests/summary.rs b/linux/crates/app/src/ui/data_grid/tests/summary.rs index b1e9164053..6346738cf3 100644 --- a/linux/crates/app/src/ui/data_grid/tests/summary.rs +++ b/linux/crates/app/src/ui/data_grid/tests/summary.rs @@ -197,7 +197,7 @@ fn the_popover_lists_each_figure_and_copies_it_without_grouping() { let labels: Vec = descendants(&popover) .into_iter() .filter_map(|widget| widget.downcast::().ok()) - .filter(|label| label.is_visible()) + .filter(|label| label.get_visible()) .map(|label| label.label().to_string()) .collect(); assert_eq!( @@ -228,6 +228,7 @@ fn the_popover_lists_each_figure_and_copies_it_without_grouping() { .filter_map(|widget| widget.downcast::().ok()) .find(|copy| copy.tooltip_text().as_deref() == Some("Copy Maximum")) .expect("Copy Maximum"); + assert!(copy_maximum.is_sensitive(), "Copy stayed off after the figures came in"); copy_maximum.emit_clicked(); wait_until(WAIT, || !toasts.borrow().is_empty()).expect("the copy lands"); let clipboard = grid.grid.clipboard(); From 000653b4dccca63d8fd1b247e99fb9dd2bb02f21 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 29 Oct 2026 23:36:34 +0700 Subject: [PATCH 05/11] fix(linux-grid): keep a long figure on one line in the summary popover --- .../app/src/ui/data_grid/selection_summary.rs | 5 ++-- .../app/src/ui/data_grid/tests/summary.rs | 23 +++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/linux/crates/app/src/ui/data_grid/selection_summary.rs b/linux/crates/app/src/ui/data_grid/selection_summary.rs index b6075af35b..52d944cb31 100644 --- a/linux/crates/app/src/ui/data_grid/selection_summary.rs +++ b/linux/crates/app/src/ui/data_grid/selection_summary.rs @@ -236,12 +236,11 @@ impl FigureRow { fn new(grid: &DataGrid, figure: Figure) -> Self { let title = gtk::Label::builder().label(figure.title()).xalign(0.0).build(); title.add_css_class("dimmed"); - // A sum can run to thousands of digits; the popover stays narrow. + // A sum can run to thousands of digits; Copy still takes every one. let value = gtk::Label::builder() .xalign(1.0) .hexpand(true) - .wrap(true) - .wrap_mode(gtk::pango::WrapMode::Char) + .ellipsize(gtk::pango::EllipsizeMode::Middle) .max_width_chars(32) .build(); value.add_css_class("numeric"); diff --git a/linux/crates/app/src/ui/data_grid/tests/summary.rs b/linux/crates/app/src/ui/data_grid/tests/summary.rs index 6346738cf3..dc0a8c94cd 100644 --- a/linux/crates/app/src/ui/data_grid/tests/summary.rs +++ b/linux/crates/app/src/ui/data_grid/tests/summary.rs @@ -238,6 +238,29 @@ fn the_popover_lists_each_figure_and_copies_it_without_grouping() { assert_eq!(copied.as_deref(), Some("1234.50")); } +#[gtk4::test] +fn a_long_figure_keeps_its_popover_row_on_one_line() { + let grid = amounts(); + let (_, button) = readout(&grid); + grid.focus(0, 1); + grid.grid.selection().select_all(); + wait_for_label(&button, "Sum 42, Average 21, Count 2"); + let popover = button.popover().expect("the readout's popover"); + let height = || popover.child().expect("the popover's content").measure(gtk::Orientation::Vertical, -1).1; + let short = height(); + + let wide = format!("1{}", "0".repeat(1_500)).parse::().expect("a decimal"); + grid.grid.apply_delta(&grid.tracker.apply(TrackerEdit::Cell { + row_key: key(1), + col: 1, + original: Value::Int(10), + value: Value::Decimal(wide), + })); + wait_until(WAIT, || button.label().is_some_and(|label| label.len() > 1_000)).expect("the wide sum"); + + assert_eq!(height(), short, "a long figure made the popover taller"); +} + #[gtk4::test] fn the_readout_goes_with_its_grid() { let grid = amounts(); From fd4411cc4c434d627107195777b3211c90270d0c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 30 Oct 2026 00:38:10 +0700 Subject: [PATCH 06/11] style(linux-grid): rustfmt the selection summary --- .../app/src/services/grid/selection_summary.rs | 16 ++++++++++++---- .../app/src/ui/data_grid/selection_summary.rs | 3 ++- .../crates/app/src/ui/data_grid/tests/summary.rs | 12 ++++++++++-- linux/crates/app/src/ui/key_tab/tests.rs | 5 ++++- 4 files changed, 28 insertions(+), 8 deletions(-) diff --git a/linux/crates/app/src/services/grid/selection_summary.rs b/linux/crates/app/src/services/grid/selection_summary.rs index d64f8c4557..9796f922ff 100644 --- a/linux/crates/app/src/services/grid/selection_summary.rs +++ b/linux/crates/app/src/services/grid/selection_summary.rs @@ -14,7 +14,9 @@ const AVERAGE_EXTRA_DIGITS: u32 = 4; pub enum SummaryRule { /// `scale` is the column's declared one, which its cells are padded to. /// A value that is not a number, such as NaN, is counted as one. - Numeric { scale: u32 }, + Numeric { + scale: u32, + }, /// No declared type, so text that reads as a number is one. Typed text /// holds identifiers like ZIP codes and is never summed. NumericIfParsable, @@ -249,9 +251,9 @@ fn pow10(exponent: u32) -> BigInt { mod tests { use std::cell::Cell; + use tablepro_core::Value; use tablepro_core::column::{ColumnKind, IntegerKind, IntegerWidth, TextLength}; use tablepro_core::value::{OtherValue, SqlDecimal}; - use tablepro_core::Value; use super::{NumberFigures, SelectionSummary, SummaryRule, summarize}; use crate::services::grid::test_columns::result_only; @@ -372,7 +374,10 @@ mod tests { let untyped = summed(&values, SummaryRule::NumericIfParsable); assert_eq!(untyped.value_count, 4); - assert_eq!(untyped.not_a_number_count, 0, "free text in an untyped column is just text"); + assert_eq!( + untyped.not_a_number_count, 0, + "free text in an untyped column is just text" + ); assert_eq!(untyped.numbers.expect("numbers").sum, "19.5"); let typed = summed(&values, SummaryRule::CountOnly); @@ -410,7 +415,10 @@ mod tests { SummaryRule::for_column(&result_only("price", cents)), SummaryRule::Numeric { scale: 2 } ); - assert_eq!(SummaryRule::for_column(&result_only("zip", zip)), SummaryRule::CountOnly); + assert_eq!( + SummaryRule::for_column(&result_only("zip", zip)), + SummaryRule::CountOnly + ); assert_eq!( SummaryRule::for_column(&result_only("x", ColumnKind::Other)), SummaryRule::NumericIfParsable diff --git a/linux/crates/app/src/ui/data_grid/selection_summary.rs b/linux/crates/app/src/ui/data_grid/selection_summary.rs index 52d944cb31..af7f64a9d8 100644 --- a/linux/crates/app/src/ui/data_grid/selection_summary.rs +++ b/linux/crates/app/src/ui/data_grid/selection_summary.rs @@ -331,7 +331,8 @@ impl DataGrid { } let run = state.runs.get().wrapping_add(1); state.runs.set(run); - let (Some(tasks), Some(input)) = (self.config().map(|config| config.tasks.clone()), self.summary_input()) else { + let tasks = self.config().map(|config| config.tasks.clone()); + let (Some(tasks), Some(input)) = (tasks, self.summary_input()) else { readout.show(None); return; }; diff --git a/linux/crates/app/src/ui/data_grid/tests/summary.rs b/linux/crates/app/src/ui/data_grid/tests/summary.rs index dc0a8c94cd..19640de894 100644 --- a/linux/crates/app/src/ui/data_grid/tests/summary.rs +++ b/linux/crates/app/src/ui/data_grid/tests/summary.rs @@ -246,10 +246,18 @@ fn a_long_figure_keeps_its_popover_row_on_one_line() { grid.grid.selection().select_all(); wait_for_label(&button, "Sum 42, Average 21, Count 2"); let popover = button.popover().expect("the readout's popover"); - let height = || popover.child().expect("the popover's content").measure(gtk::Orientation::Vertical, -1).1; + let height = || { + popover + .child() + .expect("the popover's content") + .measure(gtk::Orientation::Vertical, -1) + .1 + }; let short = height(); - let wide = format!("1{}", "0".repeat(1_500)).parse::().expect("a decimal"); + let wide = format!("1{}", "0".repeat(1_500)) + .parse::() + .expect("a decimal"); grid.grid.apply_delta(&grid.tracker.apply(TrackerEdit::Cell { row_key: key(1), col: 1, diff --git a/linux/crates/app/src/ui/key_tab/tests.rs b/linux/crates/app/src/ui/key_tab/tests.rs index 4db0e79740..bb491aac75 100644 --- a/linux/crates/app/src/ui/key_tab/tests.rs +++ b/linux/crates/app/src/ui/key_tab/tests.rs @@ -218,7 +218,10 @@ fn the_footer_sums_the_selected_entries() { wait_until(WAIT, || tab.grid().focus_cell(0, 0).is_some()).expect("a cell takes the focus"); tab.grid().selection().select_all(); - let button = readout.child().and_downcast::().expect("the menu button"); + let button = readout + .child() + .and_downcast::() + .expect("the menu button"); wait_until(WAIT, || button.label().as_deref() == Some("Sum 3, Average 1, Count 3")).expect("the index summed"); button.popup(); From 281d51fac618dd4f71af4595b88164e665235d69 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 30 Oct 2026 01:41:29 +0700 Subject: [PATCH 07/11] fix(linux-grid): stop the summing and find runs when their grid goes --- linux/crates/app/src/ui/data_grid/find_bar.rs | 27 ++++++++++++++++--- linux/crates/app/src/ui/data_grid/imp.rs | 3 +++ .../app/src/ui/data_grid/selection_summary.rs | 12 ++++++--- .../app/src/ui/data_grid/tests/summary.rs | 17 ++++++++++++ 4 files changed, 51 insertions(+), 8 deletions(-) diff --git a/linux/crates/app/src/ui/data_grid/find_bar.rs b/linux/crates/app/src/ui/data_grid/find_bar.rs index 13f985eea0..8062f1aafc 100644 --- a/linux/crates/app/src/ui/data_grid/find_bar.rs +++ b/linux/crates/app/src/ui/data_grid/find_bar.rs @@ -36,6 +36,14 @@ struct Found { answered: bool, } +impl FindState { + pub(super) fn stop(&self) { + if let Some(running) = self.running.take() { + running.store(true, Ordering::Relaxed); + } + } +} + impl Found { fn label(&self) -> String { if !self.answered { @@ -180,9 +188,7 @@ impl DataGrid { /// answers. fn start_search(&self, query: String, moving: bool) { let state = &self.imp().find; - if let Some(running) = state.running.take() { - running.store(true, Ordering::Relaxed); - } + state.stop(); let search = state.searches.get().wrapping_add(1); state.searches.set(search); state.moving.set(moving); @@ -386,7 +392,7 @@ mod tests { use tablepro_core::Value; use super::super::test_grid::{Grid, GridSpec, WAIT, focus_in_grid, named, results}; - use crate::test_support::{KeyInjector, wait_until}; + use crate::test_support::{KeyInjector, drain_main_context, wait_until}; fn text(value: &str) -> Value { Value::Text(value.to_owned()) @@ -558,6 +564,19 @@ mod tests { assert_eq!(on_match(&grid), Some((42, 1))); } + #[gtk4::test] + fn closing_the_grid_stops_the_search_still_running() { + let grid = orchard(); + grid.activate("find"); + grid.grid.start_search("apple".to_owned(), true); + let running = grid.grid.imp().find.running.borrow().clone().expect("the search runs"); + + drop(grid); + drain_main_context(); + + assert!(running.load(std::sync::atomic::Ordering::Relaxed)); + } + #[gtk4::test] fn rows_that_change_are_searched_again() { let grid = orchard(); diff --git a/linux/crates/app/src/ui/data_grid/imp.rs b/linux/crates/app/src/ui/data_grid/imp.rs index ca64c78d80..e689b876f8 100644 --- a/linux/crates/app/src/ui/data_grid/imp.rs +++ b/linux/crates/app/src/ui/data_grid/imp.rs @@ -115,6 +115,9 @@ impl ObjectImpl for DataGrid { } fn dispose(&self) { + // A run still out on the task pool would go on over every row. + self.find.stop(); + self.summary.stop(); self.obj().stop_watching_focus(); if let Some(menus) = self.menus.get() { menus.unparent(); diff --git a/linux/crates/app/src/ui/data_grid/selection_summary.rs b/linux/crates/app/src/ui/data_grid/selection_summary.rs index af7f64a9d8..b16b537818 100644 --- a/linux/crates/app/src/ui/data_grid/selection_summary.rs +++ b/linux/crates/app/src/ui/data_grid/selection_summary.rs @@ -25,13 +25,19 @@ pub struct SummaryState { queued: Cell, /// Counts runs, so an answer to an older one is dropped. runs: Cell, - running: RefCell>>, + pub(super) running: RefCell>>, } impl SummaryState { pub(super) fn forget_column(&self) { self.column.set(None); } + + pub(super) fn stop(&self) { + if let Some(running) = self.running.take() { + running.store(true, Ordering::Relaxed); + } + } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -326,9 +332,7 @@ impl DataGrid { let Some(readout) = state.readout.get() else { return; }; - if let Some(running) = state.running.take() { - running.store(true, Ordering::Relaxed); - } + state.stop(); let run = state.runs.get().wrapping_add(1); state.runs.set(run); let tasks = self.config().map(|config| config.tasks.clone()); diff --git a/linux/crates/app/src/ui/data_grid/tests/summary.rs b/linux/crates/app/src/ui/data_grid/tests/summary.rs index 19640de894..d92ffb7934 100644 --- a/linux/crates/app/src/ui/data_grid/tests/summary.rs +++ b/linux/crates/app/src/ui/data_grid/tests/summary.rs @@ -3,6 +3,7 @@ use std::rc::Rc; use gtk4 as gtk; use gtk4::prelude::*; +use gtk4::subclass::prelude::*; use tablepro_core::Value; use tablepro_core::value::SqlDecimal; @@ -285,3 +286,19 @@ fn the_readout_goes_with_its_grid() { assert!(revealer_ref.upgrade().is_none(), "the readout outlived its grid"); assert!(button_ref.upgrade().is_none(), "the menu button outlived its grid"); } + +#[gtk4::test] +fn closing_the_grid_stops_the_run_still_out() { + let grid = amounts(); + readout(&grid); + grid.focus(0, 1); + grid.grid.selection().select_all(); + let running = || grid.grid.imp().summary.running.borrow().clone(); + wait_until(WAIT, || running().is_some()).expect("a run starts"); + let running = running().expect("the run"); + + drop(grid); + drain_main_context(); + + assert!(running.load(std::sync::atomic::Ordering::Relaxed)); +} From 5621f6f58e14bd533ac5f62a1f2381efe6c0e4ca Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 30 Oct 2026 01:50:23 +0700 Subject: [PATCH 08/11] perf(linux-grid): find a pending draft by its id, not by a scan of every draft --- linux/Cargo.lock | 1 + linux/crates/app/Cargo.toml | 2 + .../crates/app/src/services/change_tracker.rs | 89 +++++++++++-------- .../app/src/services/tab_tracker_handle.rs | 2 +- 4 files changed, 55 insertions(+), 39 deletions(-) diff --git a/linux/Cargo.lock b/linux/Cargo.lock index ba07f1c9b3..1767555881 100644 --- a/linux/Cargo.lock +++ b/linux/Cargo.lock @@ -5549,6 +5549,7 @@ dependencies = [ "gtk4", "icu_decimal", "icu_locale_core", + "indexmap 2.14.2", "libadwaita", "log", "num-bigint", diff --git a/linux/crates/app/Cargo.toml b/linux/crates/app/Cargo.toml index 5187a3365d..c340f035ec 100644 --- a/linux/crates/app/Cargo.toml +++ b/linux/crates/app/Cargo.toml @@ -100,6 +100,8 @@ gettext-rs = { version = "0.8", features = ["gettext-system"] } # Counts grouped the way the user's locale groups digits. icu_decimal = { version = "2.3", features = ["alloc"] } icu_locale_core = { version = "2.3", features = ["alloc"] } +# Pending drafts, found by id and kept in order. +indexmap = "2.14" # A selection's sum and average, exact past what i128 holds. num-bigint.workspace = true # The assistant's replies are Markdown; the HTML writer stays off. diff --git a/linux/crates/app/src/services/change_tracker.rs b/linux/crates/app/src/services/change_tracker.rs index 68d0deabc1..7ca6368097 100644 --- a/linux/crates/app/src/services/change_tracker.rs +++ b/linux/crates/app/src/services/change_tracker.rs @@ -5,6 +5,8 @@ use std::collections::{HashMap, HashSet, VecDeque}; use std::sync::Arc; +use indexmap::IndexMap; +use indexmap::map::Slice; use tablepro_core::dml::BuildSqlError; use tablepro_core::edit::CellInput; use tablepro_core::meta::RowIdentity; @@ -128,7 +130,7 @@ struct SetAside { #[derive(Debug, Default)] pub struct TabChangeTracker { - drafts: Vec, + drafts: IndexMap, deletes: HashMap, updates: HashMap<(RowKey, usize), CellEdit>, undo: VecDeque, @@ -198,8 +200,8 @@ impl TabChangeTracker { fn draft_in_flight(&self, draft_id: u64) -> bool { self.saving.as_ref().is_some_and(|save| { self.drafts - .iter() - .any(|draft| draft.draft_id == draft_id && save.snapshot.includes(draft.touched)) + .get(&draft_id) + .is_some_and(|draft| save.snapshot.includes(draft.touched)) }) } @@ -317,9 +319,7 @@ impl TabChangeTracker { // The draft's undo and redo entries go with it, so Ctrl+Z cannot // bring it back half-edited. fn drop_draft(&mut self, draft_id: u64) -> bool { - let before = self.drafts.len(); - self.drafts.retain(|d| d.draft_id != draft_id); - if self.drafts.len() == before { + if self.drafts.shift_remove(&draft_id).is_none() { return false; } self.updates @@ -348,7 +348,7 @@ impl TabChangeTracker { } fn draft_cell(&self, draft_id: u64, col: usize) -> Option<(CellInput, Touch)> { - let draft = self.drafts.iter().find(|d| d.draft_id == draft_id)?; + let draft = self.drafts.get(&draft_id)?; Some((draft.cells.get(col)?.clone(), draft.touched)) } @@ -390,7 +390,8 @@ impl TabChangeTracker { fn put_drafts(&mut self, drafts: Vec) -> TrackerDelta { let draft_ids: Vec = drafts.iter().map(|draft| draft.draft_id).collect(); - self.drafts.extend(drafts); + self.drafts + .extend(drafts.into_iter().map(|draft| (draft.draft_id, draft))); TrackerDelta { inserted_drafts: draft_ids.clone(), ..self.delta(draft_ids.into_iter().map(RowKey::Draft).collect()) @@ -399,7 +400,8 @@ impl TabChangeTracker { fn remove_drafts(&mut self, drafts: &[DraftRow]) -> TrackerDelta { let draft_ids: Vec = drafts.iter().map(|draft| draft.draft_id).collect(); - self.drafts.retain(|d| !draft_ids.contains(&d.draft_id)); + let removed: HashSet = draft_ids.iter().copied().collect(); + self.drafts.retain(|draft_id, _| !removed.contains(draft_id)); TrackerDelta { removed_drafts: draft_ids.clone(), ..self.delta(draft_ids.into_iter().map(RowKey::Draft).collect()) @@ -407,7 +409,7 @@ impl TabChangeTracker { } fn put_draft_cell(&mut self, draft_id: u64, col: usize, input: CellInput, touched: Touch) -> TrackerDelta { - let Some(draft) = self.drafts.iter_mut().find(|d| d.draft_id == draft_id) else { + let Some(draft) = self.drafts.get_mut(&draft_id) else { return self.delta(Vec::new()); }; let Some(cell) = draft.cells.get_mut(col) else { @@ -487,10 +489,10 @@ impl TabChangeTracker { .map(|(row_key, _)| row_key) .chain(self.deletes.keys()) .cloned() - .chain(self.drafts.iter().map(|draft| RowKey::Draft(draft.draft_id))) + .chain(self.drafts.keys().map(|draft_id| RowKey::Draft(*draft_id))) .filter(|row_key| seen.insert(row_key.clone())) .collect(); - let removed_drafts = self.drafts.iter().map(|draft| draft.draft_id).collect(); + let removed_drafts = self.drafts.keys().copied().collect(); self.drafts.clear(); self.deletes.clear(); self.updates.clear(); @@ -570,8 +572,8 @@ impl TabChangeTracker { } } - pub fn drafts(&self) -> &[DraftRow] { - &self.drafts + pub fn drafts(&self) -> &Slice { + self.drafts.as_slice() } /// Mark the row a failed save points at. The row marked before, if @@ -595,39 +597,29 @@ impl TabChangeTracker { /// A draft's values, with a cell left to the server as NULL. pub fn draft_values(&self, draft_id: u64) -> Option> { - self.drafts - .iter() - .find(|draft| draft.draft_id == draft_id) - .map(|draft| { - draft - .cells - .iter() - .map(|input| input.value().cloned().unwrap_or(Value::Null)) - .collect() - }) + self.drafts.get(&draft_id).map(|draft| { + draft + .cells + .iter() + .map(|input| input.value().cloned().unwrap_or(Value::Null)) + .collect() + }) } pub fn draft_inputs(&self, draft_id: u64) -> Option> { - self.drafts - .iter() - .find(|draft| draft.draft_id == draft_id) - .map(|draft| draft.cells.clone()) + self.drafts.get(&draft_id).map(|draft| draft.cells.clone()) } pub fn draft_input(&self, draft_id: u64, col: usize) -> Option { self.drafts - .iter() - .find(|draft| draft.draft_id == draft_id) + .get(&draft_id) .and_then(|draft| draft.cells.get(col)) .cloned() } /// A draft's cells as its INSERT carries them. pub fn draft_cells(&self, draft_id: u64, columns: &[ColumnInfo]) -> Option> { - self.drafts - .iter() - .find(|draft| draft.draft_id == draft_id) - .map(|draft| insert_cells(columns, draft)) + self.drafts.get(&draft_id).map(|draft| insert_cells(columns, draft)) } /// What the user changed, as the change set a save is planned from. @@ -642,7 +634,7 @@ impl TabChangeTracker { /// one, as does any column the server fills. pub fn change_set(&self, columns: &[ColumnInfo]) -> Result { let mut rows: Vec<(Touch, RowChange, StatementSource)> = Vec::new(); - for draft in &self.drafts { + for draft in self.drafts.values() { rows.push(( draft.touched, RowChange::Insert { @@ -750,11 +742,11 @@ impl TabChangeTracker { } rows.push(row_key); } - for mut draft in std::mem::take(&mut self.drafts) { + for (draft_id, mut draft) in std::mem::take(&mut self.drafts) { match layout.draft(&draft.cells) { Ok(cells) => { draft.cells = cells; - self.drafts.push(draft); + self.drafts.insert(draft_id, draft); } Err(misfit) => { removed_drafts.push(draft.draft_id); @@ -948,7 +940,7 @@ impl TabChangeTracker { !saved }); self.set_aside.retain(|change| !snapshot.includes(change.touched)); - self.drafts.retain(|draft| { + self.drafts.retain(|_, draft| { let saved = snapshot.includes(draft.touched); if saved { rows.push(RowKey::Draft(draft.draft_id)); @@ -1687,6 +1679,27 @@ mod tests { assert!(t.can_undo()); } + #[test] + fn the_largest_paste_reads_back_and_undoes_without_a_scan_per_draft() { + let mut t = TabChangeTracker::default(); + let rows = (0..tablepro_core::import::MAX_PASTED_ROWS) + .map(|row| vec![CellInput::Value(Value::Int(row as i64))]) + .collect(); + let drafts = t.track_insert(rows).inserted_drafts; + + let started = std::time::Instant::now(); + let read = drafts + .iter() + .filter(|draft_id| t.draft_values(**draft_id).is_some()) + .count(); + assert!(t.undo().is_some()); + let elapsed = started.elapsed(); + + assert_eq!(read, tablepro_core::import::MAX_PASTED_ROWS); + assert!(t.drafts().is_empty()); + assert!(elapsed < std::time::Duration::from_secs(2), "took {elapsed:?}"); + } + #[test] fn one_delete_over_many_rows_is_one_undo_step() { let mut t = TabChangeTracker::default(); diff --git a/linux/crates/app/src/services/tab_tracker_handle.rs b/linux/crates/app/src/services/tab_tracker_handle.rs index 3a82b7b5f6..ba8f576cbf 100644 --- a/linux/crates/app/src/services/tab_tracker_handle.rs +++ b/linux/crates/app/src/services/tab_tracker_handle.rs @@ -72,7 +72,7 @@ impl TabTrackerHandle { } pub fn drafts(&self) -> Vec { - self.0.borrow().drafts().to_vec() + self.0.borrow().drafts().values().cloned().collect() } pub fn draft_values(&self, draft_id: u64) -> Option> { From 4768eab2848cadbb522f40f6bbf239081b821e59 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 30 Oct 2026 01:54:56 +0700 Subject: [PATCH 09/11] perf(linux-grid): read only the summed column of each selected row --- .../app/src/ui/data_grid/selection_summary.rs | 2 +- .../app/src/ui/data_grid/tracked_values.rs | 25 +++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/linux/crates/app/src/ui/data_grid/selection_summary.rs b/linux/crates/app/src/ui/data_grid/selection_summary.rs index b16b537818..fc47750136 100644 --- a/linux/crates/app/src/ui/data_grid/selection_summary.rs +++ b/linux/crates/app/src/ui/data_grid/selection_summary.rs @@ -397,7 +397,7 @@ impl DataGrid { let rows = rows .iter() .filter(|row| !tracked.mark(row, index).row_deleted) - .map(|row| tracked.snapshot(row)) + .map(|row| tracked.column_snapshot(row, index)) .collect(); Some(SummaryInput { column: column.name.clone(), diff --git a/linux/crates/app/src/ui/data_grid/tracked_values.rs b/linux/crates/app/src/ui/data_grid/tracked_values.rs index 0bf12b3739..8e9aac6670 100644 --- a/linux/crates/app/src/ui/data_grid/tracked_values.rs +++ b/linux/crates/app/src/ui/data_grid/tracked_values.rs @@ -217,6 +217,17 @@ impl TrackedValues { RowSnapshot::new(cells, overrides) } + /// `snapshot` that reads only `column` through the tracker, for a + /// caller that reads no other column of the row. + pub fn column_snapshot(&self, row: &GridRow, column: usize) -> RowSnapshot { + let cells = row.cells().unwrap_or_else(|| Arc::from([])); + let pending = self + .tracked_key(row) + .and_then(|(tracker, key)| tracker.cell_overlay(&key, column).pending) + .map(|input| (column, input.value().cloned().unwrap_or(Value::Null))); + RowSnapshot::new(cells, pending.into_iter().collect()) + } + /// Like `snapshot`, except a draft's cell left to the server stays /// `Default`. pub fn inputs(&self, row: &GridRow) -> Vec { @@ -279,11 +290,25 @@ mod tests { }); assert!(values.with_value(&row, 1, |value, _| *value == Value::Int(6))); + assert_eq!(values.column_snapshot(&row, 1).value(1), &Value::Int(6)); assert_eq!(values.edit_input(&row, 1), CellInput::Value(Value::Int(6))); assert_eq!(values.row_state(&row), RowPendingState::Modified); assert_eq!(values.mark(&row, 1).state, CellState::Modified); } + #[test] + fn a_column_snapshot_of_a_draft_reads_its_cell_and_a_default_as_null() { + let tracker = TabTrackerHandle::default(); + let values = TrackedValues::tracked(tracker.clone(), keyed()); + let delta = tracker.apply(TrackerEdit::InsertDrafts { + rows: vec![vec![CellInput::Default, CellInput::Value(Value::Int(7))]], + }); + let row = GridRow::new(RowData::draft(delta.inserted_drafts[0])); + + assert_eq!(values.column_snapshot(&row, 1).value(1), &Value::Int(7)); + assert_eq!(values.column_snapshot(&row, 0).value(0), &Value::Null); + } + #[test] fn every_pending_state_has_an_icon_and_a_sentence() { for state in [ From 1382fd9a13bf6d147b491b63af317dcdd6b25f19 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 30 Oct 2026 05:00:40 +0700 Subject: [PATCH 10/11] test(linux-grid): park the run's answer so closing the grid is what stops it --- linux/crates/app/src/ui/data_grid/find_bar.rs | 9 +++++++-- .../crates/app/src/ui/data_grid/selection_summary.rs | 2 +- linux/crates/app/src/ui/data_grid/tests/summary.rs | 11 ++++++++--- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/linux/crates/app/src/ui/data_grid/find_bar.rs b/linux/crates/app/src/ui/data_grid/find_bar.rs index 8062f1aafc..551e7ee0eb 100644 --- a/linux/crates/app/src/ui/data_grid/find_bar.rs +++ b/linux/crates/app/src/ui/data_grid/find_bar.rs @@ -388,7 +388,7 @@ mod tests { use gtk4::prelude::*; use gtk4::subclass::prelude::*; - use gtk4::{self as gtk, gdk}; + use gtk4::{self as gtk, gdk, glib}; use tablepro_core::Value; use super::super::test_grid::{Grid, GridSpec, WAIT, focus_in_grid, named, results}; @@ -568,7 +568,12 @@ mod tests { fn closing_the_grid_stops_the_search_still_running() { let grid = orchard(); grid.activate("find"); - grid.grid.start_search("apple".to_owned(), true); + // The answer lands on a context nothing runs, so the search is still + // out when the grid goes. + let parked = glib::MainContext::new(); + parked + .with_thread_default(|| grid.grid.start_search("apple".to_owned(), true)) + .expect("the context is free"); let running = grid.grid.imp().find.running.borrow().clone().expect("the search runs"); drop(grid); diff --git a/linux/crates/app/src/ui/data_grid/selection_summary.rs b/linux/crates/app/src/ui/data_grid/selection_summary.rs index fc47750136..7a3bba69a5 100644 --- a/linux/crates/app/src/ui/data_grid/selection_summary.rs +++ b/linux/crates/app/src/ui/data_grid/selection_summary.rs @@ -327,7 +327,7 @@ impl DataGrid { /// Sum the values off the GTK thread: a page holds up to 100,000 rows. /// A newer run stops this one. - fn start_summary(&self) { + pub(super) fn start_summary(&self) { let state = &self.imp().summary; let Some(readout) = state.readout.get() else { return; diff --git a/linux/crates/app/src/ui/data_grid/tests/summary.rs b/linux/crates/app/src/ui/data_grid/tests/summary.rs index d92ffb7934..3b26c54766 100644 --- a/linux/crates/app/src/ui/data_grid/tests/summary.rs +++ b/linux/crates/app/src/ui/data_grid/tests/summary.rs @@ -293,9 +293,14 @@ fn closing_the_grid_stops_the_run_still_out() { readout(&grid); grid.focus(0, 1); grid.grid.selection().select_all(); - let running = || grid.grid.imp().summary.running.borrow().clone(); - wait_until(WAIT, || running().is_some()).expect("a run starts"); - let running = running().expect("the run"); + drain_main_context(); + // The answer lands on a context nothing runs, so the run is still out + // when the grid goes. + let parked = gtk::glib::MainContext::new(); + parked + .with_thread_default(|| grid.grid.start_summary()) + .expect("the context is free"); + let running = grid.grid.imp().summary.running.borrow().clone().expect("the run"); drop(grid); drain_main_context(); From aab0aa683d0cb1a8de05abbd394e7868d0c05027 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 30 Oct 2026 05:00:41 +0700 Subject: [PATCH 11/11] fix(linux-structure): show the table as read, not the discarded edits, while Discard reads it again --- linux/changelog.d/3877.fixed.md | 1 + .../app/src/ui/structure_tab/save_handlers.rs | 9 +++++---- linux/crates/app/src/ui/structure_tab/tests.rs | 18 ++++++++++++++++++ 3 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 linux/changelog.d/3877.fixed.md diff --git a/linux/changelog.d/3877.fixed.md b/linux/changelog.d/3877.fixed.md new file mode 100644 index 0000000000..821acdeb90 --- /dev/null +++ b/linux/changelog.d/3877.fixed.md @@ -0,0 +1 @@ +Discarding structure changes on a table that changed elsewhere left them on screen while it was read again. diff --git a/linux/crates/app/src/ui/structure_tab/save_handlers.rs b/linux/crates/app/src/ui/structure_tab/save_handlers.rs index 03ed1318dd..a28efa38c0 100644 --- a/linux/crates/app/src/ui/structure_tab/save_handlers.rs +++ b/linux/crates/app/src/ui/structure_tab/save_handlers.rs @@ -37,10 +37,6 @@ impl StructureModel { SaveInput::Discard => { // This also lets go of a save whose outcome was unknown. self.tracker.clear(); - if self.mode() == StructureMode::Edit && self.definition_stale { - fx.input(SaveInput::Reload); - return; - } // New mode clears everything but the name, since a new // table starts empty. let restored = match self.context.borrow().original() { @@ -49,6 +45,11 @@ impl StructureModel { }; self.show_draft(restored); self.recompute_dirty_state(fx); + // The read keeps the restored definition on screen while it + // runs and through a failure, never the discarded edits. + if self.mode() == StructureMode::Edit && self.definition_stale { + fx.input(SaveInput::Reload); + } } SaveInput::SaveCompleted { new_table_name } => self.save_completed(new_table_name, fx), SaveInput::SaveFailed(message) => { diff --git a/linux/crates/app/src/ui/structure_tab/tests.rs b/linux/crates/app/src/ui/structure_tab/tests.rs index c3c1fd3ea2..3c28fa2a04 100644 --- a/linux/crates/app/src/ui/structure_tab/tests.rs +++ b/linux/crates/app/src/ui/structure_tab/tests.rs @@ -1110,6 +1110,24 @@ fn a_discard_after_a_save_found_the_table_changed_reads_it_again() { assert_eq!(reads_after_discard(Some(SaveInput::TableChangedBeforeSave.into())), 2); } +#[gtk4::test] +fn a_discard_whose_read_fails_leaves_the_discarded_column_out() { + let mut script = postgres_script(); + script + .fetch_table_definition + .once(FakeReply::ok(definition("id"))) + .always(FakeReply::err(DriverError::server("denied"))); + let harness = Harness::open(StructureMode::Edit, script, []); + wait_until(WAIT, || harness.page().as_deref() == Some("editor")).expect("the structure"); + harness.tab().send(ColumnsInput::AddColumn); + harness.tab().send(SaveInput::TableChangedBeforeSave); + + harness.tab().send(SaveInput::Discard); + + wait_until(WAIT, || banner(&harness).is_revealed()).expect("the failed read was not shown"); + assert_eq!(harness.column_names(), ["id"]); +} + /// The titles of the add buttons the tab shows under `container`. fn add_buttons(container: >k::Box) -> Vec { descendants(container)