[yaml] Add optional layout retention for comments, whitespace, and delimiter spans by sigurdm · Pull Request #2595 · dart-lang/tools

sigurdm · 2026-09-10T09:23:36Z

Overview

This PR adds optional layout retention to package:yaml without impacting existing parsing performance or backwards compatibility when disabled.

When retainLayout: true is passed to loadYaml, loadYamlNode, loadYamlDocument, loadYamlStream, or Loader:

Captures source layout elements (LayoutElement: CommentElement, WhitespaceElement, NewlineElement) in leadingLayout and trailingLayout on tokens, events, YamlNode, and YamlDocument.
Distinguishes same-line trailing comments (CommentElement.isTrailing: true) from full-line/preceding comments (CommentElement.isTrailing: false).
Captures delimiter spans:
- YamlMap.colonSpan(key): the SourceSpan of the association colon (:) for mapping entries.
- YamlList.dashSpan(index): the SourceSpan of the sequence hyphen (-) for sequence entries.
Pattern matching support via sealed LayoutElement hierarchy.

When retainLayout: false (default):

100% backwards compatible.
Zero extra allocations for layout element lists and span maps.
All 585 existing tests in package:yaml pass without modification.

TAG=agy
CONV=eabffc54-0045-490f-beb6-67e4668362d1

…limiter spans Adds retainLayout: false (default) to loadYaml, loadYamlNode, loadYamlDocument, and Loader. When enabled: - Preserves layout elements (LayoutElement: CommentElement, WhitespaceElement, NewlineElement) in leadingLayout and trailingLayout on tokens, events, YamlNode, and YamlDocument. - Preserves delimiter source spans: YamlMap.colonSpan(key) and YamlList.dashSpan(index). - Has zero overhead and maintains 100% backwards compatibility when retainLayout is false. TAG=agy CONV=eabffc54-0045-490f-beb6-67e4668362d1

gemini-code-assist

Code Review

This pull request introduces a layout retention feature to the yaml package, allowing the parser to preserve non-semantic source layout elements (such as comments, whitespace, and newlines) as well as source spans for colons and hyphens when retainLayout is enabled. The review feedback focuses on optimizing memory usage and avoiding unnecessary allocations when retainLayout is disabled (the default). Specifically, the reviewer suggests lazily allocating trailingLayout lists on Tokens, as well as lazily initializing dashSpans and colonSpans in the Loader class only when they are actually encountered.

gemini-code-assist · 2026-09-10T09:25:58Z

+  final List<LayoutElement> trailingLayout;

-  Token(this.type, this.span);
+  Token(
+    this.type,
+    this.span, {
+    this.leadingLayout = const [],
+    List<LayoutElement>? trailingLayout,
+  }) : trailingLayout = trailingLayout ?? [];


To avoid allocating a new empty growable list [] for trailingLayout on every single token when layout retention is disabled, we can make trailingLayout non-final, default it to const [], and provide a helper method addTrailingLayout that lazily allocates the growable list only when a trailing layout element is actually added.

Suggested change

final List<LayoutElement> trailingLayout;

Token(this.type, this.span);

Token(

this.type,

this.span, {

this.leadingLayout = const [],

List<LayoutElement>? trailingLayout,

}) : trailingLayout = trailingLayout ?? [];

List<LayoutElement> trailingLayout;

Token(

this.type,

this.span, {

this.leadingLayout = const [],

this.trailingLayout = const [],

});

void addTrailingLayout(LayoutElement element) {

if (identical(trailingLayout, const <LayoutElement>[])) {

trailingLayout = <LayoutElement>[];

}

trailingLayout.add(element);

}

gemini-code-assist · 2026-09-10T09:25:58Z

