Skip to content

A write from the AST discards the file's comments and formatting #97

Description

@martin-fleck-at

Bug Description

Three write paths re-serialize a whole document from the AST: an integrity repair's write-back to a closed document, a diagram operation whose semantic projection changed, and a transfer write from a form editor. Comments are hidden CST tokens, so a re-serialization cannot carry them, and formatting goes with them.

The diagram case is the sharpest, because a .process file is meant to stay hand-authorable. A gesture that changes the projection — a create, a delete, a label edit — strips the file's leading comment, re-wraps task X writes … onto two lines and drops the trailing newline.

The loss is bounded by one guard, and it is worth knowing where. ReconcilingMultiDocumentGlspState.persist writes each document through ModelService.update only when hasChanged reports its projection differs, and that method's own comment states the skip is correctness rather than optimization. So a document the gesture did not change is safe, and one it did change is re-serialized in full — there is no comment-preserving serializer for the write to reach. Saving does not widen this: HydraniumGlspStorage.saveSourceModel flushes the text already in the store and runs no serializer.

serializer-golden.integration.test.ts feeds a source carrying comments and asserts semantic-model equality only, so it currently pins the loss rather than guarding against it.

Expected Behavior

A write that changes one part of a document leaves the rest of the file as the author wrote it, comments included.

Steps to Reproduce

  1. Open a .process file that has a leading comment and a wrapped task line.
  2. Change the diagram so the semantic projection differs — add a node, or rename one.
  3. The comment is gone, the wrapped line is reflowed, the trailing newline is dropped.

Additional Information

The comments are recoverable; attachment is the hard part. Langium's parser inserts hidden tokens into the CST via addHiddenNodes, and findCommentNode(node.$cstNode, …) is how the comment provider and hover already read a node's leading comment. So identifying which tokens are comments is not the difficulty. Three things on the write side are:

  • Attachment is unambiguous only for the leading case. A trailing comment, one between two members, or one separated by a blank line each need a rule, and a wrong rule moves a comment rather than losing it.
  • The nodes that matter to a repair are created or moved, so they carry no CST range at all — while the nodes that do carry one could have been copied verbatim.
  • Every adopter writes their own serializer, so a comment-carrying API is surface each of them has to participate in.

Three approaches, in increasing cost:

  • Skip the write. Detect hidden tokens and decline the silent disk write. Removes the data loss with no API change, but a commented file then stays unrepaired.
  • Splice the subtree. Write the re-serialised subtree over the dispatched node's CST range. Preserves comments outside that node, but cannot be complete: IntegrityRule.enforce returns only a boolean, so a rule may mutate outside the node it was dispatched on, and a newly created node has no range — both need a whole-document fallback that loses comments again.
  • Comment-preserving serialization. Complete, and the largest change; it wants participation from every adopter serializer and arguably support further upstream.

Formatting is a separate loss from comments — the re-wrap and the trailing newline survive no amount of comment reattachment.

The trailing newline is separable from the rest. It is not a consequence of re-serializing from the AST: AbstractSerializer.trimSerialized ends every serialization with text.trim(), which removes trailing blank lines and the final newline together. Its trimTrailingWhitespace option is all-or-nothing — false keeps every trailing blank line, for grammars whose values may legitimately end a document with them — so there is currently no way to ask for "no trailing blank lines, one final newline", which is what a conventional text file wants. Fixing that needs neither CST attachment nor a formatting-aware serializer, but it changes the bytes of every serialized document.

The transfer-write instance of this is described in docs/concepts/document-layers.md.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions