From 2ead77d05d7104a7aee4b22df0779998069e8f23 Mon Sep 17 00:00:00 2001 From: Eemeli Aro Date: Mon, 29 Mar 2021 00:21:26 +0300 Subject: [PATCH 1/2] feat!: Make Pair not extend NodeBase; drop its prop forwarding BREAKING CHANGE: While the inheritance of the class is just an implementation detail, dropping the prop forwarding means that a user that previously set the `commentBefore`, `comment` or `spaceBefore` property of a pair will now need to set it for the appropriate key or value. While adjusting the test suite for the changes, the somewhat surprising behaviour of multi-line comments on scalar values getting handled as `commentBefore` instead of `comment` is also dropped. --- docs/05_content_nodes.md | 2 +- src/compose/composer.ts | 5 ++- src/nodes/Pair.ts | 38 +++------------- src/schema/types.ts | 2 +- src/schema/yaml-1.1/pairs.ts | 26 ++++++----- src/stringify/stringifyCollection.ts | 40 +++++++++++------ src/stringify/stringifyPair.ts | 23 +++------- src/stringify/stringifyString.ts | 14 +----- src/test-events.ts | 2 +- tests/doc/comments.js | 67 ++++++++-------------------- tests/doc/parse.js | 2 +- 11 files changed, 82 insertions(+), 139 deletions(-) diff --git a/docs/05_content_nodes.md b/docs/05_content_nodes.md index afc9b5a2..9e7a89f6 100644 --- a/docs/05_content_nodes.md +++ b/docs/05_content_nodes.md @@ -46,7 +46,7 @@ On the other hand, `!!int` and `!!float` stringifiers will take `format` into ac ## Collections ```js -class Pair extends NodeBase { +class Pair { key: K // When parsed, key and value are always value: V // Node or null, but can be set to anything } diff --git a/src/compose/composer.ts b/src/compose/composer.ts index 0749d8f4..2a1013d5 100644 --- a/src/compose/composer.ts +++ b/src/compose/composer.ts @@ -1,7 +1,7 @@ import { Directives } from '../doc/directives.js' import { Document } from '../doc/Document.js' import { ErrorCode, YAMLParseError, YAMLWarning } from '../errors.js' -import { isCollection } from '../nodes/Node.js' +import { isCollection, isPair } from '../nodes/Node.js' import { defaultOptions, DocumentOptions, @@ -99,7 +99,8 @@ export class Composer { } else if (afterEmptyLine || doc.directives.marker || !dc) { doc.commentBefore = comment } else if (isCollection(dc) && !dc.flow && dc.items.length > 0) { - const it = dc.items[0] + let it = dc.items[0] + if (isPair(it)) it = it.key const cb = it.commentBefore it.commentBefore = cb ? `${comment}\n${cb}` : comment } else { diff --git a/src/nodes/Pair.ts b/src/nodes/Pair.ts index 02542109..e751cc2e 100644 --- a/src/nodes/Pair.ts +++ b/src/nodes/Pair.ts @@ -2,8 +2,7 @@ import { createNode, CreateNodeContext } from '../doc/createNode.js' import { StringifyContext } from '../stringify/stringify.js' import { stringifyPair } from '../stringify/stringifyPair.js' import { addPairToJSMap } from './addPairToJSMap.js' -import { isNode, NodeBase, PAIR } from './Node.js' -import { Scalar } from './Scalar.js' +import { NODE_TYPE, PAIR } from './Node.js' import { ToJSContext } from './toJS.js' export function createPair( @@ -16,7 +15,9 @@ export function createPair( return new Pair(k, v) } -export class Pair extends NodeBase { +export class Pair { + readonly [NODE_TYPE]: symbol + /** Always Node or null when parsed, but can be set to anything. */ key: K @@ -24,38 +25,9 @@ export class Pair extends NodeBase { value: V | null constructor(key: K, value: V | null = null) { - super(PAIR) + Object.defineProperty(this, NODE_TYPE, { value: PAIR }) this.key = key this.value = value - - // TS doesn't allow for accessors to override properties - // https://github.com/microsoft/TypeScript/pull/33509 - Object.defineProperties(this, { - commentBefore: { - get: () => (isNode(this.key) ? this.key.commentBefore : undefined), - set: (cb: string | null) => { - if (this.key == null) this.key = (new Scalar(null) as unknown) as K - if (isNode(this.key)) this.key.commentBefore = cb - else { - const msg = - 'Pair.commentBefore is an alias for Pair.key.commentBefore. To set it, the key must be a Node.' - throw new Error(msg) - } - } - }, - spaceBefore: { - get: () => (isNode(this.key) ? this.key.spaceBefore : undefined), - set: (sb: boolean) => { - if (this.key == null) this.key = (new Scalar(null) as unknown) as K - if (isNode(this.key)) this.key.spaceBefore = sb - else { - const msg = - 'Pair.spaceBefore is an alias for Pair.key.spaceBefore. To set it, the key must be a Node.' - throw new Error(msg) - } - } - } - }) } toJSON(_?: unknown, ctx?: ToJSContext): ReturnType { diff --git a/src/schema/types.ts b/src/schema/types.ts index c580d9ad..9ee2368a 100644 --- a/src/schema/types.ts +++ b/src/schema/types.ts @@ -100,7 +100,7 @@ export interface CollectionTag extends TagBase { * If returning a non-`Node` value, the output will be wrapped as a `Scalar`. */ resolve( - value: YAMLMap | YAMLSeq, + value: YAMLMap.Parsed | YAMLSeq.Parsed, onError: (message: string) => void, options: ParseOptions ): unknown diff --git a/src/schema/yaml-1.1/pairs.ts b/src/schema/yaml-1.1/pairs.ts index 9cbf039a..6a89cb1f 100644 --- a/src/schema/yaml-1.1/pairs.ts +++ b/src/schema/yaml-1.1/pairs.ts @@ -1,13 +1,16 @@ import type { CreateNodeContext } from '../../doc/createNode.js' -import type { Schema } from '../../schema/Schema.js' -import { isMap, isPair, isSeq } from '../../nodes/Node.js' +import { isMap, isPair, isSeq, ParsedNode } from '../../nodes/Node.js' import { createPair, Pair } from '../../nodes/Pair.js' +import { Scalar } from '../../nodes/Scalar.js' import { YAMLMap } from '../../nodes/YAMLMap.js' import { YAMLSeq } from '../../nodes/YAMLSeq.js' +import type { Schema } from '../../schema/Schema.js' import type { CollectionTag } from '../types.js' export function resolvePairs( - seq: YAMLSeq | YAMLMap, + seq: + | YAMLSeq.Parsed> + | YAMLMap.Parsed, onError: (message: string) => void ) { if (isSeq(seq)) { @@ -17,21 +20,24 @@ export function resolvePairs( else if (isMap(item)) { if (item.items.length > 1) onError('Each pair must have its own sequence indicator') - const pair = item.items[0] || new Pair(null) + const pair = + item.items[0] || new Pair(new Scalar(null) as Scalar.Parsed) if (item.commentBefore) - pair.commentBefore = pair.commentBefore - ? `${item.commentBefore}\n${pair.commentBefore}` + pair.key.commentBefore = pair.key.commentBefore + ? `${item.commentBefore}\n${pair.key.commentBefore}` : item.commentBefore - if (item.comment) - pair.comment = pair.comment - ? `${item.comment}\n${pair.comment}` + if (item.comment) { + const cn = pair.value || pair.key + cn.comment = cn.comment + ? `${item.comment}\n${cn.comment}` : item.comment + } item = pair } seq.items[i] = isPair(item) ? item : new Pair(item) } } else onError('Expected a sequence for this tag') - return seq as YAMLSeq + return seq as YAMLSeq.Parsed> } export function createPairs( diff --git a/src/stringify/stringifyCollection.ts b/src/stringify/stringifyCollection.ts index 09fc00eb..111feb20 100644 --- a/src/stringify/stringifyCollection.ts +++ b/src/stringify/stringifyCollection.ts @@ -28,29 +28,43 @@ export function stringifyCollection( const inFlow = flow || ctx.inFlow if (inFlow) itemIndent += indentStep ctx = Object.assign({}, ctx, { indent: itemIndent, inFlow, type: null }) - let chompKeep = false + let chompKeep = false // flag for the preceding node's status let hasItemWithNewLine = false const nodes = items.reduce((nodes: StringifyNode[], item, i) => { let comment: string | null = null - if (isNode(item) || isPair(item)) { - if (!chompKeep && item.spaceBefore) nodes.push({ comment: true, str: '' }) - + if (isNode(item)) { + if (!chompKeep && item.spaceBefore) { + nodes.push({ comment: true, str: '' }) + if (inFlow) hasItemWithNewLine = true + } if (item.commentBefore) { // This match will always succeed on a non-empty string for (const line of item.commentBefore.match(/^.*$/gm) as string[]) nodes.push({ comment: true, str: `#${line}` }) + if (inFlow) hasItemWithNewLine = true + } + if (item.comment) { + comment = item.comment + if (inFlow) hasItemWithNewLine = true + } + } else if (isPair(item)) { + if (isNode(item.key)) { + if (!chompKeep && item.key.spaceBefore) { + nodes.push({ comment: true, str: '' }) + if (inFlow) hasItemWithNewLine = true + } + if (item.key.commentBefore) { + // This match will always succeed on a non-empty string + for (const line of item.key.commentBefore.match(/^.*$/gm) as string[]) + nodes.push({ comment: true, str: `#${line}` }) + if (inFlow) hasItemWithNewLine = true + } + if (inFlow && item.key.comment) hasItemWithNewLine = true } - - if (item.comment) comment = item.comment - - const pair = item as any // Apply guards manually in the following if ( inFlow && - ((!chompKeep && item.spaceBefore) || - item.commentBefore || - item.comment || - (pair.key && (pair.key.commentBefore || pair.key.comment)) || - (pair.value && (pair.value.commentBefore || pair.value.comment))) + isNode(item.value) && + (item.value.commentBefore || item.value.comment) ) hasItemWithNewLine = true } diff --git a/src/stringify/stringifyPair.ts b/src/stringify/stringifyPair.ts index 49b14a4e..cee3303b 100644 --- a/src/stringify/stringifyPair.ts +++ b/src/stringify/stringifyPair.ts @@ -5,9 +5,9 @@ import { addComment } from './addComment.js' import { stringify, StringifyContext } from './stringify.js' export function stringifyPair( - { comment, key, value }: Readonly, + { key, value }: Readonly, ctx: StringifyContext, - onComment?: () => void, + _onComment?: () => void, onChompKeep?: () => void ) { const { @@ -30,7 +30,7 @@ export function stringifyPair( let explicitKey = !simpleKeys && (!key || - (keyComment && value == null) || + (keyComment && value == null && !ctx.inFlow) || isCollection(key) || (isScalar(key) ? key.type === Scalar.BLOCK_FOLDED || key.type === Scalar.BLOCK_LITERAL @@ -62,24 +62,13 @@ export function stringifyPair( (value == null && (explicitKey || ctx.inFlow)) ) { str = addComment(str, ctx.indent, keyComment) - if (comment) { - if (keyComment && !comment.includes('\n')) - str += `\n${ctx.indent || ''}#${comment}` - else str = addComment(str, ctx.indent, comment) - if (onComment) onComment() - } else if (chompKeep && !keyComment && onChompKeep) onChompKeep() + if (chompKeep && !keyComment && onChompKeep) onChompKeep() return ctx.inFlow && !explicitKey ? str : `? ${str}` } str = explicitKey ? `? ${addComment(str, ctx.indent, keyComment)}\n${indent}:` : addComment(`${str}:`, ctx.indent, keyComment) - if (comment) { - if (keyComment && !explicitKey && !comment.includes('\n')) - str += `\n${ctx.indent || ''}#${comment}` - else str = addComment(str, ctx.indent, comment) - if (onComment) onComment() - } let vcb = '' let valueComment = null @@ -94,7 +83,7 @@ export function stringifyPair( value = doc.createNode(value) } ctx.implicitKey = false - if (!explicitKey && !keyComment && !comment && isScalar(value)) + if (!explicitKey && !keyComment && isScalar(value)) ctx.indentAtStart = str.length + 1 chompKeep = false if ( @@ -117,7 +106,7 @@ export function stringifyPair( () => (chompKeep = true) ) let ws = ' ' - if (vcb || keyComment || comment) { + if (vcb || keyComment) { ws = `${vcb}\n${ctx.indent}` } else if (!explicitKey && isCollection(value)) { const flow = valueStr[0] === '[' || valueStr[0] === '{' diff --git a/src/stringify/stringifyString.ts b/src/stringify/stringifyString.ts index 3b2f088b..98da1437 100644 --- a/src/stringify/stringifyString.ts +++ b/src/stringify/stringifyString.ts @@ -1,5 +1,4 @@ import { Scalar } from '../nodes/Scalar.js' -import { addCommentBefore } from './addComment.js' import { foldFlowLines, FoldOptions, @@ -227,7 +226,7 @@ function plainString( onComment?: () => void, onChompKeep?: () => void ) { - const { comment, type, value } = item + const { type, value } = item const { actualString, implicitKey, indent, inFlow } = ctx if ( (implicitKey && /[\n[\]{},]/.test(value)) || @@ -290,18 +289,9 @@ function plainString( return doubleQuotedString(value, ctx) } } - const body = implicitKey + return implicitKey ? str : foldFlowLines(str, indent, FOLD_FLOW, getFoldOptions(ctx)) - if ( - comment && - !inFlow && - (body.indexOf('\n') !== -1 || comment.indexOf('\n') !== -1) - ) { - if (onComment) onComment() - return addCommentBefore(body, indent, comment) - } - return body } export function stringifyString( diff --git a/src/test-events.ts b/src/test-events.ts index 46a7b547..4da8d7d0 100644 --- a/src/test-events.ts +++ b/src/test-events.ts @@ -93,7 +93,7 @@ function addEvents( } props = ` &${anchor}` } - if (node.tag) props += ` <${node.tag}>` + if (isNode(node) && node.tag) props += ` <${node.tag}>` if (isMap(node)) { events.push(`+MAP${props}`) diff --git a/tests/doc/comments.js b/tests/doc/comments.js index 8ef642ee..e24afcd1 100644 --- a/tests/doc/comments.js +++ b/tests/doc/comments.js @@ -189,8 +189,8 @@ describe('parse comments', () => { commentBefore: 'c0\nc1', items: [ {}, - { commentBefore: 'c2', value: { comment: 'c3' } }, - { commentBefore: 'c4' } + { key: { commentBefore: 'c2' }, value: { comment: 'c3' } }, + { key: { commentBefore: 'c4' } } ] } ] @@ -289,7 +289,7 @@ describe('parse comments', () => { const doc = YAML.parseDocument('{ a,\n#c0\n#c1\nb }') expect(doc.contents.items).toMatchObject([ { key: { value: 'a' } }, - { commentBefore: 'c0\nc1', key: { value: 'b' } } + { key: { commentBefore: 'c0\nc1', value: 'b' } } ]) }) }) @@ -324,7 +324,7 @@ describe('stringify comments', () => { const src = 'string' const doc = YAML.parseDocument(src) doc.contents.comment = 'comment\nlines' - expect(String(doc)).toBe('#comment\n#lines\nstring\n') + expect(String(doc)).toBe('string\n#comment\n#lines\n') }) test('"quoted"', () => { @@ -404,8 +404,8 @@ describe('stringify comments', () => { const doc = YAML.parseDocument(src) doc.contents.items[0].key.commentBefore = 'c0' doc.contents.items[0].key.comment = 'c1' - doc.contents.items[0].comment = 'c2' const seq = doc.contents.items[0].value + seq.commentBefore = 'c2' seq.items[0].commentBefore = 'c3' seq.items[1].commentBefore = 'c4' seq.comment = 'c5' @@ -426,9 +426,9 @@ describe('stringify comments', () => { test('plain', () => { const src = 'key1: value 1\nkey2: value 2\n' const doc = YAML.parseDocument(src) - doc.contents.items[0].commentBefore = 'c0' - doc.contents.items[1].commentBefore = 'c1' - doc.contents.items[1].comment = 'c2' + doc.contents.items[0].key.commentBefore = 'c0' + doc.contents.items[1].key.commentBefore = 'c1' + doc.contents.items[1].key.comment = 'c2' doc.contents.items[1].value.spaceBefore = true doc.contents.comment = 'c3' expect(String(doc)).toBe(source` @@ -445,9 +445,9 @@ describe('stringify comments', () => { test('multiline', () => { const src = 'key1: value 1\nkey2: value 2\n' const doc = YAML.parseDocument(src) - doc.contents.items[0].commentBefore = 'c0\nc1' - doc.contents.items[1].commentBefore = '\nc2\n\nc3' - doc.contents.items[1].comment = 'c4\nc5' + doc.contents.items[0].key.commentBefore = 'c0\nc1' + doc.contents.items[1].key.commentBefore = '\nc2\n\nc3' + doc.contents.items[1].key.comment = 'c4\nc5' doc.contents.items[1].value.spaceBefore = true doc.contents.items[1].value.commentBefore = 'c6' doc.contents.comment = 'c7\nc8' @@ -471,7 +471,7 @@ describe('stringify comments', () => { }) }) - describe.skip('flow collection commens', () => { + describe('flow collection commens', () => { test('line comment after , in seq', () => { const doc = YAML.parseDocument(source` [ a, #c0 @@ -485,7 +485,7 @@ describe('stringify comments', () => { `) }) - test('line comment after , in map', () => { + test.skip('line comment after , in map', () => { const doc = YAML.parseDocument(source` { a, #c0 b: c, #c1 @@ -595,8 +595,10 @@ describe('blank lines', () => { test(name, () => { const doc = YAML.parseDocument(src) expect(String(doc)).toBe(src) - expect(doc.contents.items[1]).not.toHaveProperty('spaceBefore', true) - doc.contents.items[1].spaceBefore = true + let it = doc.contents.items[1] + if (it.key) it = it.key + expect(it).not.toHaveProperty('spaceBefore', true) + it.spaceBefore = true expect(String(doc)).toBe(src) }) } @@ -666,8 +668,8 @@ describe('blank lines', () => { const doc = YAML.parseDocument(src) expect(doc.contents).toMatchObject({ items: [ - { key: { value: 'a' }, value: { value: 1 }, spaceBefore: true }, - { key: { value: 'b' }, value: { value: 2 }, spaceBefore: true } + { key: { value: 'a', spaceBefore: true }, value: { value: 1 } }, + { key: { value: 'b', spaceBefore: true }, value: { value: 2 } } ] }) }) @@ -918,34 +920,3 @@ a: expect(String(doc)).toBe(src) }) }) - -describe('Pair.commentBefore', () => { - test('Should get key comment', () => { - const key = new YAML.Document().createNode('foo') - const pair = new YAML.Pair(key, 42) - key.commentBefore = 'cc' - expect(pair.commentBefore).toBe('cc') - }) - - test('Should set key comment', () => { - const key = new YAML.Document().createNode('foo') - const pair = new YAML.Pair(key, 42) - pair.commentBefore = 'cc' - expect(key.commentBefore).toBe('cc') - }) - - test('Should create a key from a null value', () => { - const pair = new YAML.Pair(null, 42) - expect(pair.key).toBeNull() - pair.commentBefore = 'cc' - expect(pair.key).not.toBeNull() - expect(pair.key.commentBefore).toBe('cc') - }) - - test('Should throw for non-Node key', () => { - const pair = new YAML.Pair({ foo: 'bar' }, 42) - expect(() => { - pair.commentBefore = 'cc' - }).toThrow(/commentBefore is an alias/) - }) -}) diff --git a/tests/doc/parse.js b/tests/doc/parse.js index c07c0601..03adf423 100644 --- a/tests/doc/parse.js +++ b/tests/doc/parse.js @@ -489,7 +489,7 @@ describe('maps with no values', () => { const src = `{\na: null,\n? b\n}` const doc = YAML.parseDocument(src) expect(String(doc)).toBe(`{ a: null, b }\n`) - doc.contents.items[1].comment = 'c' + doc.contents.items[1].key.comment = 'c' expect(String(doc)).toBe(`{\n a: null,\n b #c\n}\n`) doc.set('b', 'x') expect(String(doc)).toBe(`{\n a: null,\n b: #c\n x\n}\n`) From bbfffae181bbc69685313e1bfcf1f096e33049fc Mon Sep 17 00:00:00 2001 From: Eemeli Aro Date: Mon, 29 Mar 2021 02:19:59 +0300 Subject: [PATCH 2/2] fix: Line comment handling in flow collections --- src/nodes/Collection.ts | 7 ++-- src/stringify/stringifyCollection.ts | 59 +++++++++++++++------------- src/stringify/stringifyPair.ts | 35 +++++++++++------ tests/doc/comments.js | 8 ++-- 4 files changed, 63 insertions(+), 46 deletions(-) diff --git a/src/nodes/Collection.ts b/src/nodes/Collection.ts index b40547e3..1c748987 100644 --- a/src/nodes/Collection.ts +++ b/src/nodes/Collection.ts @@ -1,7 +1,6 @@ import { createNode } from '../doc/createNode.js' import type { Schema } from '../schema/Schema.js' -import { isCollection, isNode, isScalar, NodeBase, NODE_TYPE } from './Node.js' -import type { Pair } from './Pair.js' +import { isCollection, isPair, isScalar, NodeBase, NODE_TYPE } from './Node.js' export function collectionFromPath( schema: Schema, @@ -143,8 +142,8 @@ export abstract class Collection extends NodeBase { hasAllNullValues(allowScalar?: boolean) { return this.items.every(node => { - if (!node || isNode(node)) return false - const n = (node as Pair).value + if (!isPair(node)) return false + const n = node.value return ( n == null || (allowScalar && diff --git a/src/stringify/stringifyCollection.ts b/src/stringify/stringifyCollection.ts index 111feb20..3f4b3b09 100644 --- a/src/stringify/stringifyCollection.ts +++ b/src/stringify/stringifyCollection.ts @@ -28,46 +28,45 @@ export function stringifyCollection( const inFlow = flow || ctx.inFlow if (inFlow) itemIndent += indentStep ctx = Object.assign({}, ctx, { indent: itemIndent, inFlow, type: null }) + + let singleLineOutput = true let chompKeep = false // flag for the preceding node's status - let hasItemWithNewLine = false const nodes = items.reduce((nodes: StringifyNode[], item, i) => { let comment: string | null = null if (isNode(item)) { - if (!chompKeep && item.spaceBefore) { - nodes.push({ comment: true, str: '' }) - if (inFlow) hasItemWithNewLine = true - } + if (!chompKeep && item.spaceBefore) nodes.push({ comment: true, str: '' }) if (item.commentBefore) { // This match will always succeed on a non-empty string for (const line of item.commentBefore.match(/^.*$/gm) as string[]) nodes.push({ comment: true, str: `#${line}` }) - if (inFlow) hasItemWithNewLine = true } if (item.comment) { comment = item.comment - if (inFlow) hasItemWithNewLine = true + singleLineOutput = false } } else if (isPair(item)) { - if (isNode(item.key)) { - if (!chompKeep && item.key.spaceBefore) { - nodes.push({ comment: true, str: '' }) - if (inFlow) hasItemWithNewLine = true - } - if (item.key.commentBefore) { + const ik = isNode(item.key) ? item.key : null + if (ik) { + if (!chompKeep && ik.spaceBefore) nodes.push({ comment: true, str: '' }) + if (ik.commentBefore) { // This match will always succeed on a non-empty string - for (const line of item.key.commentBefore.match(/^.*$/gm) as string[]) + for (const line of ik.commentBefore.match(/^.*$/gm) as string[]) nodes.push({ comment: true, str: `#${line}` }) - if (inFlow) hasItemWithNewLine = true } - if (inFlow && item.key.comment) hasItemWithNewLine = true + if (ik.comment) singleLineOutput = false + } + + if (inFlow) { + const iv = isNode(item.value) ? item.value : null + if (iv) { + if (iv.comment) comment = iv.comment + if (iv.comment || iv.commentBefore) singleLineOutput = false + } else if (item.value == null && ik && ik.comment) { + comment = ik.comment + } } - if ( - inFlow && - isNode(item.value) && - (item.value.commentBefore || item.value.comment) - ) - hasItemWithNewLine = true } + chompKeep = false let str = stringify( item, @@ -75,24 +74,30 @@ export function stringifyCollection( () => (comment = null), () => (chompKeep = true) ) - if (inFlow && !hasItemWithNewLine && str.includes('\n')) - hasItemWithNewLine = true if (inFlow && i < items.length - 1) str += ',' str = addComment(str, itemIndent, comment) if (chompKeep && (comment || inFlow)) chompKeep = false nodes.push({ comment: false, str }) return nodes }, []) + let str: string if (nodes.length === 0) { str = flowChars.start + flowChars.end } else if (inFlow) { const { start, end } = flowChars const strings = nodes.map(n => n.str) + let singleLineLength = 2 + for (const node of nodes) { + if (node.comment || node.str.includes('\n')) { + singleLineOutput = false + break + } + singleLineLength += node.str.length + 2 + } if ( - hasItemWithNewLine || - strings.reduce((sum, str) => sum + str.length + 2, 2) > - Collection.maxFlowStringSingleLineLength + !singleLineOutput || + singleLineLength > Collection.maxFlowStringSingleLineLength ) { str = start for (const s of strings) { diff --git a/src/stringify/stringifyPair.ts b/src/stringify/stringifyPair.ts index cee3303b..b522b24c 100644 --- a/src/stringify/stringifyPair.ts +++ b/src/stringify/stringifyPair.ts @@ -7,7 +7,7 @@ import { stringify, StringifyContext } from './stringify.js' export function stringifyPair( { key, value }: Readonly, ctx: StringifyContext, - _onComment?: () => void, + onComment?: () => void, onChompKeep?: () => void ) { const { @@ -41,11 +41,12 @@ export function stringifyPair( implicitKey: !explicitKey && (simpleKeys || !allNullValues), indent: indent + indentStep }) + let keyCommentDone = false let chompKeep = false let str = stringify( key, ctx, - () => (keyComment = null), + () => (keyCommentDone = true), () => (chompKeep = true) ) @@ -57,15 +58,18 @@ export function stringifyPair( explicitKey = true } - if ( - (allNullValues && (!simpleKeys || ctx.inFlow)) || - (value == null && (explicitKey || ctx.inFlow)) - ) { - str = addComment(str, ctx.indent, keyComment) + if (ctx.inFlow) { + if (allNullValues || value == null) { + if (keyCommentDone && onComment) onComment() + return explicitKey ? `? ${str}` : str + } + } else if ((allNullValues && !simpleKeys) || (value == null && explicitKey)) { + if (keyCommentDone) keyComment = null if (chompKeep && !keyComment && onChompKeep) onChompKeep() - return ctx.inFlow && !explicitKey ? str : `? ${str}` + return addComment(`? ${str}`, ctx.indent, keyComment) } + if (keyCommentDone) keyComment = null str = explicitKey ? `? ${addComment(str, ctx.indent, keyComment)}\n${indent}:` : addComment(`${str}:`, ctx.indent, keyComment) @@ -99,10 +103,12 @@ export function stringifyPair( // If indentSeq === false, consider '- ' as part of indentation where possible ctx.indent = ctx.indent.substr(2) } + + let valueCommentDone = false const valueStr = stringify( value, ctx, - () => (valueComment = null), + () => (valueCommentDone = true), () => (chompKeep = true) ) let ws = ' ' @@ -112,6 +118,13 @@ export function stringifyPair( const flow = valueStr[0] === '[' || valueStr[0] === '{' if (!flow || valueStr.includes('\n')) ws = `\n${ctx.indent}` } else if (valueStr[0] === '\n') ws = '' - if (chompKeep && !valueComment && onChompKeep) onChompKeep() - return addComment(str + ws + valueStr, ctx.indent, valueComment) + + if (ctx.inFlow) { + if (valueCommentDone && onComment) onComment() + return str + ws + valueStr + } else { + if (valueCommentDone) valueComment = null + if (chompKeep && !valueComment && onChompKeep) onChompKeep() + return addComment(str + ws + valueStr, ctx.indent, valueComment) + } } diff --git a/tests/doc/comments.js b/tests/doc/comments.js index e24afcd1..208d0e88 100644 --- a/tests/doc/comments.js +++ b/tests/doc/comments.js @@ -471,7 +471,7 @@ describe('stringify comments', () => { }) }) - describe('flow collection commens', () => { + describe('flow collection comments', () => { test('line comment after , in seq', () => { const doc = YAML.parseDocument(source` [ a, #c0 @@ -485,7 +485,7 @@ describe('stringify comments', () => { `) }) - test.skip('line comment after , in map', () => { + test('line comment after , in map', () => { const doc = YAML.parseDocument(source` { a, #c0 b: c, #c1 @@ -493,9 +493,9 @@ describe('stringify comments', () => { }`) expect(String(doc)).toBe(source` { - ? a, #c0 + a, #c0 b: c, #c1 - ? d #c2 + d #c2 } `) })