From 5e15d2b6a54e8d4754e3a8dbf12f957b3bc7186a Mon Sep 17 00:00:00 2001 From: edjubert Date: Mon, 14 Sep 2026 11:06:29 +0200 Subject: [PATCH 1/6] test(pretty-print): allow per-fixture format configuration --- .../data/single/format_config_header.sql | 6 ++ .../tests__format_config_header_80.snap | 5 ++ crates/pgls_pretty_print/tests/tests.rs | 86 +++++++++++++++---- 3 files changed, 82 insertions(+), 15 deletions(-) create mode 100644 crates/pgls_pretty_print/tests/data/single/format_config_header.sql create mode 100644 crates/pgls_pretty_print/tests/snapshots/single/tests__format_config_header_80.snap diff --git a/crates/pgls_pretty_print/tests/data/single/format_config_header.sql b/crates/pgls_pretty_print/tests/data/single/format_config_header.sql new file mode 100644 index 000000000..80810d099 --- /dev/null +++ b/crates/pgls_pretty_print/tests/data/single/format_config_header.sql @@ -0,0 +1,6 @@ +-- pgls-format: keywordCase=upper, indentStyle=tabs, indentSize=4, lineWidth=80 +SELECT + t.a, + t.b +FROM s.t +WHERE t.a > 1; diff --git a/crates/pgls_pretty_print/tests/snapshots/single/tests__format_config_header_80.snap b/crates/pgls_pretty_print/tests/snapshots/single/tests__format_config_header_80.snap new file mode 100644 index 000000000..6c5d4d49e --- /dev/null +++ b/crates/pgls_pretty_print/tests/snapshots/single/tests__format_config_header_80.snap @@ -0,0 +1,5 @@ +--- +source: crates/pgls_pretty_print/tests/tests.rs +input_file: crates/pgls_pretty_print/tests/data/single/format_config_header.sql +--- +SELECT t.a, t.b FROM s.t WHERE t.a > 1; diff --git a/crates/pgls_pretty_print/tests/tests.rs b/crates/pgls_pretty_print/tests/tests.rs index 0eb68ed4d..9baab6c7e 100644 --- a/crates/pgls_pretty_print/tests/tests.rs +++ b/crates/pgls_pretty_print/tests/tests.rs @@ -6,7 +6,7 @@ use pgls_pretty_print::{ emitter::EventEmitter, nodes::emit_node_enum, normalize::normalize_ast, - renderer::{IndentStyle, RenderConfig, Renderer}, + renderer::{IndentStyle, KeywordCase, RenderConfig, Renderer}, }; /// Line widths to test - each test file is run at both widths @@ -37,12 +37,64 @@ enum StringState { Dollar(Vec), } +/// A fixture may open with `-- pgls-format: key=value, key=value` to declare the configuration it +/// must be rendered with. Keeping it in the fixture rather than in the harness is what lets an +/// option that is off by default own its own test data. +fn parse_fixture(content: &str) -> (RenderConfig, Option, String) { + const HEADER: &str = "-- pgls-format:"; + + let mut config = RenderConfig { + max_line_length: 100, + indent_size: 2, + indent_style: IndentStyle::Spaces, + ..Default::default() + }; + let mut explicit_width = None; + + let Some(rest) = content.strip_prefix(HEADER) else { + return (config, None, content.to_string()); + }; + + let (header, sql) = match rest.split_once('\n') { + Some((header, sql)) => (header, sql), + None => (rest, ""), + }; + + for entry in header.split(',') { + let Some((key, value)) = entry.split_once('=') else { + panic!("malformed pgls-format entry: {entry}"); + }; + + match (key.trim(), value.trim()) { + ("lineWidth", value) => { + let width = value.parse().expect("lineWidth must be a number"); + config.max_line_length = width; + explicit_width = Some(width); + } + ("indentSize", value) => { + config.indent_size = value.parse().expect("indentSize must be a number"); + } + ("indentStyle", "tabs") => config.indent_style = IndentStyle::Tabs, + ("indentStyle", "spaces") => config.indent_style = IndentStyle::Spaces, + ("keywordCase", "upper") => config.keyword_case = KeywordCase::Upper, + ("keywordCase", "lower") => config.keyword_case = KeywordCase::Lower, + ("constantCase", "upper") => config.constant_case = KeywordCase::Upper, + ("constantCase", "lower") => config.constant_case = KeywordCase::Lower, + ("typeCase", "upper") => config.type_case = KeywordCase::Upper, + ("typeCase", "lower") => config.type_case = KeywordCase::Lower, + (key, value) => panic!("unknown pgls-format entry: {key}={value}"), + } + } + + (config, explicit_width, sql.to_string()) +} + #[dir_test( dir: "$CARGO_MANIFEST_DIR/tests/data/single/", glob: "*.sql", )] fn test_single(fixture: Fixture<&str>) { - let content = fixture.content(); + let (fixture_config, explicit_width, content) = parse_fixture(fixture.content()); println!("Original content:\n{content}"); @@ -53,11 +105,15 @@ fn test_single(fixture: Fixture<&str>) { .and_then(|x| x.strip_suffix(".sql")) .unwrap(); - // Run test at each configured line width - for &max_line_length in &LINE_WIDTHS { + let widths: Vec = match explicit_width { + Some(width) => vec![width], + None => LINE_WIDTHS.to_vec(), + }; + + for max_line_length in widths { let test_name = format!("{base_test_name}_{max_line_length}"); - let parsed = pgls_query::parse(content).expect("Failed to parse SQL"); + let parsed = pgls_query::parse(&content).expect("Failed to parse SQL"); let mut ast = parsed.into_root().expect("No root node found"); println!("Parsed AST: {ast:#?}"); @@ -68,9 +124,7 @@ fn test_single(fixture: Fixture<&str>) { let mut output = String::new(); let config = RenderConfig { max_line_length, - indent_size: 2, - indent_style: IndentStyle::Spaces, - ..Default::default() + ..fixture_config.clone() }; let mut renderer = Renderer::new(&mut output, config); renderer.render(emitter.events).expect("Failed to render"); @@ -119,19 +173,23 @@ fn test_multi(fixture: Fixture<&str>) { } } - let content = fixture.content(); + let (fixture_config, explicit_width, content) = parse_fixture(fixture.content()); let input_file = absolute_fixture_path; let base_test_name = absolute_fixture_path .file_name() .and_then(|x| x.strip_suffix(".sql")) .unwrap(); - // Run test at each configured line width - for &max_line_length in &LINE_WIDTHS { + let widths: Vec = match explicit_width { + Some(width) => vec![width], + None => LINE_WIDTHS.to_vec(), + }; + + for max_line_length in widths { let test_name = format!("{base_test_name}_{max_line_length}"); // Split the content into statements - let split_result = pgls_statement_splitter::split(content); + let split_result = pgls_statement_splitter::split(&content); let mut formatted_statements = Vec::new(); for range in &split_result.ranges { @@ -153,9 +211,7 @@ fn test_multi(fixture: Fixture<&str>) { let mut output = String::new(); let config = RenderConfig { max_line_length, - indent_size: 2, - indent_style: IndentStyle::Spaces, - ..Default::default() + ..fixture_config.clone() }; let mut renderer = Renderer::new(&mut output, config); renderer.render(emitter.events).expect("Failed to render"); From a513ed25d4c3a80451fa38a61686b4f2f6414379 Mon Sep 17 00:00:00 2001 From: edjubert Date: Mon, 14 Sep 2026 11:07:49 +0200 Subject: [PATCH 2/6] refactor(pretty-print): thread the format config into the emitter --- crates/pgls_pretty_print/src/emitter.rs | 21 +++++++++++++++--- crates/pgls_pretty_print/src/lib.rs | 2 +- crates/pgls_pretty_print/src/renderer.rs | 10 ++++----- crates/pgls_pretty_print/tests/tests.rs | 27 +++++++++++++----------- 4 files changed, 39 insertions(+), 21 deletions(-) diff --git a/crates/pgls_pretty_print/src/emitter.rs b/crates/pgls_pretty_print/src/emitter.rs index 368c79b23..19c0a6db6 100644 --- a/crates/pgls_pretty_print/src/emitter.rs +++ b/crates/pgls_pretty_print/src/emitter.rs @@ -1,3 +1,4 @@ +use crate::FormatConfig; pub use crate::codegen::group_kind::GroupKind; pub use crate::codegen::token_kind::TokenKind; @@ -22,14 +23,28 @@ pub enum LayoutEvent { IndentEnd, } -#[derive(Debug, Default)] +/// Collects layout events for the renderer. +/// +/// The emitter holds the configuration because some options decide which tokens exist at all, +/// such as where a comma sits in a list, and not merely how a token is rendered. +#[derive(Debug)] pub struct EventEmitter { pub events: Vec, + config: FormatConfig, } impl EventEmitter { - pub fn new() -> Self { - Self::default() + pub fn new(config: FormatConfig) -> Self { + Self { + events: Vec::new(), + config, + } + } + + // Later option PRs inspect this while deciding which layout events to emit. + #[allow(dead_code)] + pub fn config(&self) -> &FormatConfig { + &self.config } pub fn token(&mut self, token: TokenKind) { diff --git a/crates/pgls_pretty_print/src/lib.rs b/crates/pgls_pretty_print/src/lib.rs index 008e4faa8..818b672e1 100644 --- a/crates/pgls_pretty_print/src/lib.rs +++ b/crates/pgls_pretty_print/src/lib.rs @@ -104,7 +104,7 @@ pub fn format_statement( config: &FormatConfig, ) -> Result { // Emit layout events from AST - let mut emitter = emitter::EventEmitter::new(); + let mut emitter = emitter::EventEmitter::new(config.clone()); nodes::emit_node_enum(ast, &mut emitter); // Render to string diff --git a/crates/pgls_pretty_print/src/renderer.rs b/crates/pgls_pretty_print/src/renderer.rs index 36dcbbff9..83e708cb6 100644 --- a/crates/pgls_pretty_print/src/renderer.rs +++ b/crates/pgls_pretty_print/src/renderer.rs @@ -325,7 +325,7 @@ mod tests { #[test] fn test_keyword_case_upper() { - let mut emitter = EventEmitter::new(); + let mut emitter = EventEmitter::new(crate::FormatConfig::default()); emitter.token(TokenKind::SELECT_KW); emitter.space(); emitter.token(TokenKind::INT_NUMBER(1)); @@ -342,7 +342,7 @@ mod tests { #[test] fn test_keyword_case_lower() { - let mut emitter = EventEmitter::new(); + let mut emitter = EventEmitter::new(crate::FormatConfig::default()); emitter.token(TokenKind::SELECT_KW); emitter.space(); emitter.token(TokenKind::INT_NUMBER(1)); @@ -359,7 +359,7 @@ mod tests { #[test] fn test_constant_case_upper() { - let mut emitter = EventEmitter::new(); + let mut emitter = EventEmitter::new(crate::FormatConfig::default()); emitter.token(TokenKind::SELECT_KW); emitter.space(); emitter.token(TokenKind::NULL); @@ -382,7 +382,7 @@ mod tests { #[test] fn test_constant_case_lower() { - let mut emitter = EventEmitter::new(); + let mut emitter = EventEmitter::new(crate::FormatConfig::default()); emitter.token(TokenKind::SELECT_KW); emitter.space(); emitter.token(TokenKind::NULL); @@ -402,7 +402,7 @@ mod tests { #[test] fn test_mixed_case_settings() { - let mut emitter = EventEmitter::new(); + let mut emitter = EventEmitter::new(crate::FormatConfig::default()); emitter.token(TokenKind::SELECT_KW); emitter.space(); emitter.token(TokenKind::INT_NUMBER(1)); diff --git a/crates/pgls_pretty_print/tests/tests.rs b/crates/pgls_pretty_print/tests/tests.rs index 9baab6c7e..8fd18c94d 100644 --- a/crates/pgls_pretty_print/tests/tests.rs +++ b/crates/pgls_pretty_print/tests/tests.rs @@ -3,6 +3,7 @@ use dir_test::{Fixture, dir_test}; use insta::{assert_snapshot, with_settings}; use pgls_pretty_print::{ + FormatConfig, emitter::EventEmitter, nodes::emit_node_enum, normalize::normalize_ast, @@ -40,15 +41,13 @@ enum StringState { /// A fixture may open with `-- pgls-format: key=value, key=value` to declare the configuration it /// must be rendered with. Keeping it in the fixture rather than in the harness is what lets an /// option that is off by default own its own test data. -fn parse_fixture(content: &str) -> (RenderConfig, Option, String) { +/// +/// It returns a `FormatConfig`, not a `RenderConfig`: some options decide which tokens exist and +/// are therefore read by the emitter, so the fixture has to reach both sides of the pipeline. +fn parse_fixture(content: &str) -> (FormatConfig, Option, String) { const HEADER: &str = "-- pgls-format:"; - let mut config = RenderConfig { - max_line_length: 100, - indent_size: 2, - indent_style: IndentStyle::Spaces, - ..Default::default() - }; + let mut config = FormatConfig::default(); let mut explicit_width = None; let Some(rest) = content.strip_prefix(HEADER) else { @@ -68,7 +67,7 @@ fn parse_fixture(content: &str) -> (RenderConfig, Option, String) { match (key.trim(), value.trim()) { ("lineWidth", value) => { let width = value.parse().expect("lineWidth must be a number"); - config.max_line_length = width; + config.line_width = width; explicit_width = Some(width); } ("indentSize", value) => { @@ -118,13 +117,15 @@ fn test_single(fixture: Fixture<&str>) { println!("Parsed AST: {ast:#?}"); - let mut emitter = EventEmitter::new(); + // The emitter gets the fixture config, not the default one: an option that decides which + // tokens exist is read here, before the renderer ever sees the events. + let mut emitter = EventEmitter::new(fixture_config.clone()); emit_node_enum(&ast, &mut emitter); let mut output = String::new(); let config = RenderConfig { max_line_length, - ..fixture_config.clone() + ..RenderConfig::from(fixture_config.clone()) }; let mut renderer = Renderer::new(&mut output, config); renderer.render(emitter.events).expect("Failed to render"); @@ -205,13 +206,15 @@ fn test_multi(fixture: Fixture<&str>) { println!("Parsed AST: {ast:#?}"); - let mut emitter = EventEmitter::new(); + // The emitter gets the fixture config, not the default one: an option that decides + // which tokens exist is read here, before the renderer ever sees the events. + let mut emitter = EventEmitter::new(fixture_config.clone()); emit_node_enum(&ast, &mut emitter); let mut output = String::new(); let config = RenderConfig { max_line_length, - ..fixture_config.clone() + ..RenderConfig::from(fixture_config.clone()) }; let mut renderer = Renderer::new(&mut output, config); renderer.render(emitter.events).expect("Failed to render"); From 8540b2d12ec3c781c60f8487f183be8c069b5840 Mon Sep 17 00:00:00 2001 From: edjubert Date: Mon, 14 Sep 2026 11:20:10 +0200 Subject: [PATCH 3/6] feat(format): add the commaStyle option --- crates/pgls_configuration/src/format.rs | 35 +++++++++++++++++++ crates/pgls_pretty_print/src/lib.rs | 35 +++++++++++++++---- crates/pgls_workspace/src/settings.rs | 7 +++- crates/pgls_workspace/src/workspace/server.rs | 1 + docs/features/formatting.md | 1 + docs/schema.json | 19 ++++++++++ 6 files changed, 91 insertions(+), 7 deletions(-) diff --git a/crates/pgls_configuration/src/format.rs b/crates/pgls_configuration/src/format.rs index 7855571cd..106511ed7 100644 --- a/crates/pgls_configuration/src/format.rs +++ b/crates/pgls_configuration/src/format.rs @@ -70,6 +70,37 @@ impl From for pgls_pretty_print::renderer::KeywordCase { } } +/// Where a comma sits when a list breaks across lines. +#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, Merge, PartialEq, Serialize)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[serde(rename_all = "lowercase")] +pub enum CommaStyle { + #[default] + Trailing, + Leading, +} + +impl FromStr for CommaStyle { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + match s { + "trailing" => Ok(Self::Trailing), + "leading" => Ok(Self::Leading), + _ => Err("Value not supported for CommaStyle. Use 'trailing' or 'leading'."), + } + } +} + +impl From for pgls_pretty_print::CommaStyle { + fn from(style: CommaStyle) -> Self { + match style { + CommaStyle::Trailing => Self::Trailing, + CommaStyle::Leading => Self::Leading, + } + } +} + /// The configuration for SQL formatting. #[derive(Clone, Debug, Deserialize, Eq, Partial, PartialEq, Serialize)] #[partial(derive(Bpaf, Clone, Eq, PartialEq, Merge))] @@ -97,6 +128,9 @@ pub struct FormatConfiguration { /// Data type casing (text, varchar, int): "upper" or "lower". Default: "lower". #[partial(bpaf(long("type-case")))] pub type_case: KeywordCase, + /// Where a comma sits when a list breaks: "trailing" or "leading". Default: "trailing". + #[partial(bpaf(long("comma-style")))] + pub comma_style: CommaStyle, /// If `true`, skip formatting of SQL function bodies (keep them verbatim). Default: `false`. #[partial(bpaf(long("skip-fn-bodies")))] pub skip_fn_bodies: bool, @@ -118,6 +152,7 @@ impl Default for FormatConfiguration { keyword_case: KeywordCase::default(), constant_case: KeywordCase::default(), type_case: KeywordCase::default(), + comma_style: CommaStyle::default(), skip_fn_bodies: false, ignore: Default::default(), include: Default::default(), diff --git a/crates/pgls_pretty_print/src/lib.rs b/crates/pgls_pretty_print/src/lib.rs index 818b672e1..de7b00119 100644 --- a/crates/pgls_pretty_print/src/lib.rs +++ b/crates/pgls_pretty_print/src/lib.rs @@ -32,6 +32,16 @@ pub enum FormatError { } /// Configuration for the SQL formatter. +/// Where a comma sits when a list breaks across lines. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub enum CommaStyle { + /// `a,` at the end of the line. + #[default] + Trailing, + /// `, a` at the start of the continuation line. + Leading, +} + #[derive(Debug, Clone)] pub struct FormatConfig { /// Maximum line width before breaking. Default: 100. @@ -46,6 +56,8 @@ pub struct FormatConfig { pub constant_case: KeywordCase, /// Casing for data types (text, varchar, int). Default: Lower. pub type_case: KeywordCase, + /// Where a comma sits when a list breaks. Default: Trailing. + pub comma_style: CommaStyle, } impl Default for FormatConfig { @@ -57,19 +69,30 @@ impl Default for FormatConfig { keyword_case: KeywordCase::default(), constant_case: KeywordCase::default(), type_case: KeywordCase::default(), + comma_style: CommaStyle::default(), } } } impl From for RenderConfig { fn from(config: FormatConfig) -> Self { + let FormatConfig { + line_width, + indent_size, + indent_style, + keyword_case, + constant_case, + type_case, + comma_style: _, + } = config; + Self { - max_line_length: config.line_width, - indent_size: config.indent_size, - indent_style: config.indent_style, - keyword_case: config.keyword_case, - constant_case: config.constant_case, - type_case: config.type_case, + max_line_length: line_width, + indent_size, + indent_style, + keyword_case, + constant_case, + type_case, } } } diff --git a/crates/pgls_workspace/src/settings.rs b/crates/pgls_workspace/src/settings.rs index 70d864dc0..40ffae218 100644 --- a/crates/pgls_workspace/src/settings.rs +++ b/crates/pgls_workspace/src/settings.rs @@ -18,7 +18,7 @@ use pgls_configuration::{ database::PartialDatabaseConfiguration, diagnostics::InvalidIgnorePattern, files::FilesConfiguration, - format::{FormatConfiguration, IndentStyle, KeywordCase}, + format::{CommaStyle, FormatConfiguration, IndentStyle, KeywordCase}, migrations::{MigrationsConfiguration, PartialMigrationsConfiguration}, pglinter::PglinterConfiguration, plpgsql_check::PlPgSqlCheckConfiguration, @@ -376,6 +376,7 @@ fn to_formatter_settings( keyword_case: conf.keyword_case, constant_case: conf.constant_case, type_case: conf.type_case, + comma_style: conf.comma_style, skip_fn_bodies: conf.skip_fn_bodies, ignored_files: to_matcher(working_directory.clone(), Some(&conf.ignore))?, included_files: to_matcher(working_directory.clone(), Some(&conf.include))?, @@ -578,6 +579,9 @@ pub struct FormatterSettings { /// Data type casing (text, varchar, int): upper or lower. Default: lower. pub type_case: KeywordCase, + /// Where a comma sits when a list breaks: trailing or leading. Default: trailing. + pub comma_style: CommaStyle, + /// If true, skip formatting of SQL function bodies (keep them verbatim). Default: false. pub skip_fn_bodies: bool, @@ -598,6 +602,7 @@ impl Default for FormatterSettings { keyword_case: KeywordCase::default(), constant_case: KeywordCase::default(), type_case: KeywordCase::default(), + comma_style: CommaStyle::default(), skip_fn_bodies: false, ignored_files: Matcher::empty(), included_files: Matcher::empty(), diff --git a/crates/pgls_workspace/src/workspace/server.rs b/crates/pgls_workspace/src/workspace/server.rs index ec20cf15c..0f891000e 100644 --- a/crates/pgls_workspace/src/workspace/server.rs +++ b/crates/pgls_workspace/src/workspace/server.rs @@ -937,6 +937,7 @@ impl Workspace for WorkspaceServer { keyword_case: settings.formatter.keyword_case.into(), constant_case: settings.formatter.constant_case.into(), type_case: settings.formatter.type_case.into(), + comma_style: settings.formatter.comma_style.into(), }; let mut diagnostics = Vec::new(); diff --git a/docs/features/formatting.md b/docs/features/formatting.md index e7700a0d6..56b2f1dc7 100644 --- a/docs/features/formatting.md +++ b/docs/features/formatting.md @@ -37,6 +37,7 @@ Configure formatting behavior in your `postgres-language-server.jsonc`: | `keywordCase` | `"lower"` | Casing for SQL keywords: `"upper"` or `"lower"` | | `constantCase` | `"lower"` | Casing for constants (NULL, TRUE, FALSE): `"upper"` or `"lower"` | | `typeCase` | `"lower"` | Casing for data types (text, int, varchar): `"upper"` or `"lower"` | +| `commaStyle` | `"trailing"` | Where a comma sits when a list breaks: `"trailing"` or `"leading"` | ### Example Output diff --git a/docs/schema.json b/docs/schema.json index d21ba9ee1..01360143b 100644 --- a/docs/schema.json +++ b/docs/schema.json @@ -342,6 +342,14 @@ }, "additionalProperties": false }, + "CommaStyle": { + "description": "Where a comma sits when a list breaks across lines.", + "type": "string", + "enum": [ + "trailing", + "leading" + ] + }, "DatabaseConfiguration": { "description": "The configuration of the database connection.", "type": "object", @@ -454,6 +462,17 @@ "description": "The configuration for SQL formatting.", "type": "object", "properties": { + "commaStyle": { + "description": "Where a comma sits when a list breaks: \"trailing\" or \"leading\". Default: \"trailing\".", + "anyOf": [ + { + "$ref": "#/definitions/CommaStyle" + }, + { + "type": "null" + } + ] + }, "constantCase": { "description": "Constant casing (NULL, TRUE, FALSE): \"upper\" or \"lower\". Default: \"lower\".", "anyOf": [ From dee0a73600f86a86fdc658ad8fdc9043076a6cb3 Mon Sep 17 00:00:00 2001 From: edjubert Date: Mon, 14 Sep 2026 11:33:03 +0200 Subject: [PATCH 4/6] feat(pretty-print): place list commas according to commaStyle --- .../pgls_pretty_print/src/nodes/node_list.rs | 23 +++++++++++++++---- .../tests/data/single/leading_commas.sql | 6 +++++ .../single/tests__leading_commas_80.snap | 10 ++++++++ crates/pgls_pretty_print/tests/tests.rs | 4 +++- 4 files changed, 37 insertions(+), 6 deletions(-) create mode 100644 crates/pgls_pretty_print/tests/data/single/leading_commas.sql create mode 100644 crates/pgls_pretty_print/tests/snapshots/single/tests__leading_commas_80.snap diff --git a/crates/pgls_pretty_print/src/nodes/node_list.rs b/crates/pgls_pretty_print/src/nodes/node_list.rs index 4986b3fb1..06c63fc38 100644 --- a/crates/pgls_pretty_print/src/nodes/node_list.rs +++ b/crates/pgls_pretty_print/src/nodes/node_list.rs @@ -1,7 +1,7 @@ use pgls_query::Node; -use crate::TokenKind; use crate::emitter::{EventEmitter, LineType}; +use crate::{CommaStyle, TokenKind}; /// Controls the spacing behavior after separators in list helpers #[derive(Clone, Copy, Default)] @@ -22,12 +22,25 @@ pub(super) fn emit_comma_separated_list_with_spacing( ) where F: Fn(&Node, &mut EventEmitter), { + let leading = matches!(e.config().comma_style, CommaStyle::Leading); + for (i, n) in nodes.iter().enumerate() { if i > 0 { - e.token(TokenKind::COMMA); - match spacing { - ListSeparatorSpacing::SoftOrSpace => e.line(LineType::SoftOrSpace), - ListSeparatorSpacing::Space => e.space(), + if leading { + // The break opportunity sits before the comma, so a broken list reads + // "\n, column" while a single line one still reads "a, b". + match spacing { + ListSeparatorSpacing::SoftOrSpace => e.line(LineType::Soft), + ListSeparatorSpacing::Space => {} + } + e.token(TokenKind::COMMA); + e.space(); + } else { + e.token(TokenKind::COMMA); + match spacing { + ListSeparatorSpacing::SoftOrSpace => e.line(LineType::SoftOrSpace), + ListSeparatorSpacing::Space => e.space(), + } } } render(n, e); diff --git a/crates/pgls_pretty_print/tests/data/single/leading_commas.sql b/crates/pgls_pretty_print/tests/data/single/leading_commas.sql new file mode 100644 index 000000000..4b3c91b47 --- /dev/null +++ b/crates/pgls_pretty_print/tests/data/single/leading_commas.sql @@ -0,0 +1,6 @@ +-- pgls-format: commaStyle=leading, indentStyle=tabs, indentSize=4, lineWidth=80 +SELECT + staging.buildings.identification_number, + staging.buildings.construction_year, + staging.buildings.address_fk +FROM staging.buildings; diff --git a/crates/pgls_pretty_print/tests/snapshots/single/tests__leading_commas_80.snap b/crates/pgls_pretty_print/tests/snapshots/single/tests__leading_commas_80.snap new file mode 100644 index 000000000..85b1df7ba --- /dev/null +++ b/crates/pgls_pretty_print/tests/snapshots/single/tests__leading_commas_80.snap @@ -0,0 +1,10 @@ +--- +source: crates/pgls_pretty_print/tests/tests.rs +input_file: crates/pgls_pretty_print/tests/data/single/leading_commas.sql +--- +select + staging.buildings.identification_number + , staging.buildings.construction_year + , staging.buildings.address_fk +from + staging.buildings; diff --git a/crates/pgls_pretty_print/tests/tests.rs b/crates/pgls_pretty_print/tests/tests.rs index 8fd18c94d..aa571c39d 100644 --- a/crates/pgls_pretty_print/tests/tests.rs +++ b/crates/pgls_pretty_print/tests/tests.rs @@ -3,7 +3,7 @@ use dir_test::{Fixture, dir_test}; use insta::{assert_snapshot, with_settings}; use pgls_pretty_print::{ - FormatConfig, + CommaStyle, FormatConfig, emitter::EventEmitter, nodes::emit_node_enum, normalize::normalize_ast, @@ -81,6 +81,8 @@ fn parse_fixture(content: &str) -> (FormatConfig, Option, String) { ("constantCase", "lower") => config.constant_case = KeywordCase::Lower, ("typeCase", "upper") => config.type_case = KeywordCase::Upper, ("typeCase", "lower") => config.type_case = KeywordCase::Lower, + ("commaStyle", "leading") => config.comma_style = CommaStyle::Leading, + ("commaStyle", "trailing") => config.comma_style = CommaStyle::Trailing, (key, value) => panic!("unknown pgls-format entry: {key}={value}"), } } From 79b1b89075830a3c95e9ded96b417c9ab32eae8d Mon Sep 17 00:00:00 2001 From: edjubert Date: Mon, 14 Sep 2026 11:35:04 +0200 Subject: [PATCH 5/6] feat(format): add the logicalOperatorPlacement option --- crates/pgls_configuration/src/format.rs | 38 +++++++++++++++++++ crates/pgls_pretty_print/src/lib.rs | 14 +++++++ crates/pgls_workspace/src/settings.rs | 7 +++- crates/pgls_workspace/src/workspace/server.rs | 1 + docs/features/formatting.md | 1 + docs/schema.json | 19 ++++++++++ 6 files changed, 79 insertions(+), 1 deletion(-) diff --git a/crates/pgls_configuration/src/format.rs b/crates/pgls_configuration/src/format.rs index 106511ed7..12a207d8a 100644 --- a/crates/pgls_configuration/src/format.rs +++ b/crates/pgls_configuration/src/format.rs @@ -101,6 +101,39 @@ impl From for pgls_pretty_print::CommaStyle { } } +/// Where a boolean operator sits when a condition breaks across lines. +#[derive(Clone, Copy, Debug, Default, Deserialize, Eq, Merge, PartialEq, Serialize)] +#[cfg_attr(feature = "schema", derive(schemars::JsonSchema))] +#[serde(rename_all = "lowercase")] +pub enum LogicalOperatorPlacement { + #[default] + Trailing, + Leading, +} + +impl FromStr for LogicalOperatorPlacement { + type Err = &'static str; + + fn from_str(s: &str) -> Result { + match s { + "trailing" => Ok(Self::Trailing), + "leading" => Ok(Self::Leading), + _ => Err( + "Value not supported for LogicalOperatorPlacement. Use 'trailing' or 'leading'.", + ), + } + } +} + +impl From for pgls_pretty_print::LogicalOperatorPlacement { + fn from(placement: LogicalOperatorPlacement) -> Self { + match placement { + LogicalOperatorPlacement::Trailing => Self::Trailing, + LogicalOperatorPlacement::Leading => Self::Leading, + } + } +} + /// The configuration for SQL formatting. #[derive(Clone, Debug, Deserialize, Eq, Partial, PartialEq, Serialize)] #[partial(derive(Bpaf, Clone, Eq, PartialEq, Merge))] @@ -131,6 +164,10 @@ pub struct FormatConfiguration { /// Where a comma sits when a list breaks: "trailing" or "leading". Default: "trailing". #[partial(bpaf(long("comma-style")))] pub comma_style: CommaStyle, + /// Where a boolean operator sits when a condition breaks: "trailing" or "leading". + /// Default: "trailing". + #[partial(bpaf(long("logical-operator-placement")))] + pub logical_operator_placement: LogicalOperatorPlacement, /// If `true`, skip formatting of SQL function bodies (keep them verbatim). Default: `false`. #[partial(bpaf(long("skip-fn-bodies")))] pub skip_fn_bodies: bool, @@ -153,6 +190,7 @@ impl Default for FormatConfiguration { constant_case: KeywordCase::default(), type_case: KeywordCase::default(), comma_style: CommaStyle::default(), + logical_operator_placement: LogicalOperatorPlacement::default(), skip_fn_bodies: false, ignore: Default::default(), include: Default::default(), diff --git a/crates/pgls_pretty_print/src/lib.rs b/crates/pgls_pretty_print/src/lib.rs index de7b00119..f66bd5b8c 100644 --- a/crates/pgls_pretty_print/src/lib.rs +++ b/crates/pgls_pretty_print/src/lib.rs @@ -42,6 +42,16 @@ pub enum CommaStyle { Leading, } +/// Where a boolean operator sits when a condition breaks across lines. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub enum LogicalOperatorPlacement { + /// `a = 1 AND` at the end of the line. + #[default] + Trailing, + /// `AND a = 1` at the start of the continuation line. + Leading, +} + #[derive(Debug, Clone)] pub struct FormatConfig { /// Maximum line width before breaking. Default: 100. @@ -58,6 +68,8 @@ pub struct FormatConfig { pub type_case: KeywordCase, /// Where a comma sits when a list breaks. Default: Trailing. pub comma_style: CommaStyle, + /// Where a boolean operator sits when a condition breaks. Default: Trailing. + pub logical_operator_placement: LogicalOperatorPlacement, } impl Default for FormatConfig { @@ -70,6 +82,7 @@ impl Default for FormatConfig { constant_case: KeywordCase::default(), type_case: KeywordCase::default(), comma_style: CommaStyle::default(), + logical_operator_placement: LogicalOperatorPlacement::default(), } } } @@ -84,6 +97,7 @@ impl From for RenderConfig { constant_case, type_case, comma_style: _, + logical_operator_placement: _, } = config; Self { diff --git a/crates/pgls_workspace/src/settings.rs b/crates/pgls_workspace/src/settings.rs index 40ffae218..95e2662cc 100644 --- a/crates/pgls_workspace/src/settings.rs +++ b/crates/pgls_workspace/src/settings.rs @@ -18,7 +18,7 @@ use pgls_configuration::{ database::PartialDatabaseConfiguration, diagnostics::InvalidIgnorePattern, files::FilesConfiguration, - format::{CommaStyle, FormatConfiguration, IndentStyle, KeywordCase}, + format::{CommaStyle, FormatConfiguration, IndentStyle, KeywordCase, LogicalOperatorPlacement}, migrations::{MigrationsConfiguration, PartialMigrationsConfiguration}, pglinter::PglinterConfiguration, plpgsql_check::PlPgSqlCheckConfiguration, @@ -377,6 +377,7 @@ fn to_formatter_settings( constant_case: conf.constant_case, type_case: conf.type_case, comma_style: conf.comma_style, + logical_operator_placement: conf.logical_operator_placement, skip_fn_bodies: conf.skip_fn_bodies, ignored_files: to_matcher(working_directory.clone(), Some(&conf.ignore))?, included_files: to_matcher(working_directory.clone(), Some(&conf.include))?, @@ -582,6 +583,9 @@ pub struct FormatterSettings { /// Where a comma sits when a list breaks: trailing or leading. Default: trailing. pub comma_style: CommaStyle, + /// Where a boolean operator sits when a condition breaks: trailing or leading. Default: trailing. + pub logical_operator_placement: LogicalOperatorPlacement, + /// If true, skip formatting of SQL function bodies (keep them verbatim). Default: false. pub skip_fn_bodies: bool, @@ -603,6 +607,7 @@ impl Default for FormatterSettings { constant_case: KeywordCase::default(), type_case: KeywordCase::default(), comma_style: CommaStyle::default(), + logical_operator_placement: LogicalOperatorPlacement::default(), skip_fn_bodies: false, ignored_files: Matcher::empty(), included_files: Matcher::empty(), diff --git a/crates/pgls_workspace/src/workspace/server.rs b/crates/pgls_workspace/src/workspace/server.rs index 0f891000e..03a0b5799 100644 --- a/crates/pgls_workspace/src/workspace/server.rs +++ b/crates/pgls_workspace/src/workspace/server.rs @@ -938,6 +938,7 @@ impl Workspace for WorkspaceServer { constant_case: settings.formatter.constant_case.into(), type_case: settings.formatter.type_case.into(), comma_style: settings.formatter.comma_style.into(), + logical_operator_placement: settings.formatter.logical_operator_placement.into(), }; let mut diagnostics = Vec::new(); diff --git a/docs/features/formatting.md b/docs/features/formatting.md index 56b2f1dc7..837603f7b 100644 --- a/docs/features/formatting.md +++ b/docs/features/formatting.md @@ -38,6 +38,7 @@ Configure formatting behavior in your `postgres-language-server.jsonc`: | `constantCase` | `"lower"` | Casing for constants (NULL, TRUE, FALSE): `"upper"` or `"lower"` | | `typeCase` | `"lower"` | Casing for data types (text, int, varchar): `"upper"` or `"lower"` | | `commaStyle` | `"trailing"` | Where a comma sits when a list breaks: `"trailing"` or `"leading"` | +| `logicalOperatorPlacement` | `"trailing"` | Where `AND` and `OR` sit when a condition breaks: `"trailing"` or `"leading"` | ### Example Output diff --git a/docs/schema.json b/docs/schema.json index 01360143b..b9d0d3487 100644 --- a/docs/schema.json +++ b/docs/schema.json @@ -553,6 +553,17 @@ "format": "uint16", "minimum": 0.0 }, + "logicalOperatorPlacement": { + "description": "Where a boolean operator sits when a condition breaks: \"trailing\" or \"leading\". Default: \"trailing\".", + "anyOf": [ + { + "$ref": "#/definitions/LogicalOperatorPlacement" + }, + { + "type": "null" + } + ] + }, "skipFnBodies": { "description": "If `true`, skip formatting of SQL function bodies (keep them verbatim). Default: `false`.", "type": [ @@ -688,6 +699,14 @@ }, "additionalProperties": false }, + "LogicalOperatorPlacement": { + "description": "Where a boolean operator sits when a condition breaks across lines.", + "type": "string", + "enum": [ + "trailing", + "leading" + ] + }, "MigrationsConfiguration": { "description": "The configuration of the filesystem", "type": "object", From 040e7700ec01c70b49e8bac9353483d6f1bbd313 Mon Sep 17 00:00:00 2001 From: edjubert Date: Mon, 14 Sep 2026 11:35:54 +0200 Subject: [PATCH 6/6] feat(pretty-print): place boolean operators according to logicalOperatorPlacement --- .../pgls_pretty_print/src/nodes/bool_expr.rs | 20 +++++++++++++++---- .../data/single/leading_boolean_operators.sql | 6 ++++++ .../tests__leading_boolean_operators_80.snap | 13 ++++++++++++ crates/pgls_pretty_print/tests/tests.rs | 8 +++++++- 4 files changed, 42 insertions(+), 5 deletions(-) create mode 100644 crates/pgls_pretty_print/tests/data/single/leading_boolean_operators.sql create mode 100644 crates/pgls_pretty_print/tests/snapshots/single/tests__leading_boolean_operators_80.snap diff --git a/crates/pgls_pretty_print/src/nodes/bool_expr.rs b/crates/pgls_pretty_print/src/nodes/bool_expr.rs index ad57c8108..023067e13 100644 --- a/crates/pgls_pretty_print/src/nodes/bool_expr.rs +++ b/crates/pgls_pretty_print/src/nodes/bool_expr.rs @@ -2,7 +2,7 @@ use pgls_query::protobuf::{BoolExpr, BoolExprType}; use pgls_query::{Node, NodeEnum}; use crate::{ - TokenKind, + LogicalOperatorPlacement, TokenKind, emitter::{EventEmitter, GroupKind, LineType}, }; @@ -21,12 +21,24 @@ pub(super) fn emit_bool_expr(e: &mut EventEmitter, n: &BoolExpr) { fn emit_variadic_bool_expr(e: &mut EventEmitter, n: &BoolExpr, keyword: TokenKind) { let parent_prec = bool_precedence(n.boolop()); + let leading = matches!( + e.config().logical_operator_placement, + LogicalOperatorPlacement::Leading + ); for (idx, arg) in n.args.iter().enumerate() { if idx > 0 { - e.space(); - e.token(keyword.clone()); - e.line(LineType::SoftOrSpace); + if leading { + // The break opportunity sits before the keyword, so a broken condition reads + // "\n\tAND b = 2" while a single line one still reads "a = 1 AND b = 2". + e.line(LineType::SoftOrSpace); + e.token(keyword.clone()); + e.space(); + } else { + e.space(); + e.token(keyword.clone()); + e.line(LineType::SoftOrSpace); + } } emit_bool_operand(e, arg, parent_prec); diff --git a/crates/pgls_pretty_print/tests/data/single/leading_boolean_operators.sql b/crates/pgls_pretty_print/tests/data/single/leading_boolean_operators.sql new file mode 100644 index 000000000..e9f92bd02 --- /dev/null +++ b/crates/pgls_pretty_print/tests/data/single/leading_boolean_operators.sql @@ -0,0 +1,6 @@ +-- pgls-format: logicalOperatorPlacement=leading, indentStyle=tabs, indentSize=4, lineWidth=80 +SELECT staging.buildings.id +FROM staging.buildings +WHERE staging.buildings.construction_year > 1950 + AND staging.buildings.address_fk IS NOT NULL + AND staging.buildings.identification_number IS NOT NULL; diff --git a/crates/pgls_pretty_print/tests/snapshots/single/tests__leading_boolean_operators_80.snap b/crates/pgls_pretty_print/tests/snapshots/single/tests__leading_boolean_operators_80.snap new file mode 100644 index 000000000..1faaa3601 --- /dev/null +++ b/crates/pgls_pretty_print/tests/snapshots/single/tests__leading_boolean_operators_80.snap @@ -0,0 +1,13 @@ +--- +source: crates/pgls_pretty_print/tests/tests.rs +input_file: crates/pgls_pretty_print/tests/data/single/leading_boolean_operators.sql +--- +select + staging.buildings.id +from + staging.buildings +where + staging.buildings.construction_year > + 1950 + and staging.buildings.address_fk is not null + and staging.buildings.identification_number is not null; diff --git a/crates/pgls_pretty_print/tests/tests.rs b/crates/pgls_pretty_print/tests/tests.rs index aa571c39d..7f7447dc1 100644 --- a/crates/pgls_pretty_print/tests/tests.rs +++ b/crates/pgls_pretty_print/tests/tests.rs @@ -3,7 +3,7 @@ use dir_test::{Fixture, dir_test}; use insta::{assert_snapshot, with_settings}; use pgls_pretty_print::{ - CommaStyle, FormatConfig, + CommaStyle, FormatConfig, LogicalOperatorPlacement, emitter::EventEmitter, nodes::emit_node_enum, normalize::normalize_ast, @@ -83,6 +83,12 @@ fn parse_fixture(content: &str) -> (FormatConfig, Option, String) { ("typeCase", "lower") => config.type_case = KeywordCase::Lower, ("commaStyle", "leading") => config.comma_style = CommaStyle::Leading, ("commaStyle", "trailing") => config.comma_style = CommaStyle::Trailing, + ("logicalOperatorPlacement", "leading") => { + config.logical_operator_placement = LogicalOperatorPlacement::Leading; + } + ("logicalOperatorPlacement", "trailing") => { + config.logical_operator_placement = LogicalOperatorPlacement::Trailing; + } (key, value) => panic!("unknown pgls-format entry: {key}={value}"), } }