fix(dashboard): preserve blank lines and improve Rhai parser errors
Two follow-ups on the Rhai formatter shipped in 0.5.1.
* Formatter no longer collapses user-intent blank lines between
statements. The lexer now records a side-channel list of offsets
where the source contained two-or-more consecutive newlines; the
formatter consults it and emits a single blank in the same spot
(rustfmt's `blank_lines_upper_bound = 1` policy applied strictly —
the prior forced blank between top-level `fn` decls is dropped, so
the formatter never *adds* a blank the user didn't write).
* Parse errors now read like Rhai's own diagnostics. `expect()` takes
an optional `role` hint and each call site supplies a domain phrase
(`name of a variable`, `function name in function declaration`,
`'{' to begin a block`, `name of a property`, …). End-of-input is
reported as `script is incomplete`. The dashboard banner renders
`Parse error: {message} (line L, position C)` with 1-based
coordinates, matching Rhai's format exactly.
The FormatError payload also keeps the byte `offset` so callers that
want to drive the editor cursor (CodeMirror works in offsets) still
have it.
Also folds the workspace Cargo.lock version bumps for 0.5.1 — the
lock-file rewrite that should have travelled with the prior commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
16
Cargo.lock
generated
16
Cargo.lock
generated
@@ -1273,7 +1273,7 @@ checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220"
|
||||
|
||||
[[package]]
|
||||
name = "picloud"
|
||||
version = "0.5.0"
|
||||
version = "0.5.1"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"async-trait",
|
||||
@@ -1297,7 +1297,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "picloud-executor"
|
||||
version = "0.5.0"
|
||||
version = "0.5.1"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"picloud-executor-core",
|
||||
@@ -1309,7 +1309,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "picloud-executor-core"
|
||||
version = "0.5.0"
|
||||
version = "0.5.1"
|
||||
dependencies = [
|
||||
"chrono",
|
||||
"picloud-shared",
|
||||
@@ -1323,7 +1323,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "picloud-manager"
|
||||
version = "0.5.0"
|
||||
version = "0.5.1"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"picloud-manager-core",
|
||||
@@ -1335,7 +1335,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "picloud-manager-core"
|
||||
version = "0.5.0"
|
||||
version = "0.5.1"
|
||||
dependencies = [
|
||||
"async-trait",
|
||||
"axum",
|
||||
@@ -1353,7 +1353,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "picloud-orchestrator"
|
||||
version = "0.5.0"
|
||||
version = "0.5.1"
|
||||
dependencies = [
|
||||
"anyhow",
|
||||
"picloud-orchestrator-core",
|
||||
@@ -1365,7 +1365,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "picloud-orchestrator-core"
|
||||
version = "0.5.0"
|
||||
version = "0.5.1"
|
||||
dependencies = [
|
||||
"async-trait",
|
||||
"axum",
|
||||
@@ -1384,7 +1384,7 @@ dependencies = [
|
||||
|
||||
[[package]]
|
||||
name = "picloud-shared"
|
||||
version = "0.5.0"
|
||||
version = "0.5.1"
|
||||
dependencies = [
|
||||
"async-trait",
|
||||
"chrono",
|
||||
|
||||
@@ -272,4 +272,8 @@ export interface ParseResult {
|
||||
program: BlockExpr;
|
||||
errors: ParseError[];
|
||||
comments: Comment[];
|
||||
// Offsets at which the source contained a blank line (a whitespace
|
||||
// run with two or more newlines). One entry per blank run; the
|
||||
// formatter consults these to preserve user-intent vertical grouping.
|
||||
blankLines: number[];
|
||||
}
|
||||
|
||||
@@ -22,11 +22,10 @@ describe('format — basic shape', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('separates top-level fn decls with a blank line', () => {
|
||||
it('does not insert a blank between fn decls the user did not separate', () => {
|
||||
// Strict preserve-only policy: no source blank => no emitted blank.
|
||||
const out = formatted('fn a(){1}fn b(){2}');
|
||||
expect(out).toBe(
|
||||
'fn a() {\n\t1\n}\n\nfn b() {\n\t2\n}\n'
|
||||
);
|
||||
expect(out).toBe('fn a() {\n\t1\n}\nfn b() {\n\t2\n}\n');
|
||||
});
|
||||
|
||||
it('renders if / else if / else with blocks', () => {
|
||||
@@ -87,16 +86,48 @@ describe('format — reflow', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('format — blank-line preservation', () => {
|
||||
it('preserves a single blank line between statements', () => {
|
||||
const src = 'let a = 1;\n\nlet b = 2;';
|
||||
expect(formatted(src)).toBe('let a = 1;\n\nlet b = 2;\n');
|
||||
});
|
||||
|
||||
it('collapses multiple blank lines to a single one', () => {
|
||||
const src = 'let a = 1;\n\n\n\nlet b = 2;';
|
||||
expect(formatted(src)).toBe('let a = 1;\n\nlet b = 2;\n');
|
||||
});
|
||||
|
||||
it('preserves blanks inside block bodies', () => {
|
||||
const src = 'fn process() {\n\tlet a = 1;\n\n\tlet b = 2;\n}';
|
||||
expect(formatted(src)).toBe('fn process() {\n\tlet a = 1;\n\n\tlet b = 2;\n}\n');
|
||||
});
|
||||
|
||||
it('does not invent blanks between adjacent statements', () => {
|
||||
expect(formatted('let a=1;let b=2;')).toBe('let a = 1;\nlet b = 2;\n');
|
||||
});
|
||||
});
|
||||
|
||||
describe('format — parse failures', () => {
|
||||
it('returns ok=false with the first parse error', () => {
|
||||
const r = format('let = ;');
|
||||
it('returns ok=false with a Rhai-flavored message and 1-based line/column', () => {
|
||||
// Pattern from the user complaint: `let;` should surface as
|
||||
// "Expecting name of a variable" at line/column.
|
||||
const r = format('let msg = ctx.request.params.name;\nlet;\n');
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) {
|
||||
expect(typeof r.error.message).toBe('string');
|
||||
expect(r.error.message).toBe('Expecting name of a variable');
|
||||
expect(r.error.line).toBe(2);
|
||||
expect(r.error.column).toBe(4);
|
||||
expect(r.error.offset).toBeGreaterThanOrEqual(0);
|
||||
}
|
||||
});
|
||||
|
||||
it('reports script-incomplete on truncated input', () => {
|
||||
// `fn` alone — the parser expects a function name and hits EOF.
|
||||
const r = format('fn');
|
||||
expect(r.ok).toBe(false);
|
||||
if (!r.ok) expect(r.error.message).toMatch(/script is incomplete/i);
|
||||
});
|
||||
|
||||
it('does not partially rewrite when parsing fails', () => {
|
||||
const r = format('let x = 1; this is garbage');
|
||||
expect(r.ok).toBe(false);
|
||||
|
||||
@@ -15,9 +15,14 @@
|
||||
// the indent of the statement it precedes — same-line inline
|
||||
// positioning is intentionally NOT recovered; the goal is "verbatim
|
||||
// text", not "byte-exact placement".
|
||||
// * Blank lines between statements are preserved when the user wrote
|
||||
// them; multiples collapse to one. The formatter never *adds* blank
|
||||
// lines the user didn't write (rustfmt's default policy applied
|
||||
// strictly — no forced separation between top-level fn decls).
|
||||
// * Block bodies always use multi-line braces. `{}` for empty.
|
||||
// * If parse errors are reported by the parser, the formatter refuses
|
||||
// to emit anything and returns the first error.
|
||||
// to emit anything and returns the first error with line / column
|
||||
// coordinates (1-based, matching Rhai's own diagnostic format).
|
||||
|
||||
import type {
|
||||
BlockExpr,
|
||||
@@ -36,21 +41,47 @@ const PRINT_WIDTH = 100;
|
||||
|
||||
export type FormatResult =
|
||||
| { ok: true; text: string }
|
||||
| { ok: false; error: { message: string; offset: number } };
|
||||
| { ok: false; error: FormatError };
|
||||
|
||||
export interface FormatError {
|
||||
message: string;
|
||||
// 1-based line and column, matching Rhai's own diagnostic format.
|
||||
line: number;
|
||||
column: number;
|
||||
// Byte offset retained for callers that want to jump the editor
|
||||
// cursor (CodeMirror works in offsets, not line/col).
|
||||
offset: number;
|
||||
}
|
||||
|
||||
export function format(source: string): FormatResult {
|
||||
const result = parse(source);
|
||||
if (result.errors.length > 0) {
|
||||
const first = result.errors[0];
|
||||
return { ok: false, error: errorPayload(first) };
|
||||
return { ok: false, error: errorPayload(source, result.errors[0]) };
|
||||
}
|
||||
const p = new Printer(result);
|
||||
p.printProgram();
|
||||
return { ok: true, text: p.finish() };
|
||||
}
|
||||
|
||||
function errorPayload(e: ParseError): { message: string; offset: number } {
|
||||
return { message: e.message, offset: e.start };
|
||||
function errorPayload(source: string, e: ParseError): FormatError {
|
||||
const { line, column } = lineColAt(source, e.start);
|
||||
return { message: e.message, line, column, offset: e.start };
|
||||
}
|
||||
|
||||
// Convert a byte offset into 1-based (line, column). Used for rendering
|
||||
// parser errors in a way that matches Rhai's own diagnostic format
|
||||
// (e.g. "Expecting name of a variable (line 2, position 4)").
|
||||
function lineColAt(source: string, offset: number): { line: number; column: number } {
|
||||
let line = 1;
|
||||
let lineStart = 0;
|
||||
const limit = Math.min(offset, source.length);
|
||||
for (let i = 0; i < limit; i++) {
|
||||
if (source.charCodeAt(i) === 10) {
|
||||
line++;
|
||||
lineStart = i + 1;
|
||||
}
|
||||
}
|
||||
return { line, column: limit - lineStart + 1 };
|
||||
}
|
||||
|
||||
class Printer {
|
||||
@@ -125,19 +156,27 @@ class Printer {
|
||||
|
||||
printProgram(): void {
|
||||
const stmts = this.result.program.stmts;
|
||||
let prevWasFn = false;
|
||||
for (let i = 0; i < stmts.length; i++) {
|
||||
const stmt = stmts[i];
|
||||
if (i > 0) {
|
||||
if (prevWasFn || stmt.kind === 'FnDecl') this.blankLine();
|
||||
if (this.hadBlankBetween(stmts[i - 1].end, stmt.start)) this.blankLine();
|
||||
else this.newline();
|
||||
}
|
||||
this.drainCommentsBefore(stmt.start);
|
||||
this.printStmt(stmt);
|
||||
prevWasFn = stmt.kind === 'FnDecl';
|
||||
}
|
||||
}
|
||||
|
||||
// "Did the user leave a blank line in this gap?" Consulted between
|
||||
// every pair of emitted statements to decide whether to keep the
|
||||
// vertical separator the source originally had.
|
||||
private hadBlankBetween(prevEnd: number, currStart: number): boolean {
|
||||
for (const offset of this.result.blankLines) {
|
||||
if (offset >= prevEnd && offset < currStart) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------- statements
|
||||
|
||||
private printStmt(stmt: Stmt): void {
|
||||
@@ -231,7 +270,11 @@ class Printer {
|
||||
this.emit('{');
|
||||
this.indent++;
|
||||
for (let i = 0; i < block.stmts.length; i++) {
|
||||
if (i > 0 && this.hadBlankBetween(block.stmts[i - 1].end, block.stmts[i].start)) {
|
||||
this.blankLine();
|
||||
} else {
|
||||
this.newline();
|
||||
}
|
||||
this.drainCommentsBefore(block.stmts[i].start);
|
||||
this.printStmt(block.stmts[i]);
|
||||
}
|
||||
|
||||
@@ -5,7 +5,7 @@ export { parse } from './parser';
|
||||
export { tokenize, KEYWORDS } from './lexer';
|
||||
export { buildSymbolTable, renderFnSignature } from './symbols';
|
||||
export { format } from './format';
|
||||
export type { FormatResult } from './format';
|
||||
export type { FormatError, FormatResult } from './format';
|
||||
export type { Decl, DeclKind, Scope, SymbolTable, Usage } from './symbols';
|
||||
export type {
|
||||
BlockExpr,
|
||||
|
||||
@@ -88,20 +88,38 @@ const PUNCTS = new Set(['(', ')', '{', '}', '[', ']', ';', ',', '.', ':', '#']);
|
||||
export interface LexResult {
|
||||
tokens: Token[];
|
||||
comments: Comment[];
|
||||
// Offsets at which the source contained at least one blank line (a
|
||||
// run of whitespace with two or more newlines). One entry per blank
|
||||
// run, pointing at the second-newline position. Used by the formatter
|
||||
// to preserve user-intent vertical grouping.
|
||||
blankLines: number[];
|
||||
}
|
||||
|
||||
export function tokenize(source: string): LexResult {
|
||||
const tokens: Token[] = [];
|
||||
const comments: Comment[] = [];
|
||||
const blankLines: number[] = [];
|
||||
let i = 0;
|
||||
const n = source.length;
|
||||
|
||||
while (i < n) {
|
||||
const ch = source[i];
|
||||
|
||||
// Whitespace
|
||||
// Whitespace — coalesce runs and record blank-line offsets.
|
||||
if (ch === ' ' || ch === '\t' || ch === '\n' || ch === '\r') {
|
||||
let newlines = 0;
|
||||
let blankAt = -1;
|
||||
while (i < n) {
|
||||
const c = source[i];
|
||||
if (c === '\n') {
|
||||
newlines++;
|
||||
if (newlines === 2) blankAt = i;
|
||||
} else if (c !== ' ' && c !== '\t' && c !== '\r') {
|
||||
break;
|
||||
}
|
||||
i++;
|
||||
}
|
||||
if (blankAt >= 0) blankLines.push(blankAt);
|
||||
continue;
|
||||
}
|
||||
|
||||
@@ -228,7 +246,7 @@ export function tokenize(source: string): LexResult {
|
||||
}
|
||||
|
||||
tokens.push({ kind: 'EOF', start: n, end: n, text: '' });
|
||||
return { tokens, comments };
|
||||
return { tokens, comments, blankLines };
|
||||
}
|
||||
|
||||
function isDigit(c: string): boolean {
|
||||
|
||||
@@ -21,10 +21,10 @@ import type {
|
||||
import { tokenize, type Token, type TokenKind } from './lexer';
|
||||
|
||||
export function parse(source: string): ParseResult {
|
||||
const { tokens, comments } = tokenize(source);
|
||||
const { tokens, comments, blankLines } = tokenize(source);
|
||||
const p = new Parser(source, tokens);
|
||||
const program = p.parseProgram();
|
||||
return { source, program, errors: p.errors, comments };
|
||||
return { source, program, errors: p.errors, comments, blankLines };
|
||||
}
|
||||
|
||||
// Precedence levels for binary operators. Higher binds tighter. Assignment
|
||||
@@ -92,13 +92,21 @@ class Parser {
|
||||
return true;
|
||||
}
|
||||
|
||||
private expect(kind: TokenKind, text?: string): Token {
|
||||
// `role` is a human-readable description of what was expected, used
|
||||
// in place of the bare token kind so the message reads like Rhai's
|
||||
// own diagnostics (`Expecting name of a variable` rather than
|
||||
// `expected ident`). Falls back to the literal/kind when omitted.
|
||||
private expect(kind: TokenKind, text?: string, role?: string): Token {
|
||||
const t = this.peek();
|
||||
if (t.kind === kind && (text === undefined || t.text === text)) {
|
||||
return this.advance();
|
||||
}
|
||||
const desc = text !== undefined ? `'${text}'` : kind.toLowerCase();
|
||||
this.error(t, `expected ${desc}, got '${t.text || 'end of input'}'`);
|
||||
if (t.kind === 'EOF') {
|
||||
this.error(t, role ? `Expecting ${role} — script is incomplete` : 'Script is incomplete');
|
||||
} else {
|
||||
const desc = role ?? (text !== undefined ? `'${text}'` : kind.toLowerCase());
|
||||
this.error(t, `Expecting ${desc}`);
|
||||
}
|
||||
// Return the token without consuming so the caller's parent can
|
||||
// still resync at its own boundary.
|
||||
return t;
|
||||
@@ -192,7 +200,7 @@ class Parser {
|
||||
const expr = this.tryParseExpr();
|
||||
if (!expr) {
|
||||
const bad = this.peek();
|
||||
this.error(bad, `unexpected '${bad.text || 'end of input'}'`);
|
||||
this.error(bad, bad.kind === 'EOF' ? 'Script is incomplete' : `Unexpected token '${bad.text}'`);
|
||||
this.resyncStmt();
|
||||
return null;
|
||||
}
|
||||
@@ -208,7 +216,7 @@ class Parser {
|
||||
|
||||
private parseLetOrConst(kind: 'Let' | 'Const'): Stmt {
|
||||
const start = this.advance().start; // let|const
|
||||
const nameTok = this.expect('Ident');
|
||||
const nameTok = this.expect('Ident', undefined, 'name of a variable');
|
||||
const name = nameTok.text;
|
||||
const nameRange = { start: nameTok.start, end: nameTok.end };
|
||||
let init: Expr | null = null;
|
||||
@@ -222,11 +230,11 @@ class Parser {
|
||||
|
||||
private parseFnDecl(): FnDecl {
|
||||
const start = this.advance().start; // fn
|
||||
const nameTok = this.expect('Ident');
|
||||
const nameTok = this.expect('Ident', undefined, 'function name in function declaration');
|
||||
this.expect('Punct', '(');
|
||||
const params: Param[] = [];
|
||||
while (!this.check('Punct', ')') && this.peek().kind !== 'EOF') {
|
||||
const pTok = this.expect('Ident');
|
||||
const pTok = this.expect('Ident', undefined, 'parameter name');
|
||||
params.push({ name: pTok.text, start: pTok.start, end: pTok.end });
|
||||
if (!this.match('Punct', ',')) break;
|
||||
}
|
||||
@@ -269,7 +277,7 @@ class Parser {
|
||||
|
||||
private parseFor(): Stmt {
|
||||
const start = this.advance().start; // for
|
||||
const nameTok = this.expect('Ident');
|
||||
const nameTok = this.expect('Ident', undefined, 'loop variable name');
|
||||
this.expect('Keyword', 'in');
|
||||
const iter = this.tryParseExpr() ?? this.placeholderExpr();
|
||||
const body = this.parseBlockExpr();
|
||||
@@ -305,7 +313,7 @@ class Parser {
|
||||
private parseBlockExpr(): BlockExpr {
|
||||
const openTok = this.peek();
|
||||
if (!this.match('Punct', '{')) {
|
||||
this.error(openTok, "expected '{'");
|
||||
this.error(openTok, "Expecting '{' to begin a block");
|
||||
return { kind: 'BlockExpr', start: openTok.start, end: openTok.start, stmts: [] };
|
||||
}
|
||||
const start = openTok.start;
|
||||
@@ -372,7 +380,7 @@ class Parser {
|
||||
const t = this.peek();
|
||||
if (t.kind === 'Punct' && t.text === '.') {
|
||||
this.advance();
|
||||
const prop = this.expect('Ident');
|
||||
const prop = this.expect('Ident', undefined, 'name of a property');
|
||||
expr = {
|
||||
kind: 'Member',
|
||||
start: expr.start,
|
||||
@@ -403,7 +411,7 @@ class Parser {
|
||||
// Namespace path: treat `log::info` as a Member chain on an
|
||||
// Ident so completion and lookup can walk the same shape.
|
||||
this.advance();
|
||||
const next = this.expect('Ident');
|
||||
const next = this.expect('Ident', undefined, "name after '::'");
|
||||
expr = {
|
||||
kind: 'Member',
|
||||
start: expr.start,
|
||||
@@ -476,7 +484,7 @@ class Parser {
|
||||
return this.parseBlockExpr();
|
||||
}
|
||||
|
||||
this.error(t, `unexpected '${t.text || 'end of input'}'`);
|
||||
this.error(t, t.kind === 'EOF' ? 'Script is incomplete' : `Unexpected token '${t.text}'`);
|
||||
// Consume one token so we make forward progress, then return a
|
||||
// placeholder so the surrounding parser keeps its shape.
|
||||
this.advance();
|
||||
@@ -534,7 +542,7 @@ class Parser {
|
||||
this.expect('Punct', '(');
|
||||
const params: Param[] = [];
|
||||
while (!this.check('Punct', ')') && this.peek().kind !== 'EOF') {
|
||||
const pTok = this.expect('Ident');
|
||||
const pTok = this.expect('Ident', undefined, 'parameter name');
|
||||
params.push({ name: pTok.text, start: pTok.start, end: pTok.end });
|
||||
if (!this.match('Punct', ',')) break;
|
||||
}
|
||||
@@ -577,7 +585,7 @@ class Parser {
|
||||
key = k.text.length >= 2 ? k.text.slice(1, -1) : k.text;
|
||||
keyRange = { start: k.start, end: k.end };
|
||||
} else {
|
||||
this.error(k, 'expected map key');
|
||||
this.error(k, 'Expecting name of a map key');
|
||||
break;
|
||||
}
|
||||
this.expect('Punct', ':');
|
||||
|
||||
@@ -76,7 +76,7 @@
|
||||
editableSource = r.text;
|
||||
rhaiFormatError = null;
|
||||
} else {
|
||||
rhaiFormatError = `Parse error at offset ${r.error.offset}: ${r.error.message}`;
|
||||
rhaiFormatError = `Parse error: ${r.error.message} (line ${r.error.line}, position ${r.error.column})`;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user