From 32970e0a30bbefda99d7aa94aa839802aa578753 Mon Sep 17 00:00:00 2001
From: Bryan Helmkamp <19+brynary@users.noreply.github.com>
Date: Wed, 13 May 2026 05:29:09 -0700
Subject: [PATCH] fix(graphviz): accept multi-line DOT attribute blocks without
 commas (#255)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Fixes #179.

## Summary
- `fabro validate`'s DOT parser rejected multi-line node attribute
blocks unless every attribute was comma-separated, forcing long node
definitions onto a single line.
- Per the DOT spec, the separator between attributes is optional —
whitespace (including newlines) alone is sufficient, and `,` or `;` are
both accepted as explicit separators.
- `attr_block` now uses `many0(terminated(attr, opt(',' | ';')))`
instead of `separated_list0(',', attr)`, so all three forms parse
identically.

## Before / after

```dot
// previously rejected — now parses
inspect [
    label="Inspect Code"
    shape=tab
    prompt="@prompts/inspect.md"
    class="heavy"
    reasoning_effort="high"
]
```

## Test plan
- [x] Added regression test `parse_attr_block_multiline_without_commas`
covering the exact form from #179.
- [x] `cargo nextest run -p fabro-graphviz` — 109/109 pass, including
the new test and existing comma-separated multi-line tests.
- [x] `cargo +nightly-2026-04-14 clippy -p fabro-graphviz --all-targets
-- -D warnings` clean.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Nate Aune <118984+natea@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .../fabro-graphviz/src/parser/grammar.rs      | 38 ++++++++++++++++---
 1 file changed, 33 insertions(+), 5 deletions(-)

diff --git a/lib/crates/fabro-graphviz/src/parser/grammar.rs b/lib/crates/fabro-graphviz/src/parser/grammar.rs
index dd9a83449..d205bc6c2 100644
--- a/lib/crates/fabro-graphviz/src/parser/grammar.rs
+++ b/lib/crates/fabro-graphviz/src/parser/grammar.rs
@@ -1,11 +1,11 @@
 use nom::IResult;
 use nom::branch::alt;
 use nom::bytes::complete::tag;
-use nom::character::complete::{char, multispace0};
+use nom::character::complete::{char, multispace0, one_of};
 use nom::combinator::opt;
 use nom::error::{Error, ParseError};
-use nom::multi::{many0, separated_list0};
-use nom::sequence::{delimited, preceded, tuple};
+use nom::multi::many0;
+use nom::sequence::{delimited, preceded, terminated, tuple};
 
 use crate::parser::ast::{
     AstValue, AttrBlock, DotGraph, EdgeStmt, NodeStmt, Statement, SubgraphStmt,
@@ -19,11 +19,15 @@ fn attr(input: &str) -> IResult<&str, (String, AstValue)> {
     Ok((rest, (k, v)))
 }
 
-/// Parse an attribute block: `[ attr (, attr)* ]`.
+/// Parse an attribute block: `[ attr (sep? attr)* ]` where `sep` is `,` or `;`.
+///
+/// Per the DOT spec, the separator between attributes is optional — whitespace
+/// (including newlines) alone is enough. This accepts comma-separated,
+/// semicolon-separated, and newline-separated attribute lists interchangeably.
 fn attr_block(input: &str) -> IResult<&str, AttrBlock> {
     delimited(
         preceded(ws, char('[')),
-        separated_list0(preceded(ws, char(',')), attr),
+        many0(terminated(attr, opt(preceded(ws, one_of(",;"))))),
         preceded(ws, char(']')),
     )(input)
 }
@@ -191,6 +195,30 @@ mod tests {
         assert_eq!(rest, "");
     }
 
+    // Regression test for https://github.com/fabro-sh/fabro/issues/179.
+    // Standard DOT allows newline (or any whitespace) as an attribute separator
+    // inside `[ ... ]`, with commas optional. The multi-line, comma-less form is
+    // what most DOT editors and formatters produce for long attribute lists.
+    #[test]
+    fn parse_attr_block_multiline_without_commas() {
+        let input = "[\n    label=\"Inspect Code\"\n    shape=tab\n    \
+                     prompt=\"@prompts/inspect.md\"\n    class=\"heavy\"\n    \
+                     reasoning_effort=\"high\"\n]";
+        let (rest, attrs) = attr_block(input).unwrap();
+        assert_eq!(attrs.len(), 5);
+        assert_eq!(attrs[0].0, "label");
+        assert_eq!(attrs[0].1, AstValue::Str("Inspect Code".into()));
+        assert_eq!(attrs[1].0, "shape");
+        assert_eq!(attrs[1].1, AstValue::Ident("tab".into()));
+        assert_eq!(attrs[2].0, "prompt");
+        assert_eq!(attrs[2].1, AstValue::Str("@prompts/inspect.md".into()));
+        assert_eq!(attrs[3].0, "class");
+        assert_eq!(attrs[3].1, AstValue::Str("heavy".into()));
+        assert_eq!(attrs[4].0, "reasoning_effort");
+        assert_eq!(attrs[4].1, AstValue::Str("high".into()));
+        assert_eq!(rest, "");
+    }
+
     #[test]
     fn parse_graph_attr_stmt() {
         let (_, stmt) = graph_attr_stmt("graph [goal=\"Run tests\"]").unwrap();