diff --git a/linux/Cargo.lock b/linux/Cargo.lock index 39e40f47c2..1767555881 100644 --- a/linux/Cargo.lock +++ b/linux/Cargo.lock @@ -5549,8 +5549,10 @@ dependencies = [ "gtk4", "icu_decimal", "icu_locale_core", + "indexmap 2.14.2", "libadwaita", "log", + "num-bigint", "proptest", "pulldown-cmark", "relm4", 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. 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/Cargo.toml b/linux/crates/app/Cargo.toml index 414dd20bde..c340f035ec 100644 --- a/linux/crates/app/Cargo.toml +++ b/linux/crates/app/Cargo.toml @@ -100,6 +100,10 @@ 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. pulldown-cmark = { version = "0.13", default-features = false } sqlformat = "0.5" 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/grid/mod.rs b/linux/crates/app/src/services/grid/mod.rs index 69b283298e..2252854d55 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::{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 new file mode 100644 index 0000000000..9796f922ff --- /dev/null +++ b/linux/crates/app/src/services/grid/selection_summary.rs @@ -0,0 +1,441 @@ +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}; + +const AVERAGE_EXTRA_DIGITS: u32 = 4; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +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, + }, + /// 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 { + let kind = &column.column_type.kind; + if kind.is_untyped() { + return Self::NumericIfParsable; + } + match column.display() { + CellDisplay::Number => Self::Numeric { + scale: kind.decimal_scale().unwrap_or(0), + }, + _ => Self::CountOnly, + } + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SelectionSummary { + pub value_count: u64, + pub empty_count: u64, + pub not_a_number_count: u64, + pub numbers: Option, +} + +/// 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<'a>( + values: impl IntoIterator, + rule: SummaryRule, + cancelled: &dyn Fn() -> bool, +) -> Option { + let mut tally = Tally::default(); + for value in values { + if cancelled() { + 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; + } + 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) + { + self.minimum = Some(number.clone()); + } + if self + .maximum + .as_ref() + .is_none_or(|maximum| number.compare(maximum) == Ordering::Greater) + { + self.maximum = Some(number); + } + } + + 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 } + } + + 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 plus(&self, other: &Self) -> Self { + let scale = self.scale.max(other.scale); + Self { + mantissa: self.rescaled(scale) + other.rescaled(scale), + scale, + } + } + + fn compare(&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 = &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, scale } + } + + fn trimmed(&self) -> Self { + 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 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(); + 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(10u32).pow(exponent) +} + +#[cfg(test)] +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 super::{NumberFigures, SelectionSummary, SummaryRule, summarize}; + use crate::services::grid::test_columns::result_only; + + fn summed(values: &[Value], rule: SummaryRule) -> SelectionSummary { + summarize(values, rule, &|| false).expect("a run nobody cancelled") + } + + fn numbers(values: &[Value], rule: SummaryRule) -> NumberFigures { + summed(values, rule).numbers.expect("numbers") + } + + fn decimal(text: &str) -> Value { + Value::Decimal(text.parse::().expect("a decimal")) + } + + const NUMERIC: SummaryRule = SummaryRule::Numeric { scale: 0 }; + + #[test] + fn decimals_add_exactly_and_keep_their_scale() { + let figures = numbers(&[decimal("0.10"), decimal("0.20"), decimal("19.99")], NUMERIC); + + 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"); + } + + #[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 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 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" + ); + } + + #[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 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!( + 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 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); + 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 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("\u{a0}3.5 ".into()), + Value::Text("n/a".into()), + other, + ]; + + 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.numbers.expect("numbers").sum, "19.5"); + + let typed = summed(&values, SummaryRule::CountOnly); + 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 { scale: 0 } + ); + assert_eq!( + 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("x", ColumnKind::Other)), + SummaryRule::NumericIfParsable + ); + } + + #[test] + 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/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> { 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/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..46fb075649 100644 --- a/linux/crates/app/src/ui/browse_tab/layout.rs +++ b/linux/crates/app/src/ui/browse_tab/layout.rs @@ -152,6 +152,7 @@ pub(super) fn grid(tab: &BrowseTab, init: GridInit) -> DataGrid { grid.set_hexpand(true); let imp = tab.imp(); imp.grid_holder.append(&grid); + 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/find_bar.rs b/linux/crates/app/src/ui/data_grid/find_bar.rs index 13f985eea0..551e7ee0eb 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); @@ -382,11 +388,11 @@ 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}; - 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,24 @@ 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"); + // 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); + 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/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..e689b876f8 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,6 +65,7 @@ pub struct DataGrid { pub columns: RefCell, pub actions: gio::SimpleActionGroup, pub menus: OnceCell, + pub summary: SummaryState, pub edit: EditController, pub find: FindState, /// What a menu was opened on, until it closes. @@ -113,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/mod.rs b/linux/crates/app/src/ui/data_grid/mod.rs index eba768f692..47e2ca9bde 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)] @@ -179,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 new file mode 100644 index 0000000000..7a3bba69a5 --- /dev/null +++ b/linux/crates/app/src/ui/data_grid/selection_summary.rs @@ -0,0 +1,416 @@ +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 super::DataGrid; +use crate::i18n::{gettext, gettext_f}; +use crate::services::grid::{RowSnapshot, SelectionSummary, SummaryRule, summarize}; +use crate::ui::format::{format_decimal, format_thousands}; +use crate::ui::icons; + +/// A summary of one row says no more than its cell. +const MIN_ROWS: usize = 2; + +#[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, + 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)] +struct Shown { + column: String, + summary: SelectionSummary, +} + +/// 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 Readout { + fn new(grid: &DataGrid) -> Self { + let heading = gtk::Label::builder() + .xalign(0.0) + .ellipsize(gtk::pango::EllipsizeMode::End) + .build(); + heading.add_css_class("heading"); + 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) + .margin_top(6) + .margin_bottom(6) + .margin_start(6) + .margin_end(6) + .build(); + content.append(&heading); + content.append(&table); + let button = gtk::MenuButton::builder() + .popover(>k::Popover::builder().child(&content).build()) + .direction(gtk::ArrowType::Up) + .can_shrink(true) + .build(); + button.add_css_class("flat"); + 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, + heading, + separator, + figures, + shown: RefCell::default(), + } + } + + /// 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; + } + match &shown { + Some(shown) => { + 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.revealer.set_reveal_child(shown.is_some()); + self.shown.replace(shown); + } +} + +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"), + } + } + + 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"), + } + } + + /// 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>, +} + +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; Copy still takes every one. + let value = gtk::Label::builder() + .xalign(1.0) + .hexpand(true) + .ellipsize(gtk::pango::EllipsizeMode::Middle) + .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(icons::EDIT_COPY) + .tooltip_text(©_title) + .valign(gtk::Align::Center) + .build(); + copy.add_css_class("flat"); + 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) = grid.upgrade() { + crate::ui::clipboard::copy_text(&grid, text.borrow().clone(), |message| grid.toast(message)); + } + }); + Self { + figure, + title, + value, + copy, + copied, + } + } + + 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); + } +} + +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 + } + + /// 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())); + } + if state.readout.get().is_none() || state.queued.replace(true) { + return; + } + 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(); + } + }); + } + + /// Sum the values off the GTK thread: a page holds up to 100,000 rows. + /// A newer run stops this one. + pub(super) fn start_summary(&self) { + let state = &self.imp().summary; + let Some(readout) = state.readout.get() else { + return; + }; + state.stop(); + let run = state.runs.get().wrapping_add(1); + state.runs.set(run); + let tasks = self.config().map(|config| config.tasks.clone()); + let (Some(tasks), Some(input)) = (tasks, self.summary_input()) else { + readout.show(None); + 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 || { + let values = rows.iter().map(|row| row.value(index)); + 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(); + let Some(readout) = state.readout.get() else { + return; + }; + match answer { + Ok(Some(summary)) => readout.show(Some(Shown { column, summary })), + Ok(None) => {} + Err(failure) => { + tracing::warn!(%failure, "the selection could not be summed"); + readout.show(None); + } + } + }); + } + + /// 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; + } + let index = self.imp().summary.column.get()?; + if !self.shown_columns().contains(&index) { + return None; + } + let columns = self.grid_columns(); + let column = columns.iter().find(|column| column.index == index)?; + let tracked = self.tracked(); + let rows = rows + .iter() + .filter(|row| !tracked.mark(row, index).row_deleted) + .map(|row| tracked.column_snapshot(row, index)) + .collect(); + 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/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..3b26c54766 --- /dev/null +++ b/linux/crates/app/src/ui/data_grid/tests/summary.rs @@ -0,0 +1,309 @@ +use std::cell::RefCell; +use std::rc::Rc; + +use gtk4 as gtk; +use gtk4::prelude::*; +use gtk4::subclass::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.get_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"); + 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(); + 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 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(); + 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"); +} + +#[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(); + 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(); + + assert!(running.load(std::sync::atomic::Ordering::Relaxed)); +} 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 [ 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/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..bb491aac75 100644 --- a/linux/crates/app/src/ui/key_tab/tests.rs +++ b/linux/crates/app/src/ui/key_tab/tests.rs @@ -198,6 +198,44 @@ 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 fa6a62603d..d0046c16a0 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()); + page.append(&footer); (page.upcast(), grid) } @@ -87,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/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) 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/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..13a299aff0 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 @@ -280,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