fix: designspace saves preserve project structure with atomic writes#123
Merged
Conversation
Source now carries a role (Master vs Layer) and an optional UFO layer binding. The UFO reader marks synthesized non-default-layer sources as layer-only, the designspace reader records each <source layer="..."> binding, and the UFO writer returns layer-bound data to the layer it came from instead of the source name.
…writes
The designspace writer previously collapsed every project into a single
{stem}.ufo with all non-default sources as layers, restructuring
multi-UFO projects and leaving the original master UFOs stale on disk.
Sources are now grouped by their stored filename and written back to
the UFO files they came from, mirroring the declared layer shape;
masters created inside Shift get deterministic collision-safe
filenames. Layer-only sources are written as UFO layers without
gaining <source> entries.
The designspace XML itself now goes through a shared crash-safe helper
(temp file + fsync + rename + platform-gated parent fsync, extracted
from the UFO writer) instead of a direct overwrite, and is always
written after the UFO data so the pointer file never references
half-written masters.
…otion Covers the multi-UFO round trip (same UFO filenames, source-to-file- to-layer assignment, locations, and geometry after reopen), that a failed save leaves the existing designspace XML intact, and that plain UFO extra layers stay UFO layers instead of becoming designspace masters, with and without axes.
The master-vs-layer distinction and designspace layer bindings now survive both persistence layers: the SQLite store gains a layer_name column and writes the real source kind instead of hardcoding 'master', and .shift's sources.json carries role and layerName. Both default to Master/absent so existing data reads unchanged.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #119 (retarget to main after it merges). Closes the designspace save-safety cluster from the format-interop review.
The data destroyer, fixed
Saving a multi-UFO designspace previously restructured the project into a single UFO with layers, leaving the real master UFOs stale on disk. The writer now groups sources by their stored filenames (the reader was already recording them) and writes each source back to the UFO it came from — designspace-declared layer-masters stay layers of their original file. Verified by a round-trip test that saves under a different stem and asserts the identical file set, source→file→layer assignment, locations, and per-source geometry after reopen. Shift-created sources without filenames get deterministic generated names (
{stem}-{style}.ufo, collision-safe).Atomic pointer file, data-before-pointer
The
.designspaceXML now goes through a sharedwrite_file_atomichelper (tempfile → sync_all → rename → parent fsync, with #117's unix gating), extracted intoshift-backends/src/atomic.rsalongsidesync_parent. All UFOs are written (each atomically) before the XML; the documented mid-save failure state is: old XML byte-identical, every referenced UFO either its previous revision or a completed new one, aborted new UFOs unreferenced and harmless.No phantom masters
Plain-UFO extra layers (backgrounds, sketches) were being promoted to designspace
<source>entries with empty locations on save. The IR now distinguishes them:Sourcegainsrole: SourceRole(Master/Layer) andlayer_name, the readers set them appropriately, and the writer emits<source>entries only for masters — layer-only sources are written as UFO layers. The distinction persists through both native stores (SQLitesources.layer_name+ kind'layer';.shiftSourceDoc.role/layerName, serde defaults so existing data reads as Master).Known remaining gap → #122
The designspace reader still drops per-master content it doesn't model (companion kerning/fontinfo/libs, undeclared layers, companion-only glyphs); on save companions receive the font-level payload. Tracked in #122 — per-source kerning modeling is the real fix and is on the production-parity path.
Tests: multi-UFO round-trip, XML-atomicity failure test, two no-promotion tests, atomic helper units,
.shift+ SQLite role round-trips.cargo test --workspace: 311 passed, 0 failed; clippy zero warnings.🤖 Generated with Claude Code