fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

review: the round-trip test could not see a merge that dropped the section

Browse Source 
Adversarial review injected the other fault -- Some(_existing) => {}, which
drops the duplicate section outright -- and the whole improvise-io suite stayed
green at 2000 proptest cases while the fixture's item and its group line
vanished from the document. A test that compares two printed forms cannot tell
unstable-but-lossless from stable-and-lossy, because both sides of the equality
lose the same thing. Three contains assertions now run before the equality, and
they fail under that injection.

Two more from the same review. push_category destructures PCategory, so a fourth
field has to be named rather than silently dropped on the duplicate path only.
And design-principles.md's formula-tokenizer section described a tokenizer that
no longer exists -- a split_where pre-parse step and Token::Ident/Token::Str,
none of which are in the tree -- and claimed bare identifiers allow spaces,
which bare_ident forbids. This branch changes that production, so correcting the
sibling copy is the grep gotcha 48 asks for.

improvise-349m improvise-r08u

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FC2jVn8Au7E9JPkw6bwXXH
This commit is contained in:
Edward Langley
2026-09-20 21:26:34 -07:00
parent e9b8b708c5
commit d5995814d1
3 changed files with 46 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
context/design-principles.md
+15 -9
View File
@@ -145,11 +145,18 @@ Escapes inside pipes: `\|` (literal pipe), `\\` (backslash), `\n` (newline).
This convention is shared between the `.improv` persistence format and the
formula parser's identifier syntax.
### Formula Tokenizer: Identifiers and Quoting
### Formula Identifiers and Quoting
**Bare identifiers** support multi-word names (e.g., `Total Revenue`) by
allowing spaces when followed by non-operator, non-keyword characters. Keywords
(`WHERE`, `SUM`, `AVG`, `MIN`, `MAX`, `COUNT`, `IF`) act as token boundaries.
**Bare identifiers** are `(ASCII_ALPHA | "_") ~ (ASCII_ALPHANUMERIC | "_" | "-")*`
— no spaces, the same shape `improv.pest`'s `bare_name` has. A multi-word name
such as `Total Revenue` must be pipe-quoted.
**Keywords need a word boundary after them, not before.** `IS`, `WHERE` and the
aggregate names are spelled `^"…"` in `formula.pest`, and `is_expr`/`where_clause`/
`inline_where` are compound-atomic with `sp+` so that `Region ISLAND` and
`SUM(Rev WHEREAS x = y)` are parse errors rather than silently-wrong filters. A
`SUM`/`IF` without parens falls through to `ref_expr`, which is why a bare
keyword still names an item; the writer quotes it anyway (`quote::quote_name`).
**Pipe-quoted identifiers** (`|...|`) allow any characters — including spaces,
keywords, and operators — inside the delimiters. Use pipes when a category or
@@ -162,11 +169,10 @@ item name collides with a keyword or contains special characters:
SUM(|Net Revenue| WHERE |Region Name| = |East Coast|)
```
Pipes produce `Token::Ident` (same as bare identifiers), so they work
everywhere an identifier is expected: expressions, aggregate arguments, WHERE
clause category names and filter values. Double-quoted strings (`"..."`)
remain `Token::Str` and are used only for WHERE filter values in the
`split_where` pre-parse step.
`identifier` is `pipe_quoted | bare_ident`, so a pipe-quoted name works
everywhere a bare one does: expressions, aggregate arguments, and a `WHERE`
clause's category name. A double-quoted string is a `filter_value` only — the
right-hand side of a `WHERE` or an `IS`, never an expression.
---