Skip to content

Keep comments around arrays of tables - #104

Closed
jonathangreen wants to merge 2 commits into
pappasam:mainfrom
jonathangreen:pr/2-array-of-tables-comments
Closed

jonathangreen wants to merge 2 commits into
pappasam:mainfrom
jonathangreen:pr/2-array-of-tables-comments

Conversation

@jonathangreen

Copy link
Copy Markdown
Contributor

Two bugs in how comments around arrays of tables are carried through a sort. Both lose comments, and both make the output differ between the first and second run, so a formatter hook flags a file it has just written.

1. A comment before the third or later element is misplaced, then deleted

toml_elements_sorted looked up the element to attach a comment to with next(iter(new_aot)), which is the first element rather than the last one appended. With two elements they coincide, which is why the existing fixtures passed. With three or more, a comment before a later element was emitted after the first one:

[[a]]
x = 1

[[a]]
x = 2

# comment on third
[[a]]
x = 3

came out as

[[a]]
x = 1

# comment on third

[[a]]
x = 2
[[a]]
x = 3

Sorting that output again removed the comment entirely, since it is now separated from the next element by a blank line and counts as an orphan.

The fix attaches the comment to the last element appended so far. The same input now comes out unchanged:

[[a]]
x = 1

[[a]]
x = 2

# comment on third
[[a]]
x = 3

2. Comments before a nested array of tables are overwritten

table_to_tomlsortitem unconditionally moved a table's own comments onto the first element of a child array of tables, replacing the comments already attached to that element. For an explicit table this deleted the comment before [[m.n]] and displaced the comment on [m]:

[a]
y = 1

# about m
[m]
# about n
[[m.n]]
x = 1

came out as

[a]
y = 1

[m]

# about m
[[m.n]]
x = 1

# about n is gone and # about m has moved onto the nested array. A second run then removed that comment too.

Only super tables, which have no header of their own, need their comments pushed down to the first rendered child. The existing super-table walk now handles that case, descends into arrays of tables as well as nested super tables, and prepends instead of replacing, so comments already attached to the child are kept. The unconditional block is removed. The same input now comes out as:

[a]
y = 1

# about m
[m]

# about n
[[m.n]]
x = 1

Both comments stay where they were, and a second run leaves the file unchanged.

Tests

Two new example files under tests/examples, wired into the existing fixture test:

  • aot-comments.toml: three elements of one array of tables, with a comment before the second and the third. Output must equal input.
  • aot-nested-comments.toml: a commented table whose child is a commented array of tables, an array of tables reached through two implicit super tables, and arrays of tables nested inside array-of-tables elements. Every comment survives.

toml_elements_sorted looked up the element to attach comments to with
next(iter(new_aot)), which is the first element rather than the last.
With three or more elements a comment before a later element was emitted
after the first one, and a second run then removed it as an orphan. Use
the last element appended so far.
table_to_tomlsortitem unconditionally moved a table's own comments onto
the first element of a child array of tables, overwriting the comments
already attached to that element. For an explicit table this deleted the
comment before [[a.b]] and displaced the comment on [a], which a second
run then removed as an orphan. Only super tables need their comments
pushed down, and the existing super-table walk now also descends into
arrays of tables and prepends instead of replacing.
@dimbleby

dimbleby commented Sep 12, 2026 •

Copy link
Copy Markdown
Contributor

#102 also fixes this.

@pappasam

Copy link
Copy Markdown
Owner

Thanks for your fix! I opted to go for @dimbleby 's broader fix and am betting on his expertise for improved parsing going forward. Thanks for the PR, but closing since this should be resolved in 0.25.0

@pappasam pappasam closed this Sep 12, 2026
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.

3 participants