+          if (!afterLineBreak && _lastToken != null) {
+            _lastToken!.trailingLayout.add(elem);
+          } else {


Use the new addTrailingLayout helper method on Token to lazily allocate the trailingLayout list only when needed.

if (!afterLineBreak && _lastToken != null) { _lastToken!.addTrailingLayout(elem); } else {

gemini-code-assist · 2026-09-10T09:25:58Z

+          if (!afterLineBreak && _lastToken != null) {
+            _lastToken!.trailingLayout
+                .add(CommentElement(commentSpan, isTrailing: true));
+          } else {


Use the new addTrailingLayout helper method on Token to lazily allocate the trailingLayout list only when needed.

if (!afterLineBreak && _lastToken != null) { _lastToken!.addTrailingLayout( CommentElement(commentSpan, isTrailing: true)); } else {

gemini-code-assist · 2026-09-10T09:25:58Z

+      if (_scanner.state.position > wsStart.position) {
+        token.trailingLayout.add(WhitespaceElement(_scanner.spanFrom(wsStart)));
+      }
+      if (_scanner.peekChar() == HASH) {
+        var commentStart = _scanner.state;
+        _skipComment();
+        token.trailingLayout.add(CommentElement(
+            _scanner.spanFrom(commentStart),
+            isTrailing: true));
+      }


Use the new addTrailingLayout helper method on Token to lazily allocate the trailingLayout list only when needed.

if (_scanner.state.position > wsStart.position) { token.addTrailingLayout(WhitespaceElement(_scanner.spanFrom(wsStart))); } if (_scanner.peekChar() == HASH) { var commentStart = _scanner.state; _skipComment(); token.addTrailingLayout(CommentElement( _scanner.spanFrom(commentStart), isTrailing: true)); }

gemini-code-assist · 2026-09-10T09:25:58Z

    var children = <YamlNode>[];
+    var dashSpans = <SourceSpan?>[];


To ensure zero extra allocations when retainLayout is false, lazily initialize dashSpans only when a non-null dash span is actually encountered.

Suggested change

var children = <YamlNode>[];

var dashSpans = <SourceSpan?>[];

var children = <YamlNode>[];

List<SourceSpan?>? dashSpans;

gemini-code-assist · 2026-09-10T09:25:58Z

      while (event.type != EventType.sequenceEnd) {
+        dashSpans.add(_parser.lastDashSpan);
        children.add(_loadNode(event));
        event = _parser.parse();
      }


Lazily populate dashSpans only if we encounter a non-null dash span, padding with nulls up to the current index if necessary.

while (event.type != EventType.sequenceEnd) { var dashSpan = _parser.lastDashSpan; if (dashSpan != null) { dashSpans ??= List<SourceSpan?>.filled(children.length, null, growable: true); dashSpans.add(dashSpan); } else if (dashSpans != null) { dashSpans.add(null); } children.add(_loadNode(event)); event = _parser.parse(); }

gemini-code-assist · 2026-09-10T09:25:58Z

+    if (dashSpans.any((s) => s != null)) {
+      setDashSpans(node, dashSpans);
+    }


Only call setDashSpans if dashSpans was actually allocated.

Suggested change

if (dashSpans.any((s) => s != null)) {

setDashSpans(node, dashSpans);

}

if (dashSpans != null) {

setDashSpans(node, dashSpans);

}

gemini-code-assist · 2026-09-10T09:25:58Z

    var children = deepEqualsMap<dynamic, YamlNode>();
+    var colonSpans = deepEqualsMap<dynamic, SourceSpan>();


To ensure zero extra allocations when retainLayout is false, lazily initialize colonSpans only when a non-null colon span is actually encountered.

Suggested change

var children = deepEqualsMap<dynamic, YamlNode>();

var colonSpans = deepEqualsMap<dynamic, SourceSpan>();

var children = deepEqualsMap<dynamic, YamlNode>();

Map<dynamic, SourceSpan>? colonSpans;

gemini-code-assist · 2026-09-10T09:25:58Z

+        var colonSpan = _parser.lastColonSpan;
+        if (colonSpan != null) {
+          colonSpans[key] = colonSpan;
+        }


Lazily allocate the colonSpans map only when a non-null colon span is encountered.

Suggested change

var colonSpan = _parser.lastColonSpan;

if (colonSpan != null) {

colonSpans[key] = colonSpan;

}

var colonSpan = _parser.lastColonSpan;

if (colonSpan != null) {

colonSpans ??= deepEqualsMap<dynamic, SourceSpan>();

colonSpans[key] = colonSpan;

}

gemini-code-assist · 2026-09-10T09:25:58Z

+    if (colonSpans.isNotEmpty) {
+      setColonSpans(node, colonSpans);
+    }


Only call setColonSpans if colonSpans was actually allocated.

Suggested change

if (colonSpans.isNotEmpty) {

setColonSpans(node, colonSpans);

}

if (colonSpans != null) {

setColonSpans(node, colonSpans);

}

cst.dart was an exploratory prototype for a standalone CST with trivia definitions. Now that layout retention (LayoutElement, colonSpan, dashSpan) is provided directly by package:yaml (PR dart-lang#2595), this file is completely unused and redundant. TAG=agy CONV=eabffc54-0045-490f-beb6-67e4668362d1

…t retention - Add anchorSpan, aliasSpan, entrySpan, commaSpan, openSpan, closeSpan, startMarkerSpan, and endMarkerSpan accessors to YamlNode, YamlMap, YamlList, and YamlDocument. - In Loader, construct entry spans for mapping and sequence items preserving leading comments and indentation trivia. - In YamlMapWrapper and YamlListWrapper, forward layout span accessors. - Add comprehensive tests in test/layout_test.dart. TAG=agy CONV=eabffc54-0045-490f-beb6-67e4668362d1

sigurdm · 2026-09-10T12:38:30Z

Superseded by and merged into unified monorepo PR #2593.

github-actions Bot added the package:yaml label Sep 10, 2026

gemini-code-assist Bot reviewed Sep 10, 2026

View reviewed changes

sigurdm mentioned this pull request Sep 10, 2026

[yaml, yaml_edit] Add token stream to package:yaml and rewrite yaml_edit using token-driven CST #2593

Draft

sigurdm closed this Sep 10, 2026

Navigation Menu

[yaml] Add optional layout retention for comments, whitespace, and delimiter spans - #2595

[yaml] Add optional layout retention for comments, whitespace, and delimiter spans#2595
sigurdm wants to merge 2 commits into
dart-lang:mainfrom
sigurdm:yaml-layout-retention

sigurdm commented Sep 10, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Uh oh!

sigurdm commented Sep 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		var children = <YamlNode>[];
		var dashSpans = <SourceSpan?>[];

		var children = deepEqualsMap<dynamic, YamlNode>();
		var colonSpans = deepEqualsMap<dynamic, SourceSpan>();

Conversation

sigurdm commented Sep 10, 2026

Overview

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Sep 10, 2026

Choose a reason for hiding this comment

Uh oh!

sigurdm commented Sep 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant