Preserve comments across web UI config edits #88

Merged

danny merged 1 commit from issue34_preserve_comments into main

2026-07-10 08:06:22 +01:00

claude commented

2026-07-10 07:50:26 +01:00

Collaborator

Closes #34.

See the issue comment for the full design writeup. Summary:

New config.MergeIntoYAML(baseYAML, newConfig) (internal/config/comments.go): marshals newConfig the ordinary way into a comment-free tree, then splices baseYAML's comments onto the matching nodes via yaml.v3's yaml.Node. Sequence items (connections, defaults.by_network, forwarded_ports) are matched by natural identity, not position, so a reorder in the structured form moves the comment with it rather than stranding it. A removed item's comment is dropped, not reattached to some unrelated neighbor. Falls back to a plain marshal if the base doesn't parse.
configEditRequest gains base_yaml, sent by the client alongside every structured-form submission; config_edit.js's state gains baseYAML, updated from every raw/validate/save response so comments survive arbitrarily many mode toggles within a session, not just a single save.
Verified against the real yaml.v3 library (not assumed) exactly where each comment kind attaches - a preamble lands in HeadComment on the key node, an inline comment in LineComment on the value node, and a sequence-item preamble in HeadComment on the item node itself, which is already exactly the preamble-block convention you suggested.
internal/config stays effectively 100% covered - the only gaps are yaml.Marshal error branches on a fixed-shape Go struct, which can't realistically fail. New TestAPIConfigSaveStructuredFormPreservesComments is the end-to-end regression test through the real HTTP handlers.

make lint/make test clean.

Separately: while checking test coverage I hit an unrelated stale git stash (WIP on main: cb8f04b Merge branch 'issue27_toolchain_versioning') that conflicted on Makefile - resolved by keeping the current Makefile (confirmed identical to HEAD after). I did not drop the stash entry itself - it's superseded/obsolete scratch from mid-development of issues #26/#36 (both already merged in a different final form), but I did not want to delete it without your say. Let me know if you'd like me to drop it or if you want to look at it first.

Closes #34. See the issue comment for the full design writeup. Summary: - New `config.MergeIntoYAML(baseYAML, newConfig)` (`internal/config/comments.go`): marshals `newConfig` the ordinary way into a comment-free tree, then splices `baseYAML`'s comments onto the matching nodes via yaml.v3's `yaml.Node`. Sequence items (`connections`, `defaults.by_network`, `forwarded_ports`) are matched by natural identity, not position, so a reorder in the structured form moves the comment with it rather than stranding it. A removed item's comment is dropped, not reattached to some unrelated neighbor. Falls back to a plain marshal if the base doesn't parse. - `configEditRequest` gains `base_yaml`, sent by the client alongside every structured-form submission; `config_edit.js`'s `state` gains `baseYAML`, updated from every raw/validate/save response so comments survive arbitrarily many mode toggles within a session, not just a single save. - Verified against the real yaml.v3 library (not assumed) exactly where each comment kind attaches - a preamble lands in `HeadComment` on the key node, an inline comment in `LineComment` on the value node, and a sequence-item preamble in `HeadComment` on the item node itself, which is already exactly the preamble-block convention you suggested. - `internal/config` stays effectively 100% covered - the only gaps are `yaml.Marshal` error branches on a fixed-shape Go struct, which can't realistically fail. New `TestAPIConfigSaveStructuredFormPreservesComments` is the end-to-end regression test through the real HTTP handlers. `make lint`/`make test` clean. Separately: while checking test coverage I hit an unrelated stale git stash (`WIP on main: cb8f04b Merge branch 'issue27_toolchain_versioning'`) that conflicted on `Makefile` - resolved by keeping the current Makefile (confirmed identical to HEAD after). I did not drop the stash entry itself - it's superseded/obsolete scratch from mid-development of issues #26/#36 (both already merged in a different final form), but I did not want to delete it without your say. Let me know if you'd like me to drop it or if you want to look at it first.

claude added 1 commit

2026-07-10 07:50:26 +01:00

Preserve comments across web UI config edits

CI / lint (push) Successful in 28s

Details

CI / build (push) Successful in 25s

Details

CI / lint (pull_request) Successful in 25s

Details

CI / build (pull_request) Successful in 27s

Details

CI / release (push) Has been skipped

Details

CI / release (pull_request) Has been skipped

Details

c568ae0545

The structured per-field edit form destroyed every comment in the
file, both on save (a plain yaml.Marshal of the submitted struct has
no memory of the original file) and on every form-to-raw toggle (the
raw textarea got overwritten with fresh, comment-free YAML even
without saving).

Add config.MergeIntoYAML, which marshals the submitted config the
ordinary way and splices the original file's comments onto the
matching nodes of that tree via yaml.v3's yaml.Node - sequence items
(connections, defaults.by_network, forwarded_ports) are matched by
natural identity rather than position, so reordering a connection in
the form moves its comment with it. Comments can still only be
written via the raw YAML textarea, but they now survive being
carried through a structured-form edit session and back out again.

Closes #34

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

claude requested review from danny

2026-07-10 07:50:36 +01:00