From 3a8df586b2442759e3dc221295a1e814706a7603 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Fri, 2 Oct 2026 14:40:33 +0200 Subject: [PATCH 1/4] colums that are targetted by multiple formats merge them --- doc/syntax/clause/format.qmd | 2 +- src/execute/table/format.rs | 53 +++++++++++++++++++++++++++++++++--- src/table/mod.rs | 16 +++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/doc/syntax/clause/format.qmd b/doc/syntax/clause/format.qmd index 735f8d2fe..f085c4afd 100644 --- a/doc/syntax/clause/format.qmd +++ b/doc/syntax/clause/format.qmd @@ -116,7 +116,7 @@ TABULATE * FROM my_data SPAN "My label" OVER island, species ``` -When a column is targeted by multiple `FORMAT` clauses, the last one wins. +When a column is targeted by multiple `FORMAT` clauses, the clauses are combined; in case of conflicts the last `FORMAT` wins. Note that the `island` column is not in the stub. ```{ggsql} diff --git a/src/execute/table/format.rs b/src/execute/table/format.rs index bdcb4501c..23e032525 100644 --- a/src/execute/table/format.rs +++ b/src/execute/table/format.rs @@ -27,8 +27,8 @@ pub(super) fn setup_formats( .map_err(|e| GgsqlError::ValidationError(format!("FORMAT {}: {}", idx + 1, e)))?; } - // One `Format` per column; a later clause wins over an earlier one - // naming the same column. + // One `Format` per column; clauses naming the same column merge + // (`Format::merge`), with the later clause winning per field. let mut resolved = HashMap::new(); for format in formats { for name in &format.columns { @@ -47,7 +47,10 @@ pub(super) fn setup_formats( let mut format = format.clone(); // The map key names the column instead. format.columns = Vec::new(); - resolved.insert(column.clone(), format); + resolved + .entry(column.clone()) + .and_modify(|existing: &mut Format| existing.merge(&format)) + .or_insert(format); } } } @@ -153,7 +156,7 @@ mod tests { } #[test] - fn setup_formats_lets_a_later_clause_win_and_clears_columns() { + fn setup_formats_lets_a_later_template_win_and_clears_columns() { let frame = df! { "price" => vec![1.0f64] }.unwrap(); let formats = vec![ Format { @@ -173,6 +176,48 @@ mod tests { assert!(price.columns.is_empty()); } + #[test] + fn setup_formats_merges_clauses_on_the_same_column() { + let frame = df! { + "amount" => vec![1.0f64], + "price" => vec![2.0f64], + "currency" => vec![3.0f64] + } + .unwrap(); + let formats = vec![ + Format { + columns: vec!["amount".to_string(), "price".to_string()], + ..format_with("{:num %.2f}", None) + }, + Format { + columns: vec!["price".to_string(), "currency".to_string()], + ..format_with_hjust(ParameterValue::String("left".to_string())) + }, + ]; + + let resolved = setup_formats(&frame, &formats, &[]).unwrap(); + + // `price` merges both clauses: the later clause has the default "{}" + // template, so the earlier template survives; its settings are added. + let price = resolved.get("price").unwrap(); + assert_eq!(price.value_template, "{:num %.2f}"); + assert_eq!( + price.settings.get("hjust"), + Some(&ParameterValue::String("left".to_string())) + ); + + let amount = resolved.get("amount").unwrap(); + assert_eq!(amount.value_template, "{:num %.2f}"); + assert!(!amount.settings.contains_key("hjust")); + + let currency = resolved.get("currency").unwrap(); + assert_eq!(currency.value_template, "{}"); + assert_eq!( + currency.settings.get("hjust"), + Some(&ParameterValue::String("left".to_string())) + ); + } + #[test] fn setup_formats_expands_a_span_id_into_its_columns() { let frame = df! { "a" => vec![1.0f64], "b" => vec![2.0f64] }.unwrap(); diff --git a/src/table/mod.rs b/src/table/mod.rs index d79bbe065..1993b8975 100644 --- a/src/table/mod.rs +++ b/src/table/mod.rs @@ -359,6 +359,22 @@ const FORMAT_PARAMS: &[ParamDefinition] = &[ ]; impl Format { + /// Merge `other` into `self`: `other`'s settings and value mappings + /// override per-key, and its target replaces `self`'s; its template + /// replaces `self`'s unless it is the default `"{}"`. + pub fn merge(&mut self, other: &Format) { + self.target = other.target; + self.settings.extend(other.settings.clone()); + match (&mut self.value_mapping, &other.value_mapping) { + (Some(mapping), Some(other)) => mapping.extend(other.clone()), + (slot @ None, Some(other)) => *slot = Some(other.clone()), + _ => {} + } + if other.value_template != "{}" { + self.value_template = other.value_template.clone(); + } + } + /// Validate `settings` against `FORMAT_PARAMS`. pub fn validate_settings(&self) -> Result<(), String> { let valid: Vec<&str> = FORMAT_PARAMS.iter().map(|p| p.name).collect(); From 7dcd16ed5d266e89e811a3129bdd439af1291bfe Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Mon, 12 Oct 2026 17:36:24 +0200 Subject: [PATCH 2/4] Make Format::value_template optional A FORMAT clause that never names a wildcard template now leaves the field None instead of storing the default "{}", so merging stacked FORMAT clauses on one column only overrides a template a clause actually set. parse_renaming_clause returns Option for the template to keep an explicit * => '{}' distinguishable from no wildcard, letting a later clause reset an earlier template. --- src/execute/table/format.rs | 35 +++++++++++++++++++--------------- src/execute/table/layout.rs | 4 ++-- src/parser/builder.rs | 25 +++++++++++++++--------- src/table/mod.rs | 38 ++++++++++++++++++++++++++++++------- 4 files changed, 69 insertions(+), 33 deletions(-) diff --git a/src/execute/table/format.rs b/src/execute/table/format.rs index a3349af36..e0855a2c8 100644 --- a/src/execute/table/format.rs +++ b/src/execute/table/format.rs @@ -60,15 +60,14 @@ pub(super) fn setup_formats( /// Replace every column in `df` named in `formats` with its resolved /// display text. A column whose `Format` has no `RENAMING` at all -/// (`value_template` is the default `"{}"` and `value_mapping` is empty) is -/// left untouched. +/// (`value_template` unset and `value_mapping` empty) is left untouched. pub(super) fn apply_formats( df: &DataFrame, formats: &HashMap, ) -> Result { let mut df = df.clone(); for (name, format) in formats { - let has_renaming = format.value_template != "{}" + let has_renaming = format.value_template.is_some() || format.value_mapping.as_ref().is_some_and(|m| !m.is_empty()); if !has_renaming { // Skip the per-row formatting pass for a column nobody asked to @@ -82,7 +81,7 @@ pub(super) fn apply_formats( .clone(); let resolved = crate::format::resolve_column_values( &array, - &format.value_template, + format.value_template.as_deref().unwrap_or("{}"), &format.value_mapping, ) .map_err(|e| GgsqlError::ValidationError(format!("FORMAT column '{name}': {e}")))?; @@ -132,7 +131,7 @@ mod tests { target: crate::ColumnSection::Body, settings: Parameters::new(), value_mapping: mapping, - value_template: template.to_string(), + value_template: (template != "{}").then(|| template.to_string()), } } @@ -160,7 +159,7 @@ mod tests { let resolved = setup_formats(&frame, &formats, &[]).unwrap(); let price = resolved.get("price").unwrap(); - assert_eq!(price.value_template, "{:num %.2f}"); + assert_eq!(price.value_template.as_deref(), Some("{:num %.2f}")); assert!(price.columns.is_empty()); } @@ -185,21 +184,21 @@ mod tests { let resolved = setup_formats(&frame, &formats, &[]).unwrap(); - // `price` merges both clauses: the later clause has the default "{}" - // template, so the earlier template survives; its settings are added. + // `price` merges both clauses: the later clause has no template, so + // the earlier template survives; its settings are added. let price = resolved.get("price").unwrap(); - assert_eq!(price.value_template, "{:num %.2f}"); + assert_eq!(price.value_template.as_deref(), Some("{:num %.2f}")); assert_eq!( price.settings.get("hjust"), Some(&ParameterValue::String("left".to_string())) ); let amount = resolved.get("amount").unwrap(); - assert_eq!(amount.value_template, "{:num %.2f}"); + assert_eq!(amount.value_template.as_deref(), Some("{:num %.2f}")); assert!(!amount.settings.contains_key("hjust")); let currency = resolved.get("currency").unwrap(); - assert_eq!(currency.value_template, "{}"); + assert_eq!(currency.value_template, None); assert_eq!( currency.settings.get("hjust"), Some(&ParameterValue::String("left".to_string())) @@ -222,8 +221,14 @@ mod tests { let resolved = setup_formats(&frame, &formats, &spans).unwrap(); - assert_eq!(resolved.get("a").unwrap().value_template, "{:num %.2f}"); - assert_eq!(resolved.get("b").unwrap().value_template, "{:num %.2f}"); + assert_eq!( + resolved.get("a").unwrap().value_template.as_deref(), + Some("{:num %.2f}") + ); + assert_eq!( + resolved.get("b").unwrap().value_template.as_deref(), + Some("{:num %.2f}") + ); } #[test] @@ -288,7 +293,7 @@ mod tests { target: crate::ColumnSection::Body, settings, value_mapping: None, - value_template: "{}".to_string(), + value_template: None, } } @@ -311,7 +316,7 @@ mod tests { target: crate::ColumnSection::Stub, settings: Parameters::new(), value_mapping: None, - value_template: "{}".to_string(), + value_template: None, }; assert_eq!( diff --git a/src/execute/table/layout.rs b/src/execute/table/layout.rs index f77902bac..c6e034880 100644 --- a/src/execute/table/layout.rs +++ b/src/execute/table/layout.rs @@ -238,7 +238,7 @@ mod tests { target: ColumnSection::Stub, settings: Parameters::new(), value_mapping: None, - value_template: "{}".to_string(), + value_template: None, }, )]); @@ -460,7 +460,7 @@ mod tests { "0".to_string(), Some("-".to_string()), )])), - value_template: "${:num %.2f}".to_string(), + value_template: Some("${:num %.2f}".to_string()), }]; let cells = resolve_cells(&frame, &table).unwrap(); diff --git a/src/parser/builder.rs b/src/parser/builder.rs index a1b05d3c1..3239d478a 100644 --- a/src/parser/builder.rs +++ b/src/parser/builder.rs @@ -511,7 +511,7 @@ fn build_format_clause(node: &Node, source: &SourceTree) -> Result { }; let mut value_mapping = None; - let mut value_template = "{}".to_string(); + let mut value_template = None; if let Some(renaming_node) = source.find_node(node, "(renaming_clause) @r") { let (mappings, template) = parse_renaming_clause(&renaming_node, source)?; if !mappings.is_empty() { @@ -940,7 +940,7 @@ fn build_scale(node: &Node, source: &SourceTree) -> Result { if !mappings.is_empty() { label_mapping = Some(mappings); } - label_template = template; + label_template = template.unwrap_or_else(crate::format::default_template); } _ => {} } @@ -1060,13 +1060,14 @@ fn parse_scale_via_clause(node: &Node, source: &SourceTree) -> Result /// /// Returns a tuple of: /// - HashMap where: Key = original value, Value = Some(label) or None for suppressed labels -/// - Template string for wildcard mappings (* => '...'), defaults to "{}" +/// - Template string for wildcard mappings (* => '...'), None if no wildcard +#[allow(clippy::type_complexity)] fn parse_renaming_clause( node: &Node, source: &SourceTree, -) -> Result<(HashMap>, String)> { +) -> Result<(HashMap>, Option)> { let mut mappings = HashMap::new(); - let mut template = "{}".to_string(); + let mut template = None; // Find all renaming_assignment nodes let query = "(renaming_assignment) @assign"; @@ -1108,7 +1109,7 @@ fn parse_renaming_clause( if is_wildcard { // Wildcard: * => 'template' if let Some(tmpl) = to_value { - template = tmpl; + template = Some(tmpl); } } else { // Explicit mapping: 'A' => 'Alpha' or '10' => 'Ten' @@ -1614,7 +1615,7 @@ mod tests { assert_eq!(table.formats[0].target, ColumnSection::Body); assert!(table.formats[0].settings.is_empty()); assert_eq!(table.formats[0].value_mapping, None); - assert_eq!(table.formats[0].value_template, "{}"); + assert_eq!(table.formats[0].value_template, None); } #[test] @@ -1653,7 +1654,10 @@ mod tests { table.formats[0].value_mapping, Some(HashMap::from([("null".to_string(), Some("-".to_string()))])) ); - assert_eq!(table.formats[0].value_template, "{:num %.2f}"); + assert_eq!( + table.formats[0].value_template, + Some("{:num %.2f}".to_string()) + ); } #[test] @@ -1666,7 +1670,10 @@ mod tests { assert_eq!(table.formats.len(), 2); assert_eq!(table.formats[0].columns, vec!["foo"]); - assert_eq!(table.formats[0].value_template, "{:num %.0f}"); + assert_eq!( + table.formats[0].value_template, + Some("{:num %.0f}".to_string()) + ); assert_eq!(table.formats[1].columns, vec!["bar"]); } diff --git a/src/table/mod.rs b/src/table/mod.rs index 68cf75a1e..d283327e6 100644 --- a/src/table/mod.rs +++ b/src/table/mod.rs @@ -244,10 +244,9 @@ pub struct Format { pub value_mapping: Option>>, /// Template for generating display text from cell values (e.g. /// `"{:num %.2f}"`), applied to values with no specific `value_mapping` - /// entry. Default `"{}"` passes the value through unchanged. Same shape - /// as `Scale::label_template`. - #[serde(default = "crate::format::default_template")] - pub value_template: String, + /// entry. `None` passes values through unchanged (equivalent to `"{}"`). + #[serde(default)] + pub value_template: Option, } /// `SETTING` parameters `FORMAT` accepts. @@ -274,7 +273,7 @@ const FORMAT_PARAMS: &[ParamDefinition] = &[ impl Format { /// Merge `other` into `self`: `other`'s settings and value mappings /// override per-key, and its target replaces `self`'s; its template - /// replaces `self`'s unless it is the default `"{}"`. + /// replaces `self`'s when set. pub fn merge(&mut self, other: &Format) { self.target = other.target; self.settings.extend(other.settings.clone()); @@ -283,7 +282,7 @@ impl Format { (slot @ None, Some(other)) => *slot = Some(other.clone()), _ => {} } - if other.value_template != "{}" { + if other.value_template.is_some() { self.value_template = other.value_template.clone(); } } @@ -318,10 +317,35 @@ mod tests { target: ColumnSection::Body, settings, value_mapping: None, - value_template: "{}".to_string(), + value_template: None, } } + #[test] + fn format_merge_an_unset_template_leaves_the_existing_one() { + let mut format = Format { + value_template: Some("{:num %.2f}".to_string()), + ..format_with_settings(Parameters::new()) + }; + format.merge(&format_with_settings(Parameters::new())); + + assert_eq!(format.value_template.as_deref(), Some("{:num %.2f}")); + } + + #[test] + fn format_merge_an_explicit_default_template_overrides() { + let mut format = Format { + value_template: Some("{:num %.2f}".to_string()), + ..format_with_settings(Parameters::new()) + }; + format.merge(&Format { + value_template: Some("{}".to_string()), + ..format_with_settings(Parameters::new()) + }); + + assert_eq!(format.value_template.as_deref(), Some("{}")); + } + #[test] fn format_validate_settings_accepts_a_string_hjust() { let mut settings = Parameters::new(); From 4fa5cf257527c01654ba5b2239a970c5cff96076 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 13 Oct 2026 10:01:56 +0200 Subject: [PATCH 3/4] Add ANY target to FORMAT clause, defaulting to no section claim A FORMAT clause without a target identifier (or with the new ANY keyword) now makes no claim about which table section its columns belong to: Format.target is Option, merging stacked clauses lets concrete BODY/STUB targets win over ANY, and unclaimed columns resolve to the body in setup_formats. Previously a bare FORMAT counted as a BODY claim and would silently pull columns out of the stub when combined with FORMAT STUB. --- doc/syntax/clause/format.qmd | 16 +++++++----- src/execute/table/format.rs | 15 +++++++---- src/execute/table/layout.rs | 6 ++--- src/parser/builder.rs | 33 ++++++++++++++++-------- src/table/mod.rs | 50 +++++++++++++++++++++++++++--------- tree-sitter-ggsql/grammar.js | 5 ++-- 6 files changed, 87 insertions(+), 38 deletions(-) diff --git a/doc/syntax/clause/format.qmd b/doc/syntax/clause/format.qmd index f085c4afd..fe69fa10b 100644 --- a/doc/syntax/clause/format.qmd +++ b/doc/syntax/clause/format.qmd @@ -8,23 +8,26 @@ It is the workhorse for column-focussed changes: value formatting, display prope ## Clause syntax ```ggsql -FORMAT [BODY | STUB] , ... +FORMAT [ANY | BODY | STUB] , ... SETTING => , ... RENAMING => , ... ``` ## Target ``` -[BODY | STUB] +[ANY | BODY | STUB] ``` The target directly following the `FORMAT` clause determines what kind of column is being formatted. -The optional target defaults to `BODY` which targets cells in the table body. -`FORMAT` can be used without spelling out a target, in which case it is interpreted as a `BODY` target. +Setting the target to `BODY` affects the cells in the body part of the table. The target can be set to `STUB`, which will move the column to the left of the table to serve as row labels. +Without spelling out a target, the default will be `ANY`. +The `ANY` target is relevant when the same column is formatted by multiple `FORMAT` statements. +`ANY` will give priority to `BODY` and `STUB` targets in other statements. +Unless instructed otherwise, `ANY` will resolve to `BODY`. ```ggsql --- Same as `FORMAT BODY foo, bar` +-- Same as `FORMAT ANY foo, bar` FORMAT foo, bar -- Designate column as stub @@ -117,7 +120,8 @@ TABULATE * FROM my_data ``` When a column is targeted by multiple `FORMAT` clauses, the clauses are combined; in case of conflicts the last `FORMAT` wins. -Note that the `island` column is not in the stub. +Without target, `ANY` target is assumed which will not override targets. +Note that the `island` column is not in the stub, because `BODY` overrides `STUB`. ```{ggsql} TABULATE * FROM my_data diff --git a/src/execute/table/format.rs b/src/execute/table/format.rs index e0855a2c8..f7244a20c 100644 --- a/src/execute/table/format.rs +++ b/src/execute/table/format.rs @@ -11,7 +11,7 @@ use arrow::datatypes::DataType; use crate::array_util::{new_str_array, value_to_string}; use crate::plot::{ParameterValue, Parameters}; use crate::table::standardise_hjust; -use crate::{DataFrame, Format, GgsqlError, Result, Spanner}; +use crate::{ColumnSection, DataFrame, Format, GgsqlError, Result, Spanner}; /// Validate every FORMAT's `SETTING` parameters, then reshape `formats` /// into one `Format` per column it covers. `spans` must already be resolved @@ -55,6 +55,11 @@ pub(super) fn setup_formats( } } } + + // A column no clause claimed a section for resolves to the body. + for format in resolved.values_mut() { + format.target.get_or_insert(ColumnSection::Body); + } Ok(resolved) } @@ -106,7 +111,7 @@ pub(super) fn apply_formats( pub(super) fn resolve_column_properties(dtype: &DataType, format: Option<&Format>) -> Parameters { let mut properties = format.map(|f| f.settings.clone()).unwrap_or_default(); - let is_stub = format.is_some_and(|f| f.target.is_stub()); + let is_stub = format.is_some_and(|f| f.target == Some(ColumnSection::Stub)); let hjust = if let Some(value) = properties.get("hjust").and_then(standardise_hjust) { value } else if is_stub || dtype.is_numeric() { @@ -128,7 +133,7 @@ mod tests { fn format_with(template: &str, mapping: Option>>) -> Format { Format { columns: Vec::new(), - target: crate::ColumnSection::Body, + target: None, settings: Parameters::new(), value_mapping: mapping, value_template: (template != "{}").then(|| template.to_string()), @@ -290,7 +295,7 @@ mod tests { settings.insert("hjust".to_string(), hjust); Format { columns: Vec::new(), - target: crate::ColumnSection::Body, + target: None, settings, value_mapping: None, value_template: None, @@ -313,7 +318,7 @@ mod tests { fn resolve_column_properties_defaults_a_stub_column_to_right_regardless_of_dtype() { let format = Format { columns: Vec::new(), - target: crate::ColumnSection::Stub, + target: Some(crate::ColumnSection::Stub), settings: Parameters::new(), value_mapping: None, value_template: None, diff --git a/src/execute/table/layout.rs b/src/execute/table/layout.rs index c6e034880..da3c26fc0 100644 --- a/src/execute/table/layout.rs +++ b/src/execute/table/layout.rs @@ -99,7 +99,7 @@ fn create_table_columns( .data_type(); let format = formats.get(&name); let properties = resolve_column_properties(dtype, format); - let target = format.map(|f| f.target).unwrap_or_default(); + let target = format.and_then(|f| f.target).unwrap_or_default(); let label = match labels.labels.get(&name) { // A stub head is a row-label header, not a data column header — // default it blank rather than to the column's own name. @@ -235,7 +235,7 @@ mod tests { "region".to_string(), Format { columns: vec!["region".to_string()], - target: ColumnSection::Stub, + target: Some(ColumnSection::Stub), settings: Parameters::new(), value_mapping: None, value_template: None, @@ -454,7 +454,7 @@ mod tests { let mut table = Table::new(); table.formats = vec![crate::Format { columns: vec!["price".to_string()], - target: crate::ColumnSection::Body, + target: None, settings: Parameters::new(), value_mapping: Some(std::collections::HashMap::from([( "0".to_string(), diff --git a/src/parser/builder.rs b/src/parser/builder.rs index 3239d478a..f78acaabc 100644 --- a/src/parser/builder.rs +++ b/src/parser/builder.rs @@ -529,18 +529,22 @@ fn build_format_clause(node: &Node, source: &SourceTree) -> Result { }) } -/// Parse a FORMAT clause's optional target identifier (BODY, STUB), missing -/// meaning BODY by default. -fn parse_format_target_identifier(node: &Node, source: &SourceTree) -> Result { +/// Parse a FORMAT clause's optional target identifier: `BODY`/`STUB` make a +/// section claim, while `ANY` or a missing identifier makes none (`None`). +fn parse_format_target_identifier( + node: &Node, + source: &SourceTree, +) -> Result> { let Some(target_node) = source.find_node(node, "(format_target_identifier) @t") else { - return Ok(ColumnSection::Body); + return Ok(None); }; match source.get_text(&target_node).to_lowercase().as_str() { - "body" => Ok(ColumnSection::Body), - "stub" => Ok(ColumnSection::Stub), + "any" => Ok(None), + "body" => Ok(Some(ColumnSection::Body)), + "stub" => Ok(Some(ColumnSection::Stub)), text => Err(GgsqlError::ParseError(format!( - "Unknown FORMAT target: '{}'. Valid targets: body, stub", + "Unknown FORMAT target: '{}'. Valid targets: any, body, stub", text ))), } @@ -1612,7 +1616,7 @@ mod tests { assert_eq!(table.formats.len(), 1); assert_eq!(table.formats[0].columns, vec!["foo", "bar"]); - assert_eq!(table.formats[0].target, ColumnSection::Body); + assert_eq!(table.formats[0].target, None); assert!(table.formats[0].settings.is_empty()); assert_eq!(table.formats[0].value_mapping, None); assert_eq!(table.formats[0].value_template, None); @@ -1624,7 +1628,7 @@ mod tests { let table = specs[0].as_table().expect("expected a Table spec"); assert_eq!(table.formats[0].columns, vec!["region"]); - assert_eq!(table.formats[0].target, ColumnSection::Stub); + assert_eq!(table.formats[0].target, Some(ColumnSection::Stub)); } #[test] @@ -1633,7 +1637,16 @@ mod tests { let table = specs[0].as_table().expect("expected a Table spec"); assert_eq!(table.formats[0].columns, vec!["region"]); - assert_eq!(table.formats[0].target, ColumnSection::Body); + assert_eq!(table.formats[0].target, Some(ColumnSection::Body)); + } + + #[test] + fn test_tabulate_format_explicit_any_target() { + let specs = parse_test_specs("TABULATE * FROM sales FORMAT ANY region").unwrap(); + let table = specs[0].as_table().expect("expected a Table spec"); + + assert_eq!(table.formats[0].columns, vec!["region"]); + assert_eq!(table.formats[0].target, None); } #[test] diff --git a/src/table/mod.rs b/src/table/mod.rs index d283327e6..0f9e47d64 100644 --- a/src/table/mod.rs +++ b/src/table/mod.rs @@ -150,7 +150,7 @@ impl Table { let stub_columns: HashSet<&str> = self .formats .iter() - .filter(|format| format.target.is_stub()) + .filter(|format| format.target == Some(ColumnSection::Stub)) .flat_map(|format| &format.columns) .flat_map( |name| match resolved_spans.iter().find(|span| &span.id == name) { @@ -204,8 +204,8 @@ impl Table { /// Which section of the table a `FORMAT` clause's columns belong to. /// /// `FORMAT STUB` is the only thing that assigns a column to the table's -/// stub; `FORMAT BODY`, or no target identifier at all, leaves columns in -/// the regular body — `Body` is this enum's default for that reason. +/// stub; `FORMAT BODY`, or no claim at all, leaves columns in the regular +/// body — `Body` is this enum's default for that reason. #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum ColumnSection { @@ -229,10 +229,12 @@ impl ColumnSection { pub struct Format { /// The columns this FORMAT applies to, in the order written. pub columns: Vec, - /// Which section of the table these columns belong to (`BODY`, the - /// default, or `STUB`). + /// Which section of the table these columns belong to (`BODY` or + /// `STUB`). `None` — from `FORMAT ANY` or no target identifier — makes + /// no section claim: it never overrides another target when FORMAT + /// clauses merge, and resolves to `BODY` in `setup_formats`. #[serde(default)] - pub target: ColumnSection, + pub target: Option, /// `SETTING` parameters for this FORMAT (e.g. `hjust => 'right'`). pub settings: Parameters, /// Value mappings for custom cell display (`RENAMING` clause). Maps a raw @@ -272,10 +274,12 @@ const FORMAT_PARAMS: &[ParamDefinition] = &[ impl Format { /// Merge `other` into `self`: `other`'s settings and value mappings - /// override per-key, and its target replaces `self`'s; its template - /// replaces `self`'s when set. + /// override per-key, and its target and template replace `self`'s when + /// set. pub fn merge(&mut self, other: &Format) { - self.target = other.target; + if other.target.is_some() { + self.target = other.target; + } self.settings.extend(other.settings.clone()); match (&mut self.value_mapping, &other.value_mapping) { (Some(mapping), Some(other)) => mapping.extend(other.clone()), @@ -314,7 +318,7 @@ mod tests { fn format_with_settings(settings: Parameters) -> Format { Format { columns: vec!["a".to_string()], - target: ColumnSection::Body, + target: None, settings, value_mapping: None, value_template: None, @@ -346,6 +350,28 @@ mod tests { assert_eq!(format.value_template.as_deref(), Some("{}")); } + #[test] + fn format_merge_an_unset_target_never_overrides() { + let mut format = Format { + target: Some(ColumnSection::Stub), + ..format_with_settings(Parameters::new()) + }; + format.merge(&format_with_settings(Parameters::new())); + + assert_eq!(format.target, Some(ColumnSection::Stub)); + } + + #[test] + fn format_merge_a_set_target_overrides() { + let mut format = format_with_settings(Parameters::new()); + format.merge(&Format { + target: Some(ColumnSection::Stub), + ..format_with_settings(Parameters::new()) + }); + + assert_eq!(format.target, Some(ColumnSection::Stub)); + } + #[test] fn format_validate_settings_accepts_a_string_hjust() { let mut settings = Parameters::new(); @@ -398,7 +424,7 @@ mod tests { let table = Table { formats: vec![Format { columns: vec!["a".to_string()], - target: ColumnSection::Stub, + target: Some(ColumnSection::Stub), ..format_with_settings(Parameters::new()) }], spans: vec![Spanner { @@ -423,7 +449,7 @@ mod tests { let table = Table { formats: vec![Format { columns: vec!["G".to_string()], - target: ColumnSection::Stub, + target: Some(ColumnSection::Stub), ..format_with_settings(Parameters::new()) }], spans: vec![Spanner { diff --git a/tree-sitter-ggsql/grammar.js b/tree-sitter-ggsql/grammar.js index 25aa6cf1f..452c6da8c 100644 --- a/tree-sitter-ggsql/grammar.js +++ b/tree-sitter-ggsql/grammar.js @@ -698,7 +698,7 @@ module.exports = grammar({ // FORMAT — configures cell formatting for a group of columns. Multiple // FORMAT clauses repeat (FORMAT ... FORMAT ...) for different column // groups, the same model SPAN uses. Optional target identifier before - // the column list, defaulting to BODY. + // the column list, defaulting to ANY. format_clause: $ => seq( caseInsensitive('FORMAT'), optional($.format_target_identifier), // optional target before columns @@ -709,7 +709,8 @@ module.exports = grammar({ // FORMAT targets - which section of the table the columns belong to format_target_identifier: $ => choice( - caseInsensitive('BODY'), // regular table body (default) + caseInsensitive('ANY'), // wherever the columns are (default) + caseInsensitive('BODY'), // regular table body caseInsensitive('STUB') // row-label column(s) ), From 2eb6c91d6048455eb0e7af225c6d1a35a01972c5 Mon Sep 17 00:00:00 2001 From: Teun van den Brand Date: Tue, 13 Oct 2026 12:38:11 +0200 Subject: [PATCH 4/4] Allow FORMAT to target all columns with * FORMAT * sets a wildcard selection, expanded to the data's columns in setup_formats via the new expand_format_columns helper (which also absorbs SPAN id expansion and unknown-column validation). Wildcard clauses merge like any other, so an explicit clause can still refine a single column after a catch-all FORMAT *. --- doc/syntax/clause/format.qmd | 18 +++- src/execute/table/format.rs | 112 +++++++++++++++--------- src/execute/table/layout.rs | 2 + src/execute/table/mod.rs | 19 ++++ src/parser/builder.rs | 33 +++++-- src/table/mod.rs | 10 ++- tree-sitter-ggsql/grammar.js | 5 +- tree-sitter-ggsql/test/corpus/basic.txt | 24 +++++ 8 files changed, 171 insertions(+), 52 deletions(-) diff --git a/doc/syntax/clause/format.qmd b/doc/syntax/clause/format.qmd index fe69fa10b..80860cd73 100644 --- a/doc/syntax/clause/format.qmd +++ b/doc/syntax/clause/format.qmd @@ -8,7 +8,8 @@ It is the workhorse for column-focussed changes: value formatting, display prope ## Clause syntax ```ggsql -FORMAT [ANY | BODY | STUB] , ... +FORMAT [ANY | BODY | STUB] + (* | , ...) SETTING => , ... RENAMING => , ... ``` @@ -37,16 +38,20 @@ FORMAT STUB foo ## Column selection ```ggsql -, ... +(* | , ...) ``` The `FORMAT` clause requires a comma-separated list of columns to format. In addition to column names, it can use [`SPAN`](span.qmd) identifiers as well to target the spanner's columns. +It can also take *, meaning to format all columns. ```ggsql -- Targeting regular columns FORMAT foo, bar +-- Targeting every column +FORMAT * + -- Targeting spanner id: bar, baz and qux become targeted SPAN foo OVER bar, baz FORMAT foo, qux @@ -145,6 +150,13 @@ TABULATE * FROM my_data SETTING width => '150px' ``` +Replace null values everywhere by selecting all columns. + +```{ggsql} +TABULATE * FROM my_data + FORMAT * RENAMING null => '---' +``` + Replacing specific pieces of content. ```{ggsql} @@ -162,4 +174,4 @@ Using a formatter to target all cells in a column. TABULATE * FROM my_data FORMAT body_mass RENAMING * => '{:num %.0f}g' -``` \ No newline at end of file +``` diff --git a/src/execute/table/format.rs b/src/execute/table/format.rs index f7244a20c..52f61ea1b 100644 --- a/src/execute/table/format.rs +++ b/src/execute/table/format.rs @@ -30,29 +30,19 @@ pub(super) fn setup_formats( // One `Format` per column; clauses naming the same column merge // (`Format::merge`), with the later clause winning per field. + let colnames = df.get_column_names(); let mut resolved = HashMap::new(); for format in formats { - for name in &format.columns { - // A SPAN id never collides with a real column name - // (`Spanner::validate_columns`), so this lookup is unambiguous. - let columns: Vec<&String> = match spans.iter().find(|span| &span.id == name) { - Some(span) => span.columns.iter().collect(), - None => vec![name], - }; - for column in columns { - if df.column(column).is_err() { - return Err(GgsqlError::ValidationError(format!( - "FORMAT references unknown column '{column}'" - ))); - } - let mut format = format.clone(); - // The map key names the column instead. - format.columns = Vec::new(); - resolved - .entry(column.clone()) - .and_modify(|existing: &mut Format| existing.merge(&format)) - .or_insert(format); - } + for column in expand_format_columns(format, &colnames, spans)? { + let mut format = format.clone(); + // Resolve the selection to the single column this entry is keyed + // under, so re-resolving a resolved map is a fixed point. + format.columns = vec![column.clone()]; + format.wildcard = false; + resolved + .entry(column) + .and_modify(|existing: &mut Format| existing.merge(&format)) + .or_insert(format); } } @@ -63,6 +53,37 @@ pub(super) fn setup_formats( Ok(resolved) } +/// Expand a FORMAT's column selection to concrete column names: `*` to every +/// column, a SPAN id to its columns, anything else to itself. Errors on a +/// name that is neither a SPAN id nor a column. +fn expand_format_columns( + format: &Format, + colnames: &[String], + spans: &[Spanner], +) -> Result> { + if format.wildcard { + return Ok(colnames.to_vec()); + } + + let mut columns = Vec::new(); + for name in &format.columns { + // A SPAN id never collides with a real column name + // (`Spanner::validate_columns`), so this lookup is unambiguous. + match spans.iter().find(|span| &span.id == name) { + Some(span) => columns.extend(span.columns.iter().cloned()), + None => columns.push(name.clone()), + } + } + for column in &columns { + if !colnames.contains(column) { + return Err(GgsqlError::ValidationError(format!( + "FORMAT references unknown column '{column}'" + ))); + } + } + Ok(columns) +} + /// Replace every column in `df` named in `formats` with its resolved /// display text. A column whose `Format` has no `RENAMING` at all /// (`value_template` unset and `value_mapping` empty) is left untouched. @@ -133,6 +154,7 @@ mod tests { fn format_with(template: &str, mapping: Option>>) -> Format { Format { columns: Vec::new(), + wildcard: false, target: None, settings: Parameters::new(), value_mapping: mapping, @@ -147,27 +169,6 @@ mod tests { .collect() } - #[test] - fn setup_formats_lets_a_later_template_win_and_clears_columns() { - let frame = df! { "price" => vec![1.0f64] }.unwrap(); - let formats = vec![ - Format { - columns: vec!["price".to_string()], - ..format_with("{:num %.0f}", None) - }, - Format { - columns: vec!["price".to_string()], - ..format_with("{:num %.2f}", None) - }, - ]; - - let resolved = setup_formats(&frame, &formats, &[]).unwrap(); - - let price = resolved.get("price").unwrap(); - assert_eq!(price.value_template.as_deref(), Some("{:num %.2f}")); - assert!(price.columns.is_empty()); - } - #[test] fn setup_formats_merges_clauses_on_the_same_column() { let frame = df! { @@ -179,10 +180,12 @@ mod tests { let formats = vec![ Format { columns: vec!["amount".to_string(), "price".to_string()], + wildcard: false, ..format_with("{:num %.2f}", None) }, Format { columns: vec!["price".to_string(), "currency".to_string()], + wildcard: false, ..format_with_hjust(ParameterValue::String("left".to_string())) }, ]; @@ -215,6 +218,7 @@ mod tests { let frame = df! { "a" => vec![1.0f64], "b" => vec![2.0f64] }.unwrap(); let formats = vec![Format { columns: vec!["G".to_string()], + wildcard: false, ..format_with("{:num %.2f}", None) }]; let spans = vec![Spanner { @@ -236,11 +240,33 @@ mod tests { ); } + #[test] + fn setup_formats_expands_a_wildcard_to_all_columns() { + let frame = df! { "a" => vec![1.0f64], "b" => vec![2.0f64] }.unwrap(); + let formats = vec![Format { + wildcard: true, + ..format_with("{:num %.2f}", None) + }]; + + let resolved = setup_formats(&frame, &formats, &[]).unwrap(); + + assert_eq!(resolved.len(), 2); + assert_eq!( + resolved.get("a").unwrap().value_template.as_deref(), + Some("{:num %.2f}") + ); + assert_eq!( + resolved.get("b").unwrap().value_template.as_deref(), + Some("{:num %.2f}") + ); + } + #[test] fn setup_formats_errors_on_an_unknown_column() { let frame = df! { "price" => vec![1.0f64] }.unwrap(); let formats = vec![Format { columns: vec!["typo".to_string()], + wildcard: false, ..format_with("{}", None) }]; @@ -295,6 +321,7 @@ mod tests { settings.insert("hjust".to_string(), hjust); Format { columns: Vec::new(), + wildcard: false, target: None, settings, value_mapping: None, @@ -318,6 +345,7 @@ mod tests { fn resolve_column_properties_defaults_a_stub_column_to_right_regardless_of_dtype() { let format = Format { columns: Vec::new(), + wildcard: false, target: Some(crate::ColumnSection::Stub), settings: Parameters::new(), value_mapping: None, diff --git a/src/execute/table/layout.rs b/src/execute/table/layout.rs index da3c26fc0..a884a2eea 100644 --- a/src/execute/table/layout.rs +++ b/src/execute/table/layout.rs @@ -235,6 +235,7 @@ mod tests { "region".to_string(), Format { columns: vec!["region".to_string()], + wildcard: false, target: Some(ColumnSection::Stub), settings: Parameters::new(), value_mapping: None, @@ -454,6 +455,7 @@ mod tests { let mut table = Table::new(); table.formats = vec![crate::Format { columns: vec!["price".to_string()], + wildcard: false, target: None, settings: Parameters::new(), value_mapping: Some(std::collections::HashMap::from([( diff --git a/src/execute/table/mod.rs b/src/execute/table/mod.rs index 85d008be7..461ca6681 100644 --- a/src/execute/table/mod.rs +++ b/src/execute/table/mod.rs @@ -403,6 +403,25 @@ mod integration_tests { assert_eq!(label_cell.content, "ID"); } + #[test] + fn test_tabulate_format_wildcard_formats_every_column() { + let reader = reader_with_sales(); + let resolved = resolve_table_with_reader( + "TABULATE * FROM sales FORMAT * RENAMING * => '[{}]'", + &reader, + ) + .unwrap(); + + let mut body: Vec<_> = resolved + .cells() + .iter() + .filter(|c| c.kind == TableCellKind::Body) + .map(|c| c.content.as_str()) + .collect(); + body.sort_unstable(); + assert_eq!(body, ["[1]", "[2]", "[3]", "[a]", "[b]", "[c]"]); + } + #[test] fn test_tabulate_format_stub_moves_the_column_to_the_front() { let reader = reader_with_sales(); diff --git a/src/parser/builder.rs b/src/parser/builder.rs index f78acaabc..30798cc1b 100644 --- a/src/parser/builder.rs +++ b/src/parser/builder.rs @@ -496,14 +496,22 @@ fn build_span_clause(node: &Node, source: &SourceTree) -> Result { }) } -/// Build a Format from a format_clause node: FORMAT [BODY|STUB] col, ... [SETTING ...] [RENAMING ...] +/// Build a Format from a format_clause node: FORMAT [ANY|BODY|STUB] (*|col, ...) [SETTING ...] [RENAMING ...] fn build_format_clause(node: &Node, source: &SourceTree) -> Result { let target = parse_format_target_identifier(node, source)?; - let columns_node = source - .find_node(node, "(column_list) @cols") - .ok_or_else(|| GgsqlError::ParseError("Missing columns in FORMAT clause".to_string()))?; - let columns = parse_column_list(&columns_node, source)?; + let wildcard = source.find_node(node, "(wildcard_mapping) @w").is_some(); + let columns = match source.find_node(node, "(column_list) @cols") { + Some(columns_node) => parse_column_list(&columns_node, source)?, + // Grammar guarantees a wildcard when there is no column list; + // setup_formats expands it to the data's columns. + None if wildcard => Vec::new(), + None => { + return Err(GgsqlError::ParseError( + "Missing columns in FORMAT clause".to_string(), + )) + } + }; let settings = match source.find_node(node, "(setting_clause) @s") { Some(setting_node) => parse_setting_clause(&setting_node, source)?, @@ -522,6 +530,7 @@ fn build_format_clause(node: &Node, source: &SourceTree) -> Result { Ok(Format { columns, + wildcard, target, settings, value_mapping, @@ -1649,6 +1658,20 @@ mod tests { assert_eq!(table.formats[0].target, None); } + #[test] + fn test_tabulate_format_wildcard_columns() { + let specs = + parse_test_specs("TABULATE * FROM sales FORMAT * RENAMING * => '{:num %.2f}'").unwrap(); + let table = specs[0].as_table().expect("expected a Table spec"); + + assert!(table.formats[0].wildcard); + assert!(table.formats[0].columns.is_empty()); + assert_eq!( + table.formats[0].value_template, + Some("{:num %.2f}".to_string()) + ); + } + #[test] fn test_tabulate_format_with_setting_and_renaming() { let specs = parse_test_specs( diff --git a/src/table/mod.rs b/src/table/mod.rs index 0f9e47d64..8287da33a 100644 --- a/src/table/mod.rs +++ b/src/table/mod.rs @@ -227,8 +227,13 @@ impl ColumnSection { /// covered column's `TableCell` properties. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Format { - /// The columns this FORMAT applies to, in the order written. + /// The columns this FORMAT applies to, in the order written. Empty when + /// `wildcard` is set. pub columns: Vec, + /// Whether this FORMAT targets every column (`FORMAT *`). Expanded to + /// concrete column names in `setup_formats`, where the data is known. + #[serde(default)] + pub wildcard: bool, /// Which section of the table these columns belong to (`BODY` or /// `STUB`). `None` — from `FORMAT ANY` or no target identifier — makes /// no section claim: it never overrides another target when FORMAT @@ -318,6 +323,7 @@ mod tests { fn format_with_settings(settings: Parameters) -> Format { Format { columns: vec!["a".to_string()], + wildcard: false, target: None, settings, value_mapping: None, @@ -424,6 +430,7 @@ mod tests { let table = Table { formats: vec![Format { columns: vec!["a".to_string()], + wildcard: false, target: Some(ColumnSection::Stub), ..format_with_settings(Parameters::new()) }], @@ -449,6 +456,7 @@ mod tests { let table = Table { formats: vec![Format { columns: vec!["G".to_string()], + wildcard: false, target: Some(ColumnSection::Stub), ..format_with_settings(Parameters::new()) }], diff --git a/tree-sitter-ggsql/grammar.js b/tree-sitter-ggsql/grammar.js index 452c6da8c..56208d569 100644 --- a/tree-sitter-ggsql/grammar.js +++ b/tree-sitter-ggsql/grammar.js @@ -702,7 +702,10 @@ module.exports = grammar({ format_clause: $ => seq( caseInsensitive('FORMAT'), optional($.format_target_identifier), // optional target before columns - $.column_list, + choice( + $.wildcard_mapping, // * formats every column + $.column_list + ), optional($.setting_clause), optional($.renaming_clause) ), diff --git a/tree-sitter-ggsql/test/corpus/basic.txt b/tree-sitter-ggsql/test/corpus/basic.txt index a942acd76..57e3ec8b8 100644 --- a/tree-sitter-ggsql/test/corpus/basic.txt +++ b/tree-sitter-ggsql/test/corpus/basic.txt @@ -4834,6 +4834,30 @@ TABULATE * FROM sales FORMAT foo, bar SETTING width => '20%' RENAMING null => '- (identifier (bare_identifier))))))) +================================================================================ +TABULATE FORMAT clause with wildcard columns +================================================================================ + +TABULATE * FROM sales FORMAT ANY * + +-------------------------------------------------------------------------------- + +(query + (tabulate_statement + (tabulate_keyword) + (column_selection + (mapping_list + (mapping_element + (wildcard_mapping)))) + (single_source_from + source: (qualified_name + (identifier + (bare_identifier)))) + (tab_clause + (format_clause + (format_target_identifier) + (wildcard_mapping))))) + ================================================================================ TABULATE column selection: wildcard, bare column, and a rename ================================================================================