diff --git a/docs/tutorials/first-model.md b/docs/tutorials/first-model.md index 139dade..2d5a776 100644 --- a/docs/tutorials/first-model.md +++ b/docs/tutorials/first-model.md @@ -201,14 +201,14 @@ cat office.idf ```idf Zone, - Open Plan, !- Name - , !- Direction of Relative North - , !- X Origin - , !- Y Origin - , !- Z Origin - , !- Type - 1, !- Multiplier - 2.7; !- Ceiling Height + Open Plan, !- Name + , !- Direction of Relative North + , !- X Origin + , !- Y Origin + , !- Z Origin + , !- Type + 1, !- Multiplier + 2.7; !- Ceiling Height ``` Your zone is called `Open Plan`, and so is the wall's `Zone Name` further down diff --git a/packages/core/package.json b/packages/core/package.json index 096b241..59876f5 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -42,8 +42,8 @@ "node": ">=20" }, "idfkit": { - "conformance": "conformance-2026.10", - "governance": "governance-2026.14" + "conformance": "conformance-2026.11", + "governance": "governance-2026.15" }, "dependencies": { "@idfkit/schemas": "0.0.0" diff --git a/packages/core/src/conformance.ts b/packages/core/src/conformance.ts index f946424..092f732 100644 --- a/packages/core/src/conformance.ts +++ b/packages/core/src/conformance.ts @@ -17,4 +17,4 @@ * This is not a version number and it is not compared to one. Two installed libraries agree on the * formats when they declare the same level, whatever their own versions say (FR-025). */ -export const CONFORMANCE_LEVEL = 'conformance-2026.10'; +export const CONFORMANCE_LEVEL = 'conformance-2026.11'; diff --git a/packages/core/src/document.ts b/packages/core/src/document.ts index 19489fa..c44d226 100644 --- a/packages/core/src/document.ts +++ b/packages/core/src/document.ts @@ -1,10 +1,13 @@ import type { Schema, SlimType } from '@idfkit/schemas'; import { IdfCollection } from './collection.js'; -import { DATA, KEY, NAME, OWNER, SHAPE, SOURCE } from './internal.js'; +import { DATA, KEY, NAME, ORIGIN, OWNER, SHAPE, SOURCE } from './internal.js'; import { IdfObject, type FieldValues, type ObjectOwner, type StoredValue } from './object.js'; -import type { PreservedSource } from './preserve/source.js'; +import { isUntouched, originOf, type PreservedSource } from './preserve/source.js'; +import { derivedOf, renderStatement } from './preserve/write.js'; import { ReferenceGraph } from './references.js'; +import type { Region } from './syntax/region.js'; +import { preservingOptions, type WriteIdfOptions } from './write/idf.js'; import type { AnyTypeMap, ObjectOf, TypeNameOf, UntypedMap, ValuesOf } from './typemap.js'; /** @@ -301,6 +304,137 @@ export class IdfDocument implements ObjectOwn for (const collection of this.#collections.values()) yield* collection; } + /** + * Every object a preserving write will write afresh rather than reproduce. + * + * Empty for a document read with `preserveFormatting` and not edited since. Every object for a + * document read without it, because there is nothing to reproduce. + * + * `rawText` answers whether a write will preserve at all. This answers how many objects it will + * REWRITE, and it is the part a consumer cannot work out for itself: a rename clears the record on + * every object that referred to the renamed one, so counting from your own edit log reports one + * where the answer is nine. + * + * **It is not "everything that will differ", and a removal is the case that separates the two.** + * An object removed from the document is no longer in it to be yielded, so this can return nothing + * for a write that changes the file. A consumer treating an empty result as "the file is + * unchanged" would be wrong on every removal. To ask whether the file will differ at all, compare + * the write with `rawText`; ask this for how much of it is being written afresh. + * + * A generator, so listing what is about to be reformatted is as easy as counting it: + * + * ```ts + * const changed = [...document.changedObjects()]; + * if (changed.length > 0) warn(`Saving will rewrite ${changed.length} objects.`); + * ``` + */ + *changedObjects(): Generator { + for (const obj of this.objects()) { + if (!isUntouched(obj, this.#source)) yield obj; + } + } + + /** + * Where an object's characters sit in {@link rawText}, or `undefined` if they sit nowhere. + * + * `undefined` for an object added since the read, for a document read without preservation, and + * for one read from the object notation, which has no statements to point at and preserves + * all-or-nothing. + * + * This is what makes {@link changedObjects} usable. Turning an edit into the smallest possible + * change to a file takes three things: WHICH objects will be rewritten, WHAT text each becomes, + * and WHERE the old one was. Without the third a consumer has to write the whole file and diff + * it, which is the work `changedObjects` exists to avoid. + * + * The range is where the object WAS, and stays answerable after it changes. That is the case it + * is for: the objects worth locating are the ones being rewritten. + * + * ```ts + * for (const obj of document.changedObjects()) { + * const at = document.regionOf(obj); + * if (at === undefined) continue; // added since the read; there is no old text to replace + * edits.push({ range: at, newText: replacementFor(obj) }); + * } + * ``` + * + * **Where the replacement text comes from is not settled by this method, and `writeObject` is + * not the answer.** A preserving write hands that function the author's own per-field comments, + * which are internal, so calling it with options built by hand produces text that differs from + * what {@link writeIdf} would have produced for the same object: the author's units and notes + * come back as generated labels. A consumer that needs the two to agree has to take the whole + * file from `writeIdf`. This method locates the edit; producing its text for a single object is + * a gap that is open, and it is recorded rather than papered over. + * + * The end of the range is the WRITER's, which is not always the semicolon: a comment on the + * terminator's own line is that statement's last field's comment and a preserving write replaces + * it. A range that stopped at the semicolon would leave it behind, describing a field that had + * just moved. + * + * Offsets, not a line and column: `Region` carries the conversion, and a consumer that wants one + * has the text to compute it from, while going the other way costs a scan. + */ + regionOf(obj: IdfObject): Region | undefined { + const source = this.#source; + const at = originOf(obj, source); + if (source === undefined || at === undefined) return undefined; + // The object notation records one anchor per object and no statement, so there is nothing here + // to point at. Preservation is all-or-nothing there and a per-object range would be a fiction. + const statement = source.layer.statements[at]; + if (statement === undefined) return undefined; + // The END is the writer's, not the statement's. A comment on the terminator's own line is that + // statement's last field's comment and the preserving write replaces it; a range stopping at + // the semicolon would leave it behind, on a line describing a field that had just moved. + return { + start: statement.region.start, + end: derivedOf(source).ends[at] ?? statement.region.end, + }; + } + + /** + * One object, rendered exactly as a preserving write would render it. + * + * The text that belongs in the range {@link regionOf} returns, so the two compose into an edit + * that leaves the file byte for byte where {@link writeIdf} would have left it. `undefined` for + * an object the retained source does not hold, which is the same set `regionOf` declines. + * + * ```ts + * for (const obj of document.changedObjects()) { + * const at = document.regionOf(obj); + * const text = document.renderObject(obj); + * if (at === undefined || text === undefined) continue; // added since the read + * edits.push({ range: at, newText: text }); + * } + * ``` + * + * `writeObject` is not this, and that is the reason this exists. A preserving write hands that + * function the author's own per-field annotations, which are internal, so calling it with options + * built by hand comes back with the author's units and notes as generated labels: `!- North Axis + * {deg}` becomes `!- North Axis`. That is a unit lost from an engineering model by an editor + * asked to save a file, and no doc comment is a good enough guard against it. + * + * `fieldComments` is the ONLY option, because it is the only one a preserving write honours. + * `indent`, `commentColumn`, `ordering` and `versionFirst` are refused by {@link writeIdf} + * alongside `preserveFormatting`, and `comments: false` and `compressed` defeat preservation and + * send the whole document down the formatting path instead. Accepting any of them here would let + * a caller render one object on terms the surrounding file was not written on, which is the exact + * divergence this method exists to prevent. + * + * No trailing line break: the range this fills ends at the terminator, or at the comment on that + * line, and the break after it is the first character of what separates one object from the next, + * which a preserving write leaves in place. + */ + renderObject( + obj: IdfObject, + options: Pick = {} + ): string | undefined { + const source = this.#source; + const at = originOf(obj, source); + if (source === undefined || at === undefined) return undefined; + // The options `writeIdf` resolves for its preserving branch, resolved by the same function, so + // the two cannot disagree about the bytes. + return renderStatement(source, at, preservingOptions(options)); + } + /** Reference targets that no object provides. */ danglingReferences(): ReturnType { const valid = new Set(); diff --git a/packages/core/src/internal.ts b/packages/core/src/internal.ts index e2d41fc..f9bc981 100644 --- a/packages/core/src/internal.ts +++ b/packages/core/src/internal.ts @@ -39,3 +39,19 @@ export const KEY = Symbol('idfkit.key'); * new value and back again is unchanged by comparison and touched in truth. */ export const SOURCE = Symbol('idfkit.source'); +/** + * Which statement an object was READ from, kept whether or not it has since changed. + * + * `SOURCE` is cleared the moment an object is touched, because its absence is what marks the + * object as needing to be rewritten. That makes it useless for saying where the old characters + * were, which is exactly the question a consumer building a minimal edit has to ask about a + * CHANGED object. This is the same number, recorded once and never cleared. + * + * Deliberately untagged, as `SOURCE` above is. `stripInternal` drops a declaration whose JSDoc + * carries the tag, and `object.ts` declares a property keyed on this symbol, so tagging it emits a + * `.d.ts` that references a symbol its own module no longer declares and the build stops. Being + * absent from the package index is what makes this module internal, and that part holds. + * + * The tag is matched as TEXT anywhere in the comment, so naming it here would strip this too. + */ +export const ORIGIN = Symbol('idfkit.origin'); diff --git a/packages/core/src/object.ts b/packages/core/src/object.ts index 1fccf82..13d44cb 100644 --- a/packages/core/src/object.ts +++ b/packages/core/src/object.ts @@ -1,7 +1,7 @@ import type { SlimField, SlimType } from '@idfkit/schemas'; import { ExtensibleList } from './extensible.js'; -import { DATA, KEY, NAME, OWNER, SHAPE, SOURCE } from './internal.js'; +import { DATA, KEY, NAME, ORIGIN, OWNER, SHAPE, SOURCE } from './internal.js'; import { shapeFor, type ObjectShape } from './shape.js'; /** A scalar field value. `undefined` means the field is absent. */ @@ -56,6 +56,7 @@ export class IdfObject { declare [KEY]: string; /** Index into the document's preserved anchors, or `undefined` once anything has changed this. */ declare [SOURCE]: number | undefined; + declare [ORIGIN]: number | undefined; /** * Objects are built through `IdfObject.create`, never `new`, because each @@ -83,6 +84,7 @@ export class IdfObject { // it exists (FR-007). `clone` builds through here too, which is why a copy is touched as well: // it is a different object from the one the characters describe. Object.defineProperty(obj, SOURCE, { value: undefined, writable: true }); + Object.defineProperty(obj, ORIGIN, { value: undefined, writable: true }); for (const [field, value] of Object.entries(values)) { if (value === undefined || value === null) continue; diff --git a/packages/core/src/parse/idf.ts b/packages/core/src/parse/idf.ts index d337687..d8752b5 100644 --- a/packages/core/src/parse/idf.ts +++ b/packages/core/src/parse/idf.ts @@ -1,7 +1,7 @@ import type { Schema, SlimType } from '@idfkit/schemas'; import { IdfDocument } from '../document.js'; -import { SOURCE } from '../internal.js'; +import { ORIGIN, SOURCE } from '../internal.js'; import type { ExtensibleGroup, FieldValues, IdfObject, StoredValue } from '../object.js'; import { statementIndexes } from '../preserve/source.js'; import { layerCollector } from '../syntax/layer.js'; @@ -143,6 +143,9 @@ export function parseIdf( if (at !== undefined) { anchors[at] = built; built[SOURCE] = at; + // The same number, kept past the first edit. `SOURCE` goes when the object is touched, + // which is what marks it for rewriting; this one answers where its characters WERE. + built[ORIGIN] = at; } // Reported after the object is built, never instead of building it: a value of the wrong diff --git a/packages/core/src/preserve/source.ts b/packages/core/src/preserve/source.ts index ecd278d..30c5568 100644 --- a/packages/core/src/preserve/source.ts +++ b/packages/core/src/preserve/source.ts @@ -1,4 +1,4 @@ -import { SOURCE } from '../internal.js'; +import { ORIGIN, SOURCE } from '../internal.js'; import type { IdfObject } from '../object.js'; import type { RawObject } from '../parse/lexer.js'; import type { SyntaxLayer } from '../syntax/layer.js'; @@ -77,6 +77,25 @@ export function isWholeDocumentUntouched( return true; } +/** + * The statement an object was read from, or `undefined` if this source did not read it. + * + * The identity check is the one `isUntouched` makes below, for the same reason: an object carrying + * an index from a file it is no longer in would otherwise be answered from this one. It is stated + * once here because `regionOf` and `renderObject` both decline exactly this set, and a rule two + * methods share is a rule one of them will eventually be fixed without. + * + * Reads `ORIGIN`, not `SOURCE`: the question is where the characters WERE, which stays answerable + * after the object changes, and changing it is what clears `SOURCE`. + * + * @internal + */ +export function originOf(obj: IdfObject, source: PreservedSource | undefined): number | undefined { + if (source === undefined) return undefined; + const at = obj[ORIGIN]; + return at !== undefined && source.anchors[at] === obj ? at : undefined; +} + export function isUntouched(obj: IdfObject, source: PreservedSource | undefined): boolean { if (source === undefined) return false; const at = obj[SOURCE]; diff --git a/packages/core/src/preserve/write.ts b/packages/core/src/preserve/write.ts index 0f0cd64..ccbd284 100644 --- a/packages/core/src/preserve/write.ts +++ b/packages/core/src/preserve/write.ts @@ -1,6 +1,6 @@ import { OWNER } from '../internal.js'; import type { IdfObject } from '../object.js'; -import { writeObject, type ObjectWriteOptions } from '../write/idf.js'; +import { writeObject, type FieldAnnotation, type ObjectWriteOptions } from '../write/idf.js'; import { isUntouched, type PreservedSource } from './source.js'; /** @@ -12,10 +12,10 @@ import { isUntouched, type PreservedSource } from './source.js'; * is what makes the one-object diff structural rather than careful, and it is why nothing is * reordered, no header is added and the version statement is not moved. * - * A statement's region ends at its terminator, so a comment trailing the semicolon on the same - * line is in the gap: removing an object leaves it, and reformatting one leaves it below the new - * text. Behaviour rather than defects, and the alternative is a writer that guesses which comments - * are about which object. + * A statement's region ends at its terminator, and a comment after that semicolon on the SAME LINE + * is absorbed into the statement rather than left in the gap. See {@link extentEnds}. A comment on + * the next line, or after a blank one, stays in the gap, which is where the guessing problem about + * which object a comment belongs to actually starts. * * @internal */ @@ -26,6 +26,7 @@ export function writePreserved( ): string { const text = source.layer.text; const statements = source.layer.statements; + const ends = derivedOf(source).ends; const parts: string[] = []; // Everything before the first statement, which for a file with none is the whole text: an empty @@ -33,12 +34,9 @@ export function writePreserved( parts.push(text.slice(0, statements[0]?.region.start ?? text.length)); for (let index = 0; index < statements.length; index += 1) { - const statement = statements[index]!; - parts.push(statementPart(source, index, text, options)); + parts.push(statementPart(source, index, text, ends[index]!, options)); // Unconditional: the gap is emitted whether the statement was copied, reformatted or dropped. - parts.push( - text.slice(statement.region.end, statements[index + 1]?.region.start ?? text.length) - ); + parts.push(text.slice(ends[index]!, statements[index + 1]?.region.start ?? text.length)); } appendNewObjects(document, source, parts, options); @@ -54,17 +52,48 @@ function statementPart( source: PreservedSource, index: number, text: string, + end: number, options: ObjectWriteOptions ): string { const statement = source.layer.statements[index]!; - const verbatim = text.slice(statement.region.start, statement.region.end); + const verbatim = text.slice(statement.region.start, end); const anchored = source.anchors[index]; if (anchored === undefined) return verbatim; // Removal is answered from ownership: `remove` already clears the owner, and recording it on // the object would mean holding a reference to something the document has let go. if (anchored[OWNER] === undefined) return ''; if (isUntouched(anchored, source)) return verbatim; - return writeObject(anchored, options); + return renderStatement(source, index, options); +} + +/** + * One statement's object, rendered the way this walk renders it. + * + * Factored out rather than inlined because `IdfDocument.renderObject` has to produce exactly this + * text: a consumer splicing something else into the range `regionOf` returns gets a file that + * differs from `writeIdf`, silently. Two copies of this would be two answers to one question. + * + * Without a trailing newline. A statement's extent ends at its terminator, or at the comment on + * that same line, and in neither case does it include the line break: the break is the first + * character of the gap. `writeObject` ends with one because it is also used to write whole + * documents, so emitting it here would put the break in twice and grow the file by a blank line + * per reformatted object. Every object in a file, edited and saved twice, would grow it twice. + * + * @internal + */ +export function renderStatement( + source: PreservedSource, + index: number, + options: ObjectWriteOptions +): string { + // Re-render the VALUES, and keep the author's comments. An edit asks for the first and never for + // the second, and rebuilding a comment destroys whatever the schema cannot regenerate: a note to + // a colleague, and the field's unit, which the ordinary label does not carry. + const written = writeObject(source.anchors[index]!, { + ...options, + annotations: annotations(source, index), + }); + return written.endsWith('\n') ? written.slice(0, -1) : written; } /** @@ -100,3 +129,166 @@ function lastNonEmpty(parts: readonly string[]): string { } return ''; } + +/** + * The two facts about each statement that are derived from the token stream, computed together. + * + * `ends` is where a statement's text ends FOR THE WRITER, which is not always its terminator. A + * comment after the semicolon with nothing but horizontal whitespace between them is the last + * field's comment. Leaving it in the gap is invisible while the statement is copied, because the + * gap is copied too, and wrong the moment it is reformatted: the writer emits its own field comment + * and the author's then arrives from the gap on the line below, so the output carries a line nobody + * wrote. It is not even a duplicate, because the ordinary writer drops the unit the original + * usually carries, so it reads as a stray fragment. + * + * That is not the writer guessing which comment belongs to which object. "On the same line as the + * terminator" is a positional fact, and it is the one case where the owner is not in question. + * + * Three consequences, decided rather than discovered: + * + * - A file written unchanged is byte-identical still. The extent grows and the gap shrinks by + * exactly the same characters, so the concatenation does not move. + * - Removing an object takes that comment with it, where it used to be left on a line of its own + * describing a field that no longer exists. + * - Reformatting replaces it, which is the defect this closes. + * + * `firstToken` is where each statement's tokens begin. `annotations` used to start its cursor at + * zero and seek forward, which is a full prefix scan of the token stream per statement: quadratic + * in the file, and invisible to the benchmarks because they time an UNCHANGED write, where no + * statement is reformatted and `annotations` is never reached. On a ten thousand statement model + * that is the difference between milliseconds and seconds. + * + * Both come from one pass with monotone cursors, because the statements and the tokens are both in + * source order. Memoised per retained source, which never changes after the read, so the walk and + * the two document accessors share one answer rather than each deriving it. + */ +interface Derived { + /** Where each statement's text ends for the writer. */ + readonly ends: readonly number[]; + /** The first token at or after each statement's type name, as a cursor for `annotations`. */ + readonly firstToken: readonly number[]; +} + +const derived = new WeakMap(); + +/** + * The derived facts for one retained source, computed once. + * + * Exported so `IdfDocument.regionOf` answers with the SAME extent this walk replaces. Handing a + * consumer `statement.region` instead would stop short of the terminator-line comment, and an edit + * built on it would leave that comment behind, which is the defect the extent exists to close. + */ +export function derivedOf(source: PreservedSource): Derived { + let found = derived.get(source); + if (found === undefined) { + found = compute(source); + derived.set(source, found); + } + return found; +} + +function compute(source: PreservedSource): Derived { + const { statements, tokens, text } = source.layer; + const ends = statements.map((statement) => statement.region.end); + const firstToken: number[] = new Array(statements.length); + + // Two cursors rather than one, because they track different points and both only move forward. + let atStatement = 0; + let atExtent = 0; + for (let index = 0; index < statements.length; index += 1) { + const statement = statements[index]!; + + while (atStatement < tokens.length && tokens.startAt(atStatement) < statement.typeName.end) { + atStatement += 1; + } + firstToken[index] = atStatement; + + const end = ends[index]!; + while (atExtent < tokens.length && tokens.startAt(atExtent) < end) atExtent += 1; + if (atExtent >= tokens.length || tokens.kindAt(atExtent) !== 'comment') continue; + + // Horizontal whitespace only. A line feed between the two puts the comment on its own line, + // which makes it a comment about whatever comes next and none of this statement's business. + const between = text.slice(end, tokens.startAt(atExtent)); + if (between.includes('\n') || between.trim() !== '') continue; + ends[index] = tokens.endAt(atExtent); + } + return { ends, firstToken }; +} + +/** + * What the author wrote around each of a statement's fields, positionally. + * + * Two kinds, and the second is the one nothing else carries. A field's own comment is the one after + * its delimiter on the same line, which is the convention every writer of these files follows and + * the only case where which field a comment belongs to is not a guess. A comment on its OWN line + * inside the statement belongs to the field below it, and it is lost the moment the object is + * reformatted unless it is emitted with that field: a comment between two statements is carried by + * the gap, and one inside a statement is not. + * + * Positional against `Statement.fields`, which is positional against the cells `writeObject` emits: + * the name first for a named type, then the fixed fields in order. An object that gained a field + * runs past the end of this list, and a field with no entry has no author to be faithful to. + * + * A field the author left bare gets an entry with no `trailing`, which is how "written bare" is + * told apart from "not written by this author at all". Absence is as much a thing the author wrote + * as the words are. + */ +function annotations(source: PreservedSource, index: number): FieldAnnotation[] { + const { statements, tokens, text } = source.layer; + const statement = statements[index]!; + const fields = statement.fields; + // Every entry is assigned in the loop below, so there is nothing to pre-fill: a placeholder + // would be a second, contradictory statement of what an entry defaults to. + const built: FieldAnnotation[] = new Array(fields.length); + + // One cursor over the tokens, which are in source order, as the fields are. Every comment + // between the previous field's delimiter and this one's value stands on its own line above it. + // It starts where this statement starts rather than at zero: seeking from the front of the file + // made a whole-document reformat quadratic in the token count. + let token = derivedOf(source).firstToken[index]!; + let previousEnd = statement.typeName.end; + for (let at = 0; at < fields.length; at += 1) { + const field = fields[at]!; + const before: string[] = []; + + while (token < tokens.length && tokens.startAt(token) < previousEnd) token += 1; + while (token < tokens.length && tokens.startAt(token) < field.start) { + if ( + tokens.kindAt(token) === 'comment' && + !onSameLine(text, previousEnd, tokens.startAt(token)) + ) { + before.push(text.slice(tokens.startAt(token), tokens.endAt(token)).trimEnd()); + } + token += 1; + } + + // Past the value now: step over the delimiter that closes it and take the comment after it. + while (token < tokens.length && tokens.startAt(token) < field.end) token += 1; + while ( + token < tokens.length && + (tokens.kindAt(token) === 'separator' || tokens.kindAt(token) === 'terminator') + ) { + token += 1; + } + let trailing: string | undefined; + if ( + token < tokens.length && + tokens.kindAt(token) === 'comment' && + onSameLine(text, field.end, tokens.startAt(token)) + ) { + trailing = text.slice(tokens.startAt(token), tokens.endAt(token)).trimEnd(); + } + + // Whether the author began a line with this field, which is what keeps a vertex written + // `0,0,4.572,` on one line rather than three. + built[at] = { before, trailing, startsLine: !onSameLine(text, previousEnd, field.start) }; + previousEnd = field.end; + } + return built; +} + +/** Whether two offsets sit on one line, which is what makes a comment a field's rather than its own. */ +function onSameLine(text: string, from: number, to: number): boolean { + return !text.slice(from, to).includes('\n'); +} diff --git a/packages/core/src/write/idf.ts b/packages/core/src/write/idf.ts index 2c955a6..d7744d0 100644 --- a/packages/core/src/write/idf.ts +++ b/packages/core/src/write/idf.ts @@ -13,7 +13,13 @@ export interface WriteIdfOptions { */ comments?: boolean; /** - * Column the field-name comments are aligned to. + * Column the field-name comments are aligned to, counting from 1 as an editor does. + * + * The default puts `!-` where EnergyPlus itself puts it. Its own example files write the marker + * at column 30 on 223 of the 231 commented lines of `1ZoneUncontrolled.idf`, and matching that + * is what keeps a rewritten object flush with the untouched objects around it: on a preserving + * write, a column of its own would leave a visible seam at every edit. + * * @defaultValue 30 */ commentColumn?: number; @@ -71,9 +77,30 @@ export interface WriteIdfOptions { * or `comments: false`, which ask for a different output FORM the source was never going to * express, so producing it is honest. * + * Cheapest where it is used: a write that reproduces most of the file copies text rather than + * building it. It CROSSES OVER once most objects have changed, because this path renders each of + * them AND walks the tiling, which is more work than formatting alone. On a 13 MB model with + * every object edited it is roughly four times slower than a formatting write. Nothing to guard + * against, since an edit touches a handful of objects and a whole model rewrite is what + * `preserveFormatting: false` is for, but worth knowing before timing the wrong one. + * * @defaultValue undefined, meaning decide */ preserveFormatting?: boolean; + /** + * What to do about a field the author deliberately left without a comment. + * + * Only meaningful on the preserving path, which is the only path that knows what the author + * wrote. `'preserve'` leaves a bare field bare, because absence is as much a thing the author + * wrote as the words are. `'generate'` labels it, for a caller who wants the file annotated. + * + * Neither setting touches the author's own comment lines. A comment between two objects is + * carried by the text between them, and a comment on its own line inside an object is emitted + * with the field below it, so asking for labels adds them and never costs a line. + * + * @defaultValue 'preserve' + */ + fieldComments?: 'preserve' | 'generate'; } /** @@ -94,19 +121,15 @@ export function writeIdf( ): string { const preserved = decidePreservation(document, options); if (preserved !== undefined) { - return writePreserved(document, preserved, { - comments: options.comments ?? true, - commentColumn: options.commentColumn ?? 30, - indent: options.indent ?? ' ', - }); + return writePreserved(document, preserved, preservingOptions(options)); } const compressed = options.compressed ?? false; // Compressed output has no comments by definition. Asking for both is not an error, because the // narrower request is unambiguous: comments cannot survive a single-line object. const comments = compressed ? false : (options.comments ?? true); - const commentColumn = options.commentColumn ?? 30; - const indent = options.indent ?? ' '; + const commentColumn = options.commentColumn ?? DEFAULT_COMMENT_COLUMN; + const indent = options.indent ?? DEFAULT_INDENT; const versionFirst = options.versionFirst ?? true; const ordering = options.ordering ?? 'source'; @@ -176,12 +199,97 @@ function decidePreservation( return source; } +/** Where `!-` goes, counted from 1. See {@link WriteIdfOptions.commentColumn}. */ +export const DEFAULT_COMMENT_COLUMN = 30; + +/** The indent before each field line. See {@link WriteIdfOptions.indent}. */ +export const DEFAULT_INDENT = ' '; + +/** + * The options a PRESERVING write resolves, which is the only set a preserved object can be + * rendered on. + * + * `IdfDocument.renderObject` has to produce exactly the bytes this walk produces, so it resolves + * its options here rather than restating the defaults. Restating them is how the two drift: the + * comment column has a reason behind its value, and moving that reason in one place while the + * other kept the old number would break the byte-for-byte agreement `renderObject` promises. + * + * The controls are not read from the caller because a preserving write refuses them. `indent`, + * `commentColumn`, `ordering` and `versionFirst` throw when set alongside `preserveFormatting`, + * and `comments: false` and `compressed` defeat preservation and send the document down the + * formatting path, so on this path they are always their defaults. + */ +export function preservingOptions(options: WriteIdfOptions): ObjectWriteOptions { + return { + comments: true, + commentColumn: DEFAULT_COMMENT_COLUMN, + indent: DEFAULT_INDENT, + labelBareFields: options.fieldComments === 'generate', + }; +} + export interface ObjectWriteOptions { comments: boolean; + /** Where `!-` goes, counted from 1. See {@link WriteIdfOptions.commentColumn}. */ commentColumn: number; indent: string; /** Put the whole object on one line. See `WriteIdfOptions.compressed`. */ compressed?: boolean; + /** + * What the author wrote around each field, positionally. + * + * Supplied by the preserving writer and by nothing else. Re-rendering an object's VALUES is what + * an edit asks for; rebuilding what surrounds them is not, and doing it anyway destroys anything + * the schema cannot regenerate. + * + * Positional, and shorter than the cells whenever the object gained fields. A field past the end + * has no author to be faithful to, so it is labelled as it always was. + * + * @internal + */ + annotations?: readonly FieldAnnotation[]; + /** + * Label a field the author deliberately left bare. + * + * `false` is faithful and is what the preserving path asks for: a field written without a comment + * was written that way on purpose, and absence is as much a thing the author wrote as the words + * are. `true` restores the ordinary writer's behaviour of labelling every field, for a caller who + * wants the file annotated. + * + * Either way the author's own standalone comment lines are emitted, so turning this on adds + * labels and never costs a line. + * + * @internal + */ + labelBareFields?: boolean; +} + +/** + * What the author wrote around one field. + * + * @internal + */ +export interface FieldAnnotation { + /** + * Comment lines standing on their own above this field, in order, exactly as written. + * + * These live INSIDE the statement, so unlike a comment between two statements they are not + * carried by the gap and are lost the moment the object is reformatted unless they are emitted + * here. `! this value came from the 2019 survey` is the shape, and it is the author's note about + * the field below it. + */ + readonly before: readonly string[]; + /** The comment after this field's delimiter on the same line, if the author wrote one. */ + readonly trailing: string | undefined; + /** + * Whether the author began a new line with this field. + * + * False for the second and third coordinate of a vertex written `0,0,4.572,` on one line. Writing + * one value per line regardless turns a four-line surface into twelve, which is the most visible + * thing a reformat does to a geometry file: 21.5 percent of the statements in the 693 EnergyPlus + * example files group values this way, and 690 of those files contain at least one. + */ + readonly startsLine: boolean; } /** Serialize one object. */ @@ -201,7 +309,25 @@ export function writeObject(obj: IdfObject, options: ObjectWriteOptions): string // and a run of bare commas is noise. But IDF is positional: if extensible // groups follow, every fixed slot must be emitted or the groups land one // field early and each value is read into the wrong slot on the way back in. - const lastFixed = groups.length > 0 ? fixed.length - 1 : lastSetIndex(obj, fixed); + // + // The annotations are the third case, and the reason is the one that governs this whole path: a + // field the author WROTE OUT as a blank is as much a thing the author wrote as a field left + // bare of its comment. Dropping it takes the author's `!- Cooling Design Capacity Method` with + // it, so a single-field edit shortens one Sizing:System from 38 lines to 22. Across the 693 + // example files that is 20,571 lines, more than any other difference a rewrite makes. + // + // Only on this path. A write with no author behind it keeps trimming, as it always has. + // + // Counted as an INDEX into the fixed fields, with the name subtracted out, because the + // annotations lead with the name. Python states the same rule as a count of emitted values with + // the name included. Both are right against their own annotations and neither would notice if + // the other's convention moved, so a change to either belongs in both. + const authored = options.annotations?.length ?? 0; + const lastAuthored = authored - (obj.isNamed ? 1 : 0) - 1; + const lastFixed = + groups.length > 0 + ? fixed.length - 1 + : Math.min(fixed.length - 1, Math.max(lastSetIndex(obj, fixed), lastAuthored)); for (let i = 0; i <= lastFixed; i += 1) { const field = fixed[i]!; cells.push({ value: formatValue(definition, field, obj.get(field)), label: humanize(field) }); @@ -231,18 +357,60 @@ export function writeObject(obj: IdfObject, options: ObjectWriteOptions): string return `${obj.typeName},${cells.map((cell) => cell.value).join(',')};`; } - const lines: string[] = [`${obj.typeName},`]; + const lines: string[] = []; + + // A line under construction, so fields the author wrote together stay together. Flushed when the + // next field opens a line of its own, and once at the end. + // + // It starts as the TYPE NAME rather than the type name being pushed straight out, so that an + // object the author wrote entirely on one line, `Timestep,4;`, can come back on one line. That is + // 11.3 percent of the statements in the example files, and it is the case that surprises on a + // file with no geometry in it: nobody thinks of `Timestep,4;` as formatting they chose. + let open = `${obj.typeName},`; + let openComment: string | undefined; + const flush = (): void => { + if (open === '') return; + if (openComment === undefined) { + lines.push(open); + } else { + // Minus one because the option is a COLUMN, counted from 1, and this is an offset into a + // string, counted from 0. Applying it as an offset put every comment one column right of + // where the files being imitated put it. + const padding = ' '.repeat(Math.max(1, options.commentColumn - 1 - open.length)); + lines.push(`${open}${padding}${openComment}`); + } + open = ''; + openComment = undefined; + }; cells.forEach((cell, index) => { const terminator = index === cells.length - 1 ? ';' : ','; - const body = `${options.indent}${cell.value}${terminator}`; - if (!options.comments) { - lines.push(body); - return; + const annotation = options.annotations?.[index]; + // No annotation means no author to be faithful to, so one value per line as this writer always + // did. Field 0 is the one that decides whether the object opens on the type name's own line. + const opensLine = annotation === undefined || annotation.startsLine; + + if (opensLine) { + flush(); + for (const line of annotation?.before ?? []) lines.push(`${options.indent}${line}`); + open = `${options.indent}${cell.value}${terminator}`; + } else { + open = `${open} ${cell.value}${terminator}`; } - const padding = ' '.repeat(Math.max(1, options.commentColumn - body.length)); - lines.push(`${body}${padding}!- ${cell.label}`); + + if (!options.comments) return; + // The author's comment where there is one. Where the author left the field bare, nothing, + // unless the caller asked for a label. Where there is no author at all, the label as always. + // On a line carrying several values only the last has a comment, which is what the author + // wrote and what the delimiter rule recovers. + const comment = + annotation === undefined + ? `!- ${cell.label}` + : (annotation.trailing ?? + (options.labelBareFields === true ? `!- ${cell.label}` : undefined)); + if (comment !== undefined) openComment = comment; }); + flush(); return `${lines.join('\n')}\n`; } diff --git a/packages/core/tests/docs-snippets.test.ts b/packages/core/tests/docs-snippets.test.ts index 121c76f..3eaa780 100644 --- a/packages/core/tests/docs-snippets.test.ts +++ b/packages/core/tests/docs-snippets.test.ts @@ -250,7 +250,7 @@ describe('docs/tutorials/first-model.md', () => { // which is what stops the vertices shifting a field early. const outPath = join(dir, 'office.idf'); await saveIdf(doc, outPath); - expect(readFileSync(outPath, 'latin1')).toContain('Open Plan, !- Name'); + expect(readFileSync(outPath, 'latin1')).toContain('Open Plan, !- Name'); // Step 8: reading it back. const reloaded = await loadIdf(outPath); diff --git a/packages/core/tests/preserve.test.ts b/packages/core/tests/preserve.test.ts index af9bee6..8832bd4 100644 --- a/packages/core/tests/preserve.test.ts +++ b/packages/core/tests/preserve.test.ts @@ -1,6 +1,6 @@ import { beforeAll, describe, expect, it } from 'vitest'; -import { parseEpJson, parseIdf, writeEpJson, writeIdf } from '@idfkit/core'; +import { parseEpJson, parseIdf, scanIdf, writeEpJson, writeIdf, writeObject } from '@idfkit/core'; import type { Schema } from '@idfkit/schemas'; import { schema, syntaxFixture, syntaxFixtures } from './helpers.js'; @@ -200,14 +200,135 @@ describe('one field changes and one object looks changed', () => { // The removed statement's extent goes and the gaps around it do not: the blank line that // separated the two zones is in a gap and belongs to no object. // - // A statement's region ends at its TERMINATOR, so a comment trailing the semicolon on the same - // line is in the gap too, and it survives the removal. That is a consequence of the definition - // of text that belongs to no object, and it is behaviour rather than a defect: the alternative - // is a writer that decides which comments are about which object, which is a guess. - expect(written).toBe(MODEL.replace('Zone,\n Zone Two, !- Name\n 1.0E-5;', '')); + // The comment on the TERMINATOR's line goes with the object (idfkit-js#47). It is the last + // field's comment, and the field no longer exists, so leaving it behind would strand a line + // describing something that is gone. A comment on its own line, or after a blank one, stays: + // that is where deciding which object a comment belongs to becomes a guess. + expect(written).toBe( + MODEL.replace( + 'Zone,\n Zone Two, !- Name\n 1.0E-5; !- Direction of Relative North', + '' + ) + ); + // Zone One still carries its own, on its own terminator line. expect(written).toContain('!- Direction of Relative North'); }); + it("keeps the author's comments on an object it reformats", () => { + // The values are what an edit asks to re-render. The comments are not, and rebuilding them + // destroys whatever the schema cannot regenerate: a note to a colleague, and the field's unit, + // which the generated label does not carry. Both are kept, in place, exactly as written. + const text = [ + 'Version, 26.1;', + '', + 'Building,', + ' My Building, !- Name', + ' 0, !- North Axis {deg}', + ' City; !- VERIFY WITH CLIENT before the Feb review', + '', + ].join('\n'); + const { document } = parseIdf(text, v26, { strict: false, preserveFormatting: true }); + document.require('Building', 'My Building').set('north_axis', 42); + + const written = writeIdf(document); + + expect(written).toContain('!- North Axis {deg}'); + expect(written).toContain('!- VERIFY WITH CLIENT before the Feb review'); + // Once each, not twice: the writer emits the author's comment in place of its own, so there is + // nothing left in the gap to arrive on the line below (idfkit-js#47). + expect(written.match(/!- North Axis/g)).toHaveLength(1); + expect(written.match(/VERIFY WITH CLIENT/g)).toHaveLength(1); + // The value is the one thing that did change. + expect(written).toContain('42.0'); + expect(written).not.toContain(' 0,'); + }); + + it('adds no line to the file for an object it reformats', () => { + // A statement's extent ends at its terminator, or at the comment on that same line, and never + // includes the line break: the break is the first character of the gap. `writeObject` ends with + // one because it also writes whole documents, so emitting it here put the break in twice and + // grew the file by a blank line per reformatted object — compounding on every save. + // + // It was there before the comment work and was invisible: the misplaced terminator comment sat + // in the gap between the two breaks, so it read as one blank line. Fixing that exposed this. + const text = [ + 'Version, 26.1;', + '', + 'Building,', + ' My Building, !- Name', + ' 0; !- North Axis {deg}', + '', + 'Timestep, 6;', + '', + ].join('\n'); + const { document } = parseIdf(text, v26, { strict: false, preserveFormatting: true }); + document.require('Building', 'My Building').set('north_axis', 42); + + const written = writeIdf(document); + + expect(written.split('\n')).toHaveLength(text.split('\n').length); + expect(written).not.toContain('\n\n\n'); + // And again, on the output, because the growth compounded rather than saturating. + const reread = parseIdf(written, v26, { strict: false, preserveFormatting: true }).document; + reread.require('Building', 'My Building').set('north_axis', 43); + expect(writeIdf(reread).split('\n')).toHaveLength(text.split('\n').length); + }); + + it('generates a comment only for a field the author never wrote one for', () => { + const text = ['Version, 26.1;', '', 'Building,', ' My Building;', ''].join('\n'); + const { document } = parseIdf(text, v26, { strict: false, preserveFormatting: true }); + document.require('Building', 'My Building').set('north_axis', 42); + + expect(writeIdf(document)).toContain('!- North Axis'); + }); + + it('does not leave the old terminator comment below a reformatted object', () => { + // idfkit-js#47. A statement's region ends at its terminator, so a comment after the semicolon + // on the same line used to sit in the gap. Invisible while the object is copied, because the + // gap is copied too; wrong the moment it is reformatted, because the writer emits its own + // field comment and the author's then arrives on the line below. + // + // It is not even a duplicate: the ordinary writer drops the unit, so `!- North Axis {deg}` + // reads as a stray fragment under `!- North Axis`. IDFEditor writes one of these on every + // line of every object, so about half the statements in a typical file were affected. + const text = [ + 'Version, 26.1;', + '', + 'Building,', + ' My Building, !- Name', + ' 0; !- North Axis {deg}', + '', + ].join('\n'); + const { document } = parseIdf(text, v26, { strict: false, preserveFormatting: true }); + document.require('Building', 'My Building').set('north_axis', 42); + + const written = writeIdf(document); + + // Once, in place, with the author's unit intact. It used to arrive a second time from the gap. + expect(written.match(/!- North Axis/g)).toHaveLength(1); + expect(written).toContain('!- North Axis {deg}'); + }); + + it('leaves a comment on its own line where it is', () => { + // The boundary of the rule above. Only the terminator's own line is absorbed; a comment on the + // next line is about whatever follows it and is nobody's to move. + const text = [ + 'Version, 26.1;', + '', + 'Building,', + ' My Building, !- Name', + ' 0;', + '! a note about what comes next', + '', + 'Timestep, 6;', + '', + ].join('\n'); + const { document } = parseIdf(text, v26, { strict: false, preserveFormatting: true }); + document.require('Building', 'My Building').set('north_axis', 42); + + expect(writeIdf(document)).toContain('! a note about what comes next'); + }); + it('appends a new object at the end, formatted', () => { const document = read(); document.add('Zone', 'Zone Three'); @@ -406,6 +527,9 @@ describe('asking for two contradictory things is refused', () => { const bare = writeIdf(kept(), { preserveFormatting: true, comments: false }); expect(bare).not.toBe(MODEL); expect(bare).not.toContain('!-'); + // And it still loads. The type name is the line the field loop starts with, so a branch that + // returns before flushing it emits it last, which parses as nothing at all. + expect(() => parseIdf(bare, v26, { strict: false })).not.toThrow(); }); it('raises nothing when preservation is asked for on a document read without it', () => { @@ -479,3 +603,415 @@ describe('the object notation preserves on all-or-nothing terms', () => { expect(written).toContain('Zone,'); }); }); + +describe('which objects a preserving write will rewrite', () => { + // A consumer cannot derive this from its own edit log. A rename clears the record on every object + // that referred to the renamed one, so an editor counting its own edits reports one where the + // answer is three here and nine on a real model. + const REFERENCED = [ + 'Version, 26.1;', + '', + 'Zone, ZONE ONE;', + '', + 'BuildingSurface:Detailed,', + ' S1, Wall, C1, ZONE ONE, , Outdoors, , SunExposed, WindExposed, , ,', + ' 0,0,0, 1,0,0;', + '', + 'BuildingSurface:Detailed,', + ' S2, Wall, C1, ZONE ONE, , Outdoors, , SunExposed, WindExposed, , ,', + ' 0,0,0, 1,0,0;', + '', + ].join('\n'); + + const read = (preserve = true) => + parseIdf(REFERENCED, v26, { strict: false, preserveFormatting: preserve }).document; + + it('yields nothing for a document nobody has edited', () => { + expect([...read().changedObjects()]).toEqual([]); + }); + + it('yields every object a rename rewrote, not just the renamed one', () => { + const document = read(); + document.rename(document.require('Zone', 'ZONE ONE'), 'RENAMED'); + + expect([...document.changedObjects()].map((o) => o.typeName).sort()).toEqual([ + 'BuildingSurface:Detailed', + 'BuildingSurface:Detailed', + 'Zone', + ]); + }); + + it('yields nothing after a write of the value already held', () => { + const document = read(); + const zone = document.require('Zone', 'ZONE ONE'); + zone.set('multiplier', zone.get('multiplier') ?? undefined); + + expect([...document.changedObjects()]).toEqual([]); + }); + + it('yields every object for a document read without preservation', () => { + // There is nothing to reproduce, so a write rewrites the file entirely. + const document = read(false); + + expect([...document.changedObjects()]).toHaveLength(document.size); + }); + + it('agrees with what the writer actually reproduces', () => { + // The claim is only worth making if the writer honours it. After the rename, the three objects + // it touched are named and the Version is not, so the Version must come back from its own + // characters and the other three must not. + const document = read(); + document.rename(document.require('Zone', 'ZONE ONE'), 'RENAMED'); + const written = writeIdf(document); + const changed = [...document.changedObjects()]; + + expect(changed.map((o) => o.typeName)).not.toContain('Version'); + expect(written).toContain('Version, 26.1;'); + // And every object it DID name was rewritten: none of them survives as its original text. + expect(written).not.toContain('Zone, ZONE ONE;'); + expect(changed).toHaveLength(3); + }); +}); + +describe('what survives when the writer rewrites an object', () => { + // An edit asks for the VALUES to be re-rendered. Everything else the author wrote is theirs, and + // that includes the absence of a comment on a field they left bare. + const SOURCE = [ + 'Version, 26.1;', + '', + '! a note between objects', + 'Building,', + ' My Building, !- Name', + ' ! this value came from the 2019 survey', + ' 0, !- North Axis {deg}', + ' Suburbs;', + '', + 'Timestep, 6;', + '', + ].join('\n'); + + const edited = (options = {}) => { + const { document } = parseIdf(SOURCE, v26, { strict: false, preserveFormatting: true }); + document.require('Building', 'My Building').set('north_axis', 42); + return writeIdf(document, options); + }; + + it('keeps a units annotation the generated label would drop', () => { + expect(edited()).toContain('!- North Axis {deg}'); + }); + + it('keeps a comment on its own line inside the object', () => { + // The one comment nothing else carries: it is inside the object rather than between two, so + // the gap does not reach it and reformatting destroyed it. + expect(edited()).toContain('! this value came from the 2019 survey'); + }); + + it('keeps a comment between two objects', () => { + expect(edited()).toContain('! a note between objects'); + }); + + it('leaves a field the author left bare bare', () => { + // Absence is as much a thing the author wrote as the words are. + const written = edited(); + + expect(written).toMatch(/Suburbs;\s*$/m); + expect(written).not.toContain('!- Terrain'); + }); + + it('labels a bare field on request, and still costs no comment line', () => { + const written = edited({ fieldComments: 'generate' }); + + expect(written).toContain('!- Terrain'); + expect(written).toContain('! this value came from the 2019 survey'); + expect(written).toContain('! a note between objects'); + expect(written).toContain('!- North Axis {deg}'); + }); + + it('changes the value and nothing else', () => { + const written = edited(); + + expect(written).toContain('42.0'); + expect(written).not.toContain(' 0,'); + }); + + it('attaches a comment by its delimiter, not by counting lines', () => { + // Several fields share a line in real files: a surface's vertices are routinely written three + // to a line with one comment for the triple. Counting lines mis-attaches every comment after + // the first such line. + const text = [ + 'Version, 26.1;', + '', + 'Building,', + ' Packed, City, 0.04, !- three fields, one comment', + ' 0.4; !- and another', + '', + ].join('\n'); + const { document } = parseIdf(text, v26, { strict: false, preserveFormatting: true }); + document.require('Building', 'Packed').set('terrain', 'Suburbs'); + + const written = writeIdf(document); + + expect(written).toContain('!- three fields, one comment'); + expect(written).toContain('!- and another'); + }); +}); + +describe('the line the author put a value on', () => { + // 21.5% of the statements in the 693 EnergyPlus example files write several values to a line, + // and 690 of those files contain at least one. Writing one value per line regardless turns a + // four-line surface into twelve, which is the most visible thing a reformat does to geometry. + const GROUPED = [ + 'Version, 26.1;', + '', + 'BuildingSurface:Detailed,', + ' S1, Wall, C1, Z1, , Outdoors, , SunExposed, WindExposed, , ,', + ' 0, 0, 4.572, !- X,Y,Z ==> Vertex 1 {m}', + ' 0, 0, 0; !- X,Y,Z ==> Vertex 2 {m}', + '', + ].join('\n'); + + it('keeps values the author grouped onto one line', () => { + const { document } = parseIdf(GROUPED, v26, { strict: false, preserveFormatting: true }); + document.require('BuildingSurface:Detailed', 'S1').set('sun_exposure', 'NoSun'); + + const written = writeIdf(document); + + expect(written.split('\n')).toHaveLength(GROUPED.split('\n').length); + expect(written).toMatch(/0\.0, 0\.0, 4\.572,\s+!- X,Y,Z ==> Vertex 1 \{m\}/); + expect(written).toMatch(/0\.0, 0\.0, 0\.0;\s+!- X,Y,Z ==> Vertex 2 \{m\}/); + }); + + it('keeps a whole object the author wrote on one line', () => { + // The cheaper case, and the one that surprises on a file with no geometry in it: 11.3% of + // statements are written this way, and nobody thinks of `Timestep,4;` as formatting they chose. + const text = 'Version, 26.1;\n\nTimestep,4;\n'; + const { document } = parseIdf(text, v26, { strict: false, preserveFormatting: true }); + [...document.all('Timestep')][0]!.set('number_of_timesteps_per_hour', 6); + + const written = writeIdf(document); + + expect(written).toContain('Timestep,'); + expect(written.split('\n')).toHaveLength(text.split('\n').length); + }); + + it('gives a field the author never wrote a line of its own', () => { + // No author to be faithful to, so the writer's own habit applies. + const text = 'Version, 26.1;\n\nBuilding,\n My Building;\n'; + const { document } = parseIdf(text, v26, { strict: false, preserveFormatting: true }); + document.require('Building', 'My Building').set('north_axis', 42); + + expect(writeIdf(document)).toMatch(/\n\s+42\.0;\s+!- North Axis/); + }); +}); + +describe('the fields the author wrote out as blanks', () => { + // The writer stops at the last field that is SET, so a run of explicit commas the author wrote + // is dropped and their field-name comments go with them. A single-field edit took one + // Sizing:System from 38 lines to 22; across the 693 example files it is 20,571 lines, more than + // any other difference a rewrite makes. A field written out as a blank is as much a thing the + // author wrote as a field left bare of its comment, which is the rule this path already follows. + const BLANKS = [ + 'Version, 26.1;', + '', + 'Building,', + ' My Building, !- Name', + ' 0.0, !- North Axis {deg}', + ' , !- Terrain', + ' , !- Loads Convergence Tolerance Value', + ' , !- Temperature Convergence Tolerance Value', + ' ; !- Solar Distribution', + '', + ].join('\n'); + + it('keeps them, and their comments, through an edit', () => { + const { document } = parseIdf(BLANKS, v26, { strict: false, preserveFormatting: true }); + document.require('Building', 'My Building').set('north_axis', 42); + + const written = writeIdf(document); + + expect(written.split('\n')).toHaveLength(BLANKS.split('\n').length); + expect(written).toContain('!- Terrain'); + expect(written).toContain('!- Solar Distribution'); + }); + + it('still trims them where there is no author to be faithful to', () => { + // A document read without preservation has nothing to reproduce, so the ordinary writer's + // habit applies and a run of bare commas stays out of the output. + const { document } = parseIdf(BLANKS, v26, { strict: false }); + + expect(writeIdf(document)).not.toContain('!- Solar Distribution'); + }); +}); + +describe('the column the comment goes in', () => { + it('puts the marker where EnergyPlus puts it, so a rewrite leaves no seam', () => { + // `1ZoneUncontrolled.idf` writes `!-` at index 29 on 223 of its 231 commented lines. The + // option is a COLUMN, counted from 1, and was being applied as an index. + const text = 'Version, 26.1;\n\nBuilding,\n My Building,\n 0.0;\n'; + const { document } = parseIdf(text, v26, { strict: false, preserveFormatting: true }); + document.require('Building', 'My Building').set('north_axis', 42); + + for (const line of writeIdf(document).split('\n')) { + const marker = line.indexOf('!-'); + if (marker > 0) expect(marker).toBe(29); + } + }); +}); + +/** + * The file the three composing accessors are exercised against. + * + * `changedObjects`, `regionOf` and `renderObject` are one capability in three names, and they are + * tested against one file so that a change to the fixture cannot leave two suites quietly asserting + * against different text while both pass. The author's unit on the terminator line is the load + * bearing part: it is what a rewrite used to destroy and what the range has to reach past. + */ +const COMPOSING = [ + 'Version, 26.1;', + '', + 'Building,', + ' My Building, !- Name', + ' 0.0; !- North Axis {deg}', + '', + 'Timestep, 4;', + '', +].join('\n'); + +const composing = () => + parseIdf(COMPOSING, v26, { strict: false, preserveFormatting: true }).document; + +describe("where an object's characters were", () => { + // `changedObjects()` says WHICH objects a write will rewrite. Without saying WHERE the old ones + // are, a consumer building the smallest possible change has to write the whole file and diff it, + // which is the work that method exists to avoid. Found by the language server team reading the + // branch before it merged. + + it('locates an object that has not changed', () => { + const document = composing(); + const at = document.regionOf(document.require('Building', 'My Building'))!; + + expect(document.rawText!.slice(at.start, at.end)).toBe( + 'Building,\n My Building, !- Name\n 0.0; !- North Axis {deg}' + ); + }); + + it('still locates it after it changes, which is the case it is for', () => { + // The objects worth locating are the ones being rewritten, and `SOURCE` is cleared the moment + // one is touched because its absence is what marks it. A second record answers this. + const document = composing(); + const building = document.require('Building', 'My Building'); + const before = document.regionOf(building); + building.set('north_axis', 42); + + expect(document.regionOf(building)).toEqual(before); + expect([...document.changedObjects()]).toContain(building); + }); + + it('reaches past the semicolon to the comment the writer replaces', () => { + // Not `statement.region`, which stops at the terminator. A comment on the terminator's own line + // is that statement's last field's comment and a preserving write rewrites it; a consumer + // replacing the shorter range would leave it behind describing a field that had just moved. + const document = composing(); + const at = document.regionOf(document.require('Building', 'My Building'))!; + + expect(document.rawText!.slice(at.start, at.end)).toContain('!- North Axis {deg}'); + }); + + it('answers nothing for an object added since the read', () => { + const document = composing(); + const added = document.addRaw('Zone', 'Late Arrival', {}); + + expect(document.regionOf(added)).toBeUndefined(); + }); + + it('answers nothing for a document read without preservation', () => { + const document = parseIdf(COMPOSING, v26, { strict: false }).document; + + expect(document.regionOf(document.require('Building', 'My Building'))).toBeUndefined(); + }); + + it('locates each object separately, in source order', () => { + const document = composing(); + const regions = [...document.objects()] + .map((obj) => document.regionOf(obj)) + .filter((region) => region !== undefined); + + expect(regions).toHaveLength(3); + for (let i = 1; i < regions.length; i += 1) { + expect(regions[i]!.start).toBeGreaterThanOrEqual(regions[i - 1]!.end); + } + }); +}); + +describe('the text that belongs in that range', () => { + // The third leg. Knowing WHICH objects change and WHERE the old text is buys a consumer nothing + // while producing the new text for ONE object has no correct form: `writeObject` called with + // options built by hand comes back with the author's units as generated labels, because the + // annotations a preserving write hands it are internal. + + it('keeps the unit that the ordinary per-object writer drops', () => { + const document = composing(); + const building = document.require('Building', 'My Building'); + building.set('north_axis', 42); + + expect(document.renderObject(building)).toContain('!- North Axis {deg}'); + // What a consumer would have had to reach for, and what it costs. + expect( + writeObject(building, { comments: true, commentColumn: 30, indent: ' ' }) + ).not.toContain('{deg}'); + }); + + it('splices into its own range to give back exactly what a whole write gives back', () => { + // The claim the three names make together, pinned as one assertion. If this ever fails, an + // editor built on them is silently writing a different file from the one `writeIdf` writes. + const document = composing(); + document.require('Building', 'My Building').set('north_axis', 42); + [...document.all('Timestep')][0]!.set('number_of_timesteps_per_hour', 6); + + let spliced = document.rawText!; + // Back to front, so an earlier edit does not move a later range. + const changed = [...document.changedObjects()] + .map((obj) => ({ at: document.regionOf(obj)!, text: document.renderObject(obj)! })) + .sort((a, b) => b.at.start - a.at.start); + expect(changed).toHaveLength(2); + for (const { at, text } of changed) { + spliced = spliced.slice(0, at.start) + text + spliced.slice(at.end); + } + + expect(spliced).toBe(writeIdf(document)); + }); + + it('takes the one option a preserving write honours, and no others', () => { + // `indent`, `commentColumn`, `ordering` and `versionFirst` are refused by `writeIdf` alongside + // `preserveFormatting`; `comments: false` and `compressed` defeat preservation entirely and + // send the document down the formatting path. Accepting any of them here would render one + // object on terms the surrounding file was not written on. + // A source with a field the author wrote BARE, which is what the option is about. + const bare = + 'Version, 26.1;\n\nBuilding,\n My Building, !- Name\n 0.0, !- North Axis {deg}\n City;\n'; + const { document } = parseIdf(bare, v26, { strict: false, preserveFormatting: true }); + const building = document.require('Building', 'My Building'); + building.set('north_axis', 42); + + // Bare stays bare, and asking for labels is the one thing that changes. + expect(document.renderObject(building)).not.toContain('!- Terrain'); + expect(document.renderObject(building, { fieldComments: 'generate' })).toContain('!- Terrain'); + }); + + it('ends where the range ends, with no line break of its own', () => { + const document = composing(); + + expect(document.renderObject(document.require('Building', 'My Building'))).not.toMatch(/\n$/); + }); + + it('answers nothing for an object the retained source does not hold', () => { + const document = composing(); + + expect(document.renderObject(document.addRaw('Zone', 'Late Arrival', {}))).toBeUndefined(); + expect( + parseIdf(COMPOSING, v26, { strict: false }).document.renderObject( + parseIdf(COMPOSING, v26, { strict: false }).document.require('Building', 'My Building') + ) + ).toBeUndefined(); + }); +}); diff --git a/packages/core/tests/write.test.ts b/packages/core/tests/write.test.ts index 42aaf16..93d3650 100644 --- a/packages/core/tests/write.test.ts +++ b/packages/core/tests/write.test.ts @@ -142,7 +142,12 @@ describe('writer defaults are pinned (FR-017)', () => { expect(fieldLines.every((l) => l.startsWith(' ') && !l.startsWith(' '))).toBe(true); }); - it('puts the comment at column 30', () => { + it('puts the comment at column 30, which is index 29', () => { + // Where EnergyPlus itself writes it: `1ZoneUncontrolled.idf` puts `!-` at index 29 on 223 of + // its 231 commented lines. The default used to be applied as an INDEX, which put every line + // this writer produced one place right of the files it imitates. On a preserving write that + // is the difference that shows, because a rewritten object's comments then stand one column + // clear of every untouched object around it and each save leaves a visible seam. const text = writeIdf(model(v26)); let checked = 0; @@ -151,8 +156,8 @@ describe('writer defaults are pinned (FR-017)', () => { if (marker <= 0) continue; // Only lines the padding actually reached: a value longer than the column pushes the comment // right, and that overflow behaviour is itself one of the seven differences. - if (line.slice(0, marker).trimEnd().length < 30) { - expect(marker).toBe(30); + if (line.slice(0, marker).trimEnd().length < 29) { + expect(marker).toBe(29); checked += 1; } } diff --git a/scripts/check-publication.mjs b/scripts/check-publication.mjs index a9521cd..7e2114c 100644 --- a/scripts/check-publication.mjs +++ b/scripts/check-publication.mjs @@ -109,18 +109,19 @@ class CannotRun extends Error {} * asserted by the run in that repository and is not observable from here. So the level is written * down by someone who checked both, and moving it is the act of re-attesting. * - * T101 named conformance-2026.6, the level that proved the Tier 1 port. This is 2026.10, the level - * that carries the preserved-text assertion with the second language's divergence entries removed, - * and the evidence for it, taken rather than recalled: + * T101 named conformance-2026.6, the level that proved the Tier 1 port. This is 2026.11, the level + * whose runners stop reporting a false failure on preserve-edit-one-field, and the evidence for it, + * taken rather than recalled: * - * idfkit-js packages/core/package.json idfkit.conformance = conformance-2026.10 - * idfkit pyproject.toml [tool.idfkit.conformance] level = conformance-2026.10 + * idfkit-js packages/core/package.json idfkit.conformance = conformance-2026.11 + * idfkit pyproject.toml [tool.idfkit.conformance] level = conformance-2026.11 * idfkit-js npm run check:release green at that level * idfkit uv run python scripts/check_release_conformance.py green at that level * - * Both were run against the corpus checkout on the day this moved, rather than read off a CI - * badge, because the two levels that preceded this one were cut hours apart and a badge would have - * been reporting the older of them. + * Both were run against the corpus checked out AT THE TAG on the day this moved, rather than read + * off a CI badge, because the two levels that preceded 2026.10 were cut hours apart and a badge + * would have been reporting the older of them. 2026.11 changes no case: 69 cases and 211 + * assertions, as 2026.10 had, so the precondition is met on the same evidence it was. * * Each level since 2026.6 contains all of it and adds cases, so the precondition is met more * strongly rather than less. @@ -131,7 +132,7 @@ class CannotRun extends Error {} * the pin on every run, so the next time the two part company it fails a cheap gate on the change * that caused it rather than a release months later. */ -const REQUIRED_CONFORMANCE = 'conformance-2026.10'; +const REQUIRED_CONFORMANCE = 'conformance-2026.11'; /** The distribution gates, precondition 4. Order is cheapest first. */ const DISTRIBUTION_GATES = [