Skip to content

Data validation: a list typed without quotes is saved as a quoted list, and an unknown type is refused - #1084

Merged
aiongg merged 2 commits into
storytold:mainfrom
CouchRiv:fix/validation-inline-list-quotes
Oct 22, 2026
Merged

aiongg merged 2 commits into
storytold:mainfrom
CouchRiv:fix/validation-inline-list-quotes

Conversation

@CouchRiv

Copy link
Copy Markdown
Contributor

Found while driving GridCraft 0.6.1 over MCP. data.validation {type: "list", formula1: "Open,In progress,Closed"} is how Excel's Source box takes a list and how an agent naturally writes it. It works inside GridCraft (data.validate accepts "In progress"), but the rule is saved as

<formula1>Open,In progress,Closed</formula1>

In a file an inline list is a quoted string literal. Excel writes "Open,In progress,Closed", openpyxl documents formula1='"Dog,Cat,Bat"' (https://openpyxl.readthedocs.io/en/stable/validation.html), and GridCraft's own reader treats an unquoted list formula1 as a formula (read_validation prefixes it with =). This shows without Excel: on main, save and reopen in GridCraft, and the rule's first item has become =Open, so "Open" is refused and "=Open" accepted.

What changed

All in crates/engine/src/cmd/data.rs.

  • validation: for a list rule, the source goes through list_source. A literal list typed without quotes is stored as the quoted string a file holds, with inner quotes doubled (Say "hi",Bye becomes "Say ""hi"",Bye"). It is stored as before when it is a formula (typed with =), a reference ($A$1:$A$3), a name, a sheet-qualified range (Sheet2!A1:A3), already quoted, or empty. Other rule types are unchanged.
  • "Literal list" is the test list_items already used to choose between splitting and evaluating (quoted, or no ! and not a bare reference or name). It moves into is_literal_list so both use the same rule, and GridCraft's reading of a rule doesn't change.
  • list_items reads a quoted list as a string literal: "" inside stands for one ". Without this, an item with a quote would read back as Say ""hi"" after the change; it also reads such a list from a file correctly. Unquoted sources split as before.

Second commit: unknown values are refused. data.validation took any type, operator or errorStyle and fell back to a default. type: "lst" saved an "any value" rule, <dataValidation type="none" …>, which validates nothing, and an unknown operator became between. Each is now an error listing the accepted values. any stays a type, the operator aliases (greaterThan, lessThanOrEqual…) still work, and the UI dialog only sends listed values. cmd::data::tests::a_rule_with_an_unknown_type_operator_or_error_style_is_refused fails on the first commit (called Result::unwrap_err() on an Ok value: Null for type: "lst") and passes with the second; cargo test -p gridcraft-engine -p gridcraft-mcp -p gridcraft-ui-egui pass with both.

Why at creation, not in the XLSX writer

Creating the rule is the last point that knows whether the user typed =: validation drops it, so =OFFSET($A$1,0,0,5,1) and a literal list look alike once stored. Quoting in validation_formula would therefore turn a formula entered through the command into a literal string. Rules read from a file are already quoted ("a,b") or prefixed with =, so the writer has nothing to fix for them. One place also covers MCP, the CLI, the control channel and the Data Validation dialog (its Source box sends formula1 as typed), and the model then holds the same form whether a rule came from a command or from a file.

Verification (Linux x86_64, original baseline fc45b7f)

  • cargo test -p gridcraft-engine --lib cmd::data::tests::a_list_typed_without_quotes_is_saved_as_a_quoted_list: nine list rules. Sources are Open,In progress,Closed, the same already quoted, 1,2,3, Say "hi",Bye, =$K$1:$K$3, $K$1:$K$3, =Colors (a defined name), Sheet1!$K$1:$K$3 and =OFFSET($K$1,0,0,3,1). The test checks that each accepts a listed entry and refuses another, that each <formula1> is saved as expected (quoted for the first four, unchanged for the others), that the reopened workbook accepts and refuses the same entries, and that a second save writes the same sheet XML. It fails on main (data.rs from main plus this test) with A1: "Open,In progress,Closed" missing from <?xml … and passes here.
  • End to end, gridcraft-cli mcp on both builds, rules on A2:A5 to D2:D5, save, reopen:
step main (fc45b7f) this branch
A: Open,In progress,Closed saved <formula1>Open,In progress,Closed</formula1> <formula1>"Open,In progress,Closed"</formula1>
B: "Open,In progress,Closed" saved quoted quoted (unchanged)
C: =$K$1:$K$3 saved $K$1:$K$3 the same
D: =OFFSET($K$1,0,0,3,1) saved OFFSET($K$1,0,0,3,1) the same
data.validate A2 "In progress", before saving ok ok
after reopening, A2 "Open" / "=Open" refused / accepted accepted / refused
  • cargo fmt --all -- --check, cargo clippy -p gridcraft-engine --all-targets -- -D warnings: clean
  • cargo test -p gridcraft-engine (lib and tests/, 244 tests) and cargo test -p gridcraft-ui-egui (596 tests, it draws the list dropdown from list_items): all pass

Limits and questions

  • Not checked in Excel (none here). With the unquoted form I'd expect Excel to read four names joined by union and intersection operators; with the quoted form the dropdown Excel's own dialog produces.
  • A single word typed without = (Yes) is still taken as a name, as GridCraft already evaluated it; Excel's Source box would take it as a one-item list. Telling them apart would need a lookup of the workbook's names; left out.
  • A function call typed without = (OFFSET(…)) is now quoted, since list_items already read it as a literal list. With = it stays a formula.
  • Not changed, seen while testing: list_items reads a formula source (=OFFSET(…), =INDIRECT(…)) as a literal list, so such a rule refuses valid entries inside GridCraft (row D above refuses "Green" on both builds). The file keeps the formula. Worth its own fix.
  • JSON workbooks saved before this change keep their unquoted lists and save to .xlsx as before.
  • Lists split on ,, the separator files use. main has no locale list separator; if regional formats (M12: regional formats (typed and displayed numbers, dates and times) #447, M12: Excel-style languages (12 languages, local formulas, regional formats) #191) let users type ;, it would be mapped here.
  • Excel limits an inline list to 255 characters; not enforced here.
  • With Data Validation dialog opens on the active cell's current rule (#522) #633 (the dialog opens on the current rule), the Source box would show the quoted form; OK keeps the rule unchanged. Excel's dialog shows the list without quotes, so Data Validation dialog opens on the active cell's current rule (#522) #633 could strip them for display. With Paste Special: copy the validation rule with the Validation and All options (#562) #602 (paste the validation rule), a quoted list is a string and is not moved as references; I haven't tested that combination.
  • The params doc of data.validation is unchanged, since both forms are accepted as before. A hint like formula1: "a,b,c" | "=$A$1:$A$3" could help agents; happy to add it.

Independent verification against updated main (84acf62, 2026-10-13)

The independent review reran cargo fmt --all -- --check, cargo clippy -p gridcraft-engine --all-targets -- -D warnings and cargo test -p gridcraft-engine on the branch and its clean merge with 84acf62; all exited 0. The added regression tests failed with production code from 84acf62 and passed on both branch and merged trees. These checks are separate from the historical fc45b7f measurements above.

Conflicts

Against upstream/main at 84acf62, git merge-tree is clean. This partially overlaps #1064, which also rejects unknown validation types and operators, and conflicts in data.rs. The quoted inline-list handling and errorStyle validation are additional. Whichever lands second should retain these changes and reuse #1064's enum name tables. #870, #480, #884 and #885 merge cleanly. The review covered 100 listed open PRs plus #480, not every older PR.

CouchRiv and others added 2 commits October 12, 2026 23:08
data.validation {type: "list", formula1: "Open,In progress,Closed"}, which is
how Excel's Source box takes a list, worked inside GridCraft but was saved as
<formula1>Open,In progress,Closed</formula1>. In a file an inline list is a
string literal, "Open,In progress,Closed" (what Excel writes and what our own
reader expects); unquoted, formula1 reads as a formula of names.

- validation quotes such a source when the rule is created, doubling inner
  quotes, using the test list_items already used to tell a literal list from
  a reference or a name (now is_literal_list). A source typed with = (a
  formula), a reference, a name, a sheet-qualified range and an already quoted
  list are stored as before.
- list_items reads a quoted list as a string literal: "" inside stands for
  one quote, so an item with a quote reads back the same after the change and
  from a file.

Creating the rule is the one place that still knows whether the user typed =,
so the XLSX writer is left alone: it can't tell OFFSET(...) entered as a
formula from a literal list, and a rule read from a file is already quoted.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
data.validation took any `type`, `operator` or `errorStyle` and fell back
to a default: a typo such as `type: "lst"` saved an "any value" rule
(`type="none"`), which validates nothing, and an unknown operator became
`between`. Each is now an error that lists the accepted values; `any`
stays a type, and the operator aliases (`greaterThan`…) still work.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@aiongg
aiongg merged commit f8f7193 into storytold:main Oct 22, 2026
1 check passed
@CouchRiv
CouchRiv deleted the fix/validation-inline-list-quotes branch October 29, 2026 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants