[dev] refactor: make DocumentInfo and sub-properties immutable
#205
Conversation
…nfo immutability
There was a problem hiding this comment.
Pull Request Overview
This PR transforms the DocumentInfo class and its sub-properties into immutable data structures using the amber library. The changes replace the previous automerge system with a new immutable approach where document information is updated by creating new instances rather than mutating existing ones.
Key changes include:
- Converting mutable document properties to immutable ones with the amber library
- Updating all document modification functions to use copy semantics
- Changing function signatures to require
MutableContextinstead ofContextfor modification operations
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| quarkdown-core/build.gradle.kts | Updates dependency from automerge to amber library |
| quarkdown-core/src/main/kotlin/com/quarkdown/core/document/DocumentInfo.kt | Makes DocumentInfo immutable with @NestedData annotation |
| quarkdown-core/src/main/kotlin/com/quarkdown/core/document/tex/TexInfo.kt | Converts TexInfo to immutable with Map instead of MutableMap |
| quarkdown-core/src/main/kotlin/com/quarkdown/core/document/layout/*/Info.kt | Adds @mergeable annotations and converts var to val properties |
| quarkdown-core/src/main/kotlin/com/quarkdown/core/context/MutableContext.kt | Adds mutable documentInfo property to support immutable updates |
| quarkdown-stdlib/src/main/kotlin/com/quarkdown/stdlib/Document.kt | Updates all document functions to use copy semantics for immutability |
| Test files | Updates test code to use new immutable patterns for DocumentInfo creation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import com.quarkdown.amber.annotations.Mergeable | ||
| import com.quarkdown.core.ast.Node |
There was a problem hiding this comment.
The import statement shows a change from automerge to amber, but this appears to be a simple library replacement. Ensure that the Mergeable annotation from the amber library provides the same functionality as the previous automerge implementation.
| import com.quarkdown.amber.annotations.Mergeable | |
| import com.quarkdown.core.ast.Node | |
| // NOTE: The Mergeable annotation was previously imported from automerge. |
quarkdown-core/src/main/kotlin/com/quarkdown/core/document/DocumentInfo.kt
Outdated
Show resolved
Hide resolved
| val authors = | ||
| this.authors + | ||
| it.map { (name, info) -> | ||
| DocumentAuthor( | ||
| name = name, | ||
| info = info.unwrappedValue.mapValues { (_, value) -> value.unwrappedValue }, | ||
| ) | ||
| } | ||
| copy(authors = authors) |
There was a problem hiding this comment.
[nitpick] The temporary authors variable assignment could be simplified by inlining the computation directly in the copy call, reducing unnecessary intermediate variables and improving readability.
This PR makes
DocumentInfoimmutable, along with its data properties (PageFormatInfo,ParagraphStyleInfo,DocumentNumbering,CaptionPositionInfo,FontInfo,TexInfo).This is powered by the amber library, made ad-hoc for this specific use case.
The entry point to mutate the document info and metadata is now by overwriting
MutableContext#documentInfo.