From 894d2be3b4f78fb72c26b9dbcdf8cfc754a1fec9 Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:42:27 +0000 Subject: [PATCH 1/3] fix: preserve whitespace around `..` in `prqlc highlight` --- prqlc/prqlc/src/cli/highlight.rs | 71 +++++++++++++++++++++++++++++--- 1 file changed, 66 insertions(+), 5 deletions(-) diff --git a/prqlc/prqlc/src/cli/highlight.rs b/prqlc/prqlc/src/cli/highlight.rs index 0ddad1dc8984..18050bbeb3ba 100644 --- a/prqlc/prqlc/src/cli/highlight.rs +++ b/prqlc/prqlc/src/cli/highlight.rs @@ -11,14 +11,46 @@ pub(crate) fn highlight(tokens: &Tokens) -> String { for token in &tokens.0 { let diff = token.span.start - last; - last = token.span.end; output.push_str(&" ".repeat(diff)); - output.push_str(&highlight_token_kind(&token.kind)); + // A range is the one token whose span covers more than the token's own + // text, so it's the one kind that needs the span to render. + match &token.kind { + TokenKind::Range { + bind_left, + bind_right, + } => output.push_str(&highlight_range(*bind_left, *bind_right, token.span.len())), + kind => output.push_str(&highlight_token_kind(kind)), + } + last = token.span.end; } output } +/// Render a range token, including the whitespace either side of the `..`. +/// +/// The lexer folds that whitespace into the range token's own span, and it's +/// what decides whether the range binds: `take 1..5` compiles, while +/// `take 1 .. 5` is an error. Writing back a bare `..` would print a different +/// program than the one being highlighted. +/// +/// `width` is the span's width, which the padding fills so that everything +/// later on the line keeps its column. +fn highlight_range(bind_left: bool, bind_right: bool, width: usize) -> String { + let min_left = usize::from(!bind_left); + let min_right = usize::from(!bind_right); + // Whatever the span holds beyond the minimum is whitespace on an unbound + // side. When both sides are unbound the split isn't recoverable from the + // token, so the surplus all goes on the left. + let surplus = width.saturating_sub("..".len() + min_left + min_right); + let (left, right) = if bind_left { + (0, min_right + surplus) + } else { + (min_left + surplus, min_right) + }; + format!("{}..{}", " ".repeat(left), " ".repeat(right)) +} + fn highlight_token_kind(token: &TokenKind) -> String { // LineWrap is recursive with TokenKind, so we needed to split this function // out from the one above (otherwise would have it as a single func) @@ -42,10 +74,12 @@ fn highlight_token_kind(token: &TokenKind) -> String { _ => literal.to_string(), }), TokenKind::Param(param) => output.push_str(¶m.purple().to_string()), + // Only reachable through the `LineWrap` recursion below, which carries + // no spans; the padding falls back to a single space per unbound side. TokenKind::Range { - bind_left: _, - bind_right: _, - } => output.push_str(".."), + bind_left, + bind_right, + } => output.push_str(&highlight_range(*bind_left, *bind_right, 0)), TokenKind::Interpolation(_, _) => output.push_str(&format!("{}", token.yellow())), TokenKind::Control(char) => output.push(*char), TokenKind::ArrowThin @@ -139,6 +173,33 @@ mod tests { "#); } + /// The whitespace around `..` is part of the range token's span, and it + /// decides whether the range binds — `take 1..5` compiles, `take 1 .. 5` + /// doesn't — so highlighting has to preserve it rather than print a bare + /// `..`. + #[test] + fn highlight_range_whitespace() { + assert_cmd_snapshot!(prqlc_command().args(["experimental", "highlight"]).pass_stdin(r#"from x +take 1..5 +take 1 .. 5 +take 1 ..5 +take 1.. 5 +take 1 .. 5 +"#), @r" + success: true + exit_code: 0 + ----- stdout ----- + from x + take 1..5 + take 1 .. 5 + take 1 ..5 + take 1.. 5 + take 1 .. 5 + + ----- stderr ----- + "); + } + // TODO: import from existing location, need to adjust visibility fn prqlc_command() -> Command { let mut cmd = Command::new(get_cargo_bin("prqlc")); From 1b40e85625c9f31f64aad4e78c6253cf73ddaafd Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Tue, 25 Aug 2026 06:57:00 +0000 Subject: [PATCH 2/3] fix: split surplus range whitespace evenly Applies the review suggestions on #6232: split the surplus evenly when both sides are unbound, so symmetric spacing round-trips exactly; and correct the two comments about `LineWrap` spans and the unreachable `Range` arm. --- prqlc/prqlc/src/cli/highlight.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/prqlc/prqlc/src/cli/highlight.rs b/prqlc/prqlc/src/cli/highlight.rs index 18050bbeb3ba..2cb89c32f945 100644 --- a/prqlc/prqlc/src/cli/highlight.rs +++ b/prqlc/prqlc/src/cli/highlight.rs @@ -12,8 +12,10 @@ pub(crate) fn highlight(tokens: &Tokens) -> String { for token in &tokens.0 { let diff = token.span.start - last; output.push_str(&" ".repeat(diff)); - // A range is the one token whose span covers more than the token's own - // text, so it's the one kind that needs the span to render. + // A range's span covers the whitespace either side of the `..`, and + // that whitespace decides whether the range binds — so it's the one + // kind whose rendering needs the span. (`LineWrap` spans are wider + // than their text too, but the whitespace they swallow is inert.) match &token.kind { TokenKind::Range { bind_left, @@ -41,12 +43,15 @@ fn highlight_range(bind_left: bool, bind_right: bool, width: usize) -> String { let min_right = usize::from(!bind_right); // Whatever the span holds beyond the minimum is whitespace on an unbound // side. When both sides are unbound the split isn't recoverable from the - // token, so the surplus all goes on the left. + // token, so it's split evenly — which round-trips symmetric spacing like + // `1 .. 5` exactly, and only re-splits genuinely asymmetric spacing. let surplus = width.saturating_sub("..".len() + min_left + min_right); let (left, right) = if bind_left { (0, min_right + surplus) + } else if bind_right { + (min_left + surplus, 0) } else { - (min_left + surplus, min_right) + (min_left + surplus.div_ceil(2), min_right + surplus / 2) }; format!("{}..{}", " ".repeat(left), " ".repeat(right)) } @@ -74,8 +79,9 @@ fn highlight_token_kind(token: &TokenKind) -> String { _ => literal.to_string(), }), TokenKind::Param(param) => output.push_str(¶m.purple().to_string()), - // Only reachable through the `LineWrap` recursion below, which carries - // no spans; the padding falls back to a single space per unbound side. + // Unreachable: `highlight` intercepts `Range` before dispatching here, + // and a `LineWrap` only ever carries comments. Kept for exhaustiveness; + // width `0` renders the minimum padding. TokenKind::Range { bind_left, bind_right, @@ -194,7 +200,7 @@ take 1 .. 5 take 1 .. 5 take 1 ..5 take 1.. 5 - take 1 .. 5 + take 1 .. 5 ----- stderr ----- "); From 83914ca43a2815d79458da473eb3c7c62f21787a Mon Sep 17 00:00:00 2001 From: prql-bot <107324867+prql-bot@users.noreply.github.com> Date: Tue, 25 Aug 2026 07:07:08 +0000 Subject: [PATCH 3/3] docs: add changelog entry and pin the re-split range case --- CHANGELOG.md | 6 ++++++ prqlc/prqlc/src/cli/highlight.rs | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0aae6781cb9f..2765a8cb7eee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,12 @@ `invalid length 0, expected an ident with at least one part`. (@prql-bot, #6223) +- `prqlc experimental highlight` now preserves the whitespace around `..`. It + previously printed a bare `..` for every range token, which could turn the + highlighted output into a different program than the input — `take 1 .. 5` is + a compile error, but it was rendered as `take 1..5`, which compiles to + `LIMIT 5`. (@prql-bot, #6232) + **Documentation**: **Web**: diff --git a/prqlc/prqlc/src/cli/highlight.rs b/prqlc/prqlc/src/cli/highlight.rs index 2cb89c32f945..cacca9c50b24 100644 --- a/prqlc/prqlc/src/cli/highlight.rs +++ b/prqlc/prqlc/src/cli/highlight.rs @@ -183,6 +183,10 @@ mod tests { /// decides whether the range binds — `take 1..5` compiles, `take 1 .. 5` /// doesn't — so highlighting has to preserve it rather than print a bare /// `..`. + /// + /// The final line is the one lossy case: when both sides are unbound the + /// span records the total width but not how it splits, so asymmetric + /// spacing is re-centred. The width and the binding still round-trip. #[test] fn highlight_range_whitespace() { assert_cmd_snapshot!(prqlc_command().args(["experimental", "highlight"]).pass_stdin(r#"from x @@ -191,6 +195,7 @@ take 1 .. 5 take 1 ..5 take 1.. 5 take 1 .. 5 +take 1 .. 5 "#), @r" success: true exit_code: 0 @@ -201,6 +206,7 @@ take 1 .. 5 take 1 ..5 take 1.. 5 take 1 .. 5 + take 1 .. 5 ----- stderr ----- ");