From 1143aa4cb8b827a6b062ad86ee4eabf63b1b70b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gustaf=20R=C3=A4ntil=C3=A4?= Date: Mon, 25 Apr 2022 22:47:53 +0200 Subject: [PATCH] feat(indent): much better indent and flow handling, especially when copying/moving Will now try to maintain flow-ness when moving properties/elements in and out of sometimes empty objects/arrays. --- lib/document/document.ts | 34 ++++++---- lib/document/indentable.ts | 107 ++++++++++++++++++++++++++--- lib/document/nodes.ts | 84 +++++++++++++---------- lib/document/parse-to-nodes.ts | 68 +++++++++++++++++-- lib/document/types-internal.ts | 6 ++ lib/rfc6902.test.ts | 120 +++++++++++++++++++++++++++++++++ 6 files changed, 358 insertions(+), 61 deletions(-) create mode 100644 lib/document/types-internal.ts diff --git a/lib/document/document.ts b/lib/document/document.ts index 2c0c168..8d9a28a 100644 --- a/lib/document/document.ts +++ b/lib/document/document.ts @@ -13,7 +13,7 @@ import { type JsonDocumentOptions } from './types.js' import { getDocumentOptions } from './utils.js' -export class JsonDocument extends Indentable +export class JsonDocument { public readonly options: JsonDocumentOptions; @@ -24,8 +24,6 @@ export class JsonDocument extends Indentable options?: Partial< JsonDocumentOptions > ) { - super( ); - this.options = getDocumentOptions( options ); } @@ -63,14 +61,14 @@ export class JsonDocument extends Indentable toString( ): string { - const chooseTabs = this.#useTabs( + const tabs = this.#useTabs( this.root instanceof Indentable ? this.root.tabs : undefined ); const rootIndent = - this.rootIndentation.indentString( chooseTabs ); + this.rootIndentation.indentString( { tabs, fallback: false } ); if ( this.root instanceof JsonPrimitiveBase ) return rootIndent + this.root.raw; @@ -80,20 +78,27 @@ export class JsonDocument extends Indentable if ( node instanceof JsonPrimitiveBase ) return node.raw; - const indent = node.flow ? '' : node.indentString( chooseTabs ); + const indent = node.calculatedFlow + ? '' + : ( parentIndent + node.indentString( { tabs } ) ); if ( node instanceof JsonArray ) { - const ret = [ node.flow ? '[ ' : '[\n' ]; + if ( node.elements.length === 0 ) + return '[ ]'; + + const ret = [ node.calculatedFlow ? '[ ' : '[\n' ]; node.elements.forEach( ( element, i ) => { ret.push( indent + stringify( element, indent ) ); if ( i < node.elements.length - 1 ) - ret.push( node.flow ? ', ' : ',\n' ); + ret.push( node.calculatedFlow ? ', ' : ',\n' ); else - ret.push( node.flow ? ' ' : `\n${parentIndent}` ); + ret.push( + node.calculatedFlow ? ' ' : `\n${parentIndent}` + ); } ); ret.push( ']' ); @@ -101,7 +106,10 @@ export class JsonDocument extends Indentable } else if ( node instanceof JsonObject ) { - const ret = [ node.flow ? '{ ' : '{\n' ]; + if ( node.properties.length === 0 ) + return '{ }'; + + const ret = [ node.calculatedFlow ? '{ ' : '{\n' ]; node.properties.forEach( ( prop, i ) => { @@ -113,9 +121,11 @@ export class JsonDocument extends Indentable ); if ( i < node.properties.length - 1 ) - ret.push( node.flow ? ', ' : ',\n' ); + ret.push( node.calculatedFlow ? ', ' : ',\n' ); else - ret.push( node.flow ? ' ' : `\n${parentIndent}` ); + ret.push( + node.calculatedFlow ? ' ' : `\n${parentIndent}` + ); } ); ret.push( '}' ); diff --git a/lib/document/indentable.ts b/lib/document/indentable.ts index ef178ce..0bfe4dd 100644 --- a/lib/document/indentable.ts +++ b/lib/document/indentable.ts @@ -1,9 +1,74 @@ +import { JsonValueBase } from './types-internal.js' + + +interface IndentStringOptions +{ + tabs?: boolean; + fallback?: boolean; +} + export class Indentable { + private _flow: boolean | undefined = false; + constructor( private _depth = -1, private _tabs?: boolean | undefined ) { } + public getChildrenNodes( ) + : ReadonlyArray< JsonValueBase & ( Indentable | { } ) > + { + return [ ]; + } + + /** + * Flow means a one-line object/array. + * + * Defaults to undefined, and is only set if set by + */ + get flow( ) + { + return this._flow; + } + + set flow( flow: boolean | undefined ) + { + this._flow = flow; + } + + /** + * Returns the flow, or if undefined, false if _any_ child node is false + * (i.e. comes from a source with a non-flow container). Fallbacks to true. + */ + get calculatedFlow( ) + { + const recurseChildrenFlow = () => + { + const recurse = ( node: Indentable ): false | undefined => + { + const children = node.getChildrenNodes( ); + + for ( const child of children ) + { + if ( child.sourceParentFlow === false ) + return false; + } + + for ( const child of children ) + { + if ( child instanceof Indentable ) + return recurse( child ); + } + + return undefined; + } + + return recurse( this ); + }; + + return recurseChildrenFlow( ) ?? this.flow ?? true; + } + /** * The indentation depth of this collection. * @@ -15,6 +80,18 @@ export class Indentable return this._depth; } + /** + * Get the depth, and if not set, fallback to default depth if chosen + */ + getDepth( fallback = true ) + { + return ( this.depth === -1 && fallback ) + ? this.tabs + ? 1 + : 2 + : this.depth; + } + /** * Whether tabs or spaces are used. * @@ -30,7 +107,7 @@ export class Indentable return this.tabs ? '\t' : ' '; } - setIndent( depth: number, tabs: boolean ): void; + setIndent( depth: number, tabs?: boolean ): void; setIndent( from: Indentable ): void; setIndent( depth: number | Indentable, tabs?: boolean ) @@ -38,7 +115,7 @@ export class Indentable if ( typeof depth === 'number' ) { this._depth = depth; - this._tabs = tabs!; + this._tabs = tabs ?? this.tabs; } else { @@ -47,22 +124,36 @@ export class Indentable } } + /** + * Return the depth as if it was tabs or spaces + */ + depthAs( asTabs: boolean ): number + { + const tabs = this.tabs ?? false; + + return asTabs === tabs + ? this.depth + : asTabs + ? this.depth / 2 + : this.depth * 2; + } + /** * Gets the indentation string given the indentable settings. * * If `tabs` is set to true or false, this will overwrite the settings in * this indentable, and change tabs into spaces or vice versa. */ - indentString( tabs?: boolean ) + indentString( options?: IndentStringOptions ) { - if ( this.depth <= 0 ) - return ''; + const { tabs, fallback = true } = options ?? { }; + const curDepth = Math.max( 0, this.getDepth( fallback ) ); const char = tabs === true ? '\t' : tabs === false ? ' ' : this.char; const depth = - ( tabs === undefined || !!tabs === this.tabs ) - ? this.depth - : tabs === true ? this.depth / 2 : this.depth * 2; + ( tabs === undefined || !tabs === !this.tabs ) + ? curDepth + : tabs === true ? curDepth / 2 : curDepth * 2; return char.repeat( depth ); } diff --git a/lib/document/nodes.ts b/lib/document/nodes.ts index 9d1ebca..dc13943 100644 --- a/lib/document/nodes.ts +++ b/lib/document/nodes.ts @@ -1,53 +1,53 @@ import { Indentable } from './indentable.js' +import { JsonValueBase } from './types-internal.js'; -interface JsonValue +export class JsonArray extends Indentable implements JsonValueBase { - toJS( ): unknown; -} + private _elements: Array< JsonNodeType > = [ ]; -export class JsonArray extends Indentable implements JsonValue -{ - /** Flow means a one-line array */ - public flow: boolean = false; + public sourceParentFlow: JsonValueBase[ 'sourceParentFlow' ] = undefined; - #_elements: Array< JsonNodeType > = [ ]; + public override getChildrenNodes( ) + { + return this._elements; + } get elements( ): ReadonlyArray< JsonNodeType > { - return this.#_elements; + return this._elements; } set elements( elements: ReadonlyArray< JsonNodeType > ) { - this.#_elements = [ ...elements ]; + this._elements = [ ...elements ]; } add( value: JsonNodeType ) { - this.#_elements.push( value ); + this._elements.push( value ); } insert( value: JsonNodeType, beforeIndex: number ) { - this.#_elements.splice( beforeIndex, 0, value ); + this._elements.splice( beforeIndex, 0, value ); } get( index: number ) { - return this.#_elements[ index ]; + return this._elements[ index ]; } removeAt( index: number ) { - if ( index < 0 || index >= this.#_elements.length ) + if ( index < 0 || index >= this._elements.length ) throw new Error( `Can't remove element at ${index}` ); - return this.#_elements.splice( index, 1 )[ 0 ]!; + return this._elements.splice( index, 1 )[ 0 ]!; } toJS( ): unknown { - return this.#_elements.map( elem => elem.toJS( ) ); + return this._elements.map( elem => elem.toJS( ) ); } } @@ -57,16 +57,20 @@ interface JsonObjectProperty value: JsonNodeType; } -export class JsonObject extends Indentable implements JsonValue +export class JsonObject extends Indentable implements JsonValueBase { - /** Flow means a one-line object */ - public flow: boolean = false; + private _properties: Array< JsonObjectProperty > = [ ]; + + public sourceParentFlow: JsonValueBase[ 'sourceParentFlow' ] = undefined; - #_properties: Array< JsonObjectProperty > = [ ]; + public override getChildrenNodes( ) + { + return this._properties.map( ( { value } ) => value ); + } get properties( ): ReadonlyArray< JsonObjectProperty > { - return this.#_properties; + return this._properties; } set properties( properties: ReadonlyArray< JsonObjectProperty > ) @@ -82,67 +86,69 @@ export class JsonObject extends Indentable implements JsonValue value: entry[ 1 ], } ) ); - this.#_properties = uniq; + this._properties = uniq; } add( name: string, value: JsonNodeType, ordered: boolean ) { - const existing = this.#_properties.find( prop => prop.name === name ); + const existing = this._properties.find( prop => prop.name === name ); if ( existing ) existing.value = value; else { if ( !ordered ) - this.#_properties.push( { name, value } ); + this._properties.push( { name, value } ); else { // Find the first good place to put this property. // Since the source object might not be sorted, this is a best // effort implementation. let i = 0; - for ( ; i < this.#_properties.length; ++i ) + for ( ; i < this._properties.length; ++i ) { const cmp = - this.#_properties[ i ]!.name.localeCompare( name ); + this._properties[ i ]!.name.localeCompare( name ); if ( cmp === 1 ) break; } - this.#_properties.splice( i, 0, { name, value } ); + this._properties.splice( i, 0, { name, value } ); } } } get( prop: string ) { - return this.#_properties.find( ( { name } ) => name === prop )?.value; + return this._properties.find( ( { name } ) => name === prop )?.value; } remove( prop: string ) { - const index = this.#_properties.findIndex( ( { name } ) => + const index = this._properties.findIndex( ( { name } ) => name === prop ); - if ( index < 0 || index >= this.#_properties.length ) + if ( index < 0 || index >= this._properties.length ) throw new Error( `Can't remove property ${prop}, doesn't exist` ); - const value = this.#_properties[ index ]!.value; - this.#_properties.splice( index, 1 ); + const value = this._properties[ index ]!.value; + this._properties.splice( index, 1 ); return value; } toJS( ): unknown { return Object.fromEntries( - this.#_properties.map( ( { name, value } ) => + this._properties.map( ( { name, value } ) => [ name, value.toJS( ) ] ) ); } } -export class JsonPrimitiveBase< Type > implements JsonValue +export class JsonPrimitiveBase< Type > implements JsonValueBase { + public sourceParentFlow: JsonValueBase[ 'sourceParentFlow' ] = undefined; + public constructor( private _value: Type, private _raw: string ) { } get value( ) @@ -267,11 +273,17 @@ export function nodeFromJS( value: unknown ): JsonNodeType | undefined { const ret = new JsonArray( ); + if ( container.length === 0 ) + ret.flow = undefined; + container.forEach( elem => { const node = nodeFromJS( elem ); if ( node !== undefined ) + { ret.add( node ); + node.sourceParentFlow = ret.flow; + } } ); return ret; @@ -292,9 +304,13 @@ export function nodeFromJS( value: unknown ): JsonNodeType | undefined .filter( ( v ): v is NonNullable< typeof v > => !!v ) .sort( ( a, b ) => a[ 0 ].localeCompare( b[ 0 ] ) ); + if ( props.length === 0 ) + ret.flow = undefined; + props.forEach( ( [ key, node ] ) => { ret.add( key, node, false ); + node.sourceParentFlow = ret.flow; } ); return ret; diff --git a/lib/document/parse-to-nodes.ts b/lib/document/parse-to-nodes.ts index 72b9d44..16e00a5 100644 --- a/lib/document/parse-to-nodes.ts +++ b/lib/document/parse-to-nodes.ts @@ -14,6 +14,7 @@ import { type JsonPrimitive, type JsonNodeType, } from './nodes.js' +import { JsonValueBase } from './types-internal.js' export interface ParseResult @@ -29,10 +30,45 @@ export function parse( json: string ): ParseResult const { whitespace: initialWhitespace, consumedTokens: pos } = extractWhitespace( tokens, 0 ); - return { - initialIndentation: initialWhitespace.indentable, - root: makeJsonAny( tokens, pos ).value, + const initialIndentation = initialWhitespace.indentable; + const root = makeJsonAny( tokens, pos ).value; + + makeRelativeIndentations( root ); + + return { initialIndentation, root }; +} + +function makeRelativeIndentations( node: JsonNodeType ) +{ + if ( !( node instanceof Indentable ) ) + return; + + const docTabs = node.tabs ?? false; + + const recurse = ( node: Indentable & JsonValueBase, indent: number ) => + { + const children = node.getChildrenNodes( ); + + for ( const child of children ) + { + if ( child instanceof Indentable ) + { + const rawDepth = child.depth; + const depth = + rawDepth === -1 + // Probably empty object without children, hence no "depth" + // Rebuild depth as parent + 1 level ( 1 tab or 2 spaces ) + ? indent + ( child.tabs ? 1 : 2 ) + : child.depthAs( docTabs ); + + child.setIndent( depth - indent, docTabs ); + + recurse( child, depth ); + } + } }; + + recurse( node, node.depth ?? 0 ); } interface ConstructedStep< T extends JsonNodeType > @@ -66,12 +102,13 @@ function makeJsonObject( tokens: LexerTokens, pos: number ) const { whitespace, consumedTokens } = extractWhitespace( tokens, i ); i += consumedTokens; - whitespaces.push( whitespace ); const peekToken = tokens[ i ]!; if ( peekToken.type === 'string' ) { + whitespaces.push( whitespace ); + // Property name const name = peekToken.value; ++i; @@ -125,7 +162,15 @@ function makeJsonObject( tokens: LexerTokens, pos: number ) whitespaces.map( whitespace => whitespace.indentable ) ) ); - ret.flow = !hasNewline; + if ( hasNewline || ret.properties.length > 0 ) + // Set the flow, if there are elements (or it's strictly a new-line) + ret.flow = !hasNewline; + else + ret.flow = undefined; + ret.properties.forEach( ( { value } ) => + { + value.sourceParentFlow = ret.flow; + } ); return { value: ret, consumedTokens: i - pos + 1 }; } @@ -143,7 +188,6 @@ function makeJsonArray( tokens: LexerTokens, pos: number ) const { whitespace, consumedTokens } = extractWhitespace( tokens, i ); i += consumedTokens; - whitespaces.push( whitespace ); const peekToken = tokens[ i ]!; @@ -162,6 +206,8 @@ function makeJsonArray( tokens: LexerTokens, pos: number ) } else { + whitespaces.push( whitespace ); + const { value, consumedTokens } = makeJsonAny( tokens, i ); i += consumedTokens; @@ -181,7 +227,15 @@ function makeJsonArray( tokens: LexerTokens, pos: number ) whitespaces.map( whitespace => whitespace.indentable ) ) ); - ret.flow = !hasNewline; + if ( hasNewline || ret.elements.length > 0 ) + // Set the flow, if there are elements (or it's strictly a new-line) + ret.flow = !hasNewline; + else + ret.flow = undefined; + ret.elements.forEach( value => + { + value.sourceParentFlow = ret.flow; + } ); return { value: ret, consumedTokens: i - pos + 1 }; } diff --git a/lib/document/types-internal.ts b/lib/document/types-internal.ts new file mode 100644 index 0000000..ca35cfc --- /dev/null +++ b/lib/document/types-internal.ts @@ -0,0 +1,6 @@ +export interface JsonValueBase +{ + sourceParentFlow: boolean | undefined; + + toJS( ): unknown; +} diff --git a/lib/rfc6902.test.ts b/lib/rfc6902.test.ts index a8442e3..bcea62e 100644 --- a/lib/rfc6902.test.ts +++ b/lib/rfc6902.test.ts @@ -282,6 +282,81 @@ describe( 'rfc6902', ( ) => describe( 'move', ( ) => { + it( 'from object to object target flow', ( ) => + { + const before = + `{\n "foo": { "bar": "baz" },\n "fee": { "a": 1 }\n}`; + const after = + `{\n "foo": { },\n "fee": { "a": 1, "bar": "baz" }\n}`; + + const operations: Operation[ ] = [ + { op: 'move', from: '/foo/bar', path: '/fee/bar' } + ]; + + const res = jsonPatch( before, operations ); + + expect( res ).toBe( after ); + } ); + + it( 'from object to object target empty source flow', ( ) => + { + const before = + `{\n "foo": { "bar": "baz" },\n "fee": { }\n}`; + const after = + `{\n "foo": { },\n "fee": { "bar": "baz" }\n}`; + + const operations: Operation[ ] = [ + { op: 'move', from: '/foo/bar', path: '/fee/bar' } + ]; + + const res = jsonPatch( before, operations ); + + expect( res ).toBe( after ); + } ); + + it( 'from object to object target empty source non-flow', ( ) => + { + const before = + `{\n "foo": {\n "bar": "baz"\n },\n "fee": {\n }\n}`; + const after = + `{\n "foo": { },\n "fee": {\n "bar": "baz"\n }\n}`; + + const operations: Operation[ ] = [ + { op: 'move', from: '/foo/bar', path: '/fee/bar' } + ]; + + const res = jsonPatch( before, operations ); + + expect( res ).toBe( after ); + } ); + + it( 'from object to object target non-flow', ( ) => + { + const before = `{ + "foo": { + "bar": "baz" + }, + "fee": { + "a": 1 + } +}`; + const after = `{ + "foo": { }, + "fee": { + "a": 1, + "bar": "baz" + } +}`; + + const operations: Operation[ ] = [ + { op: 'move', from: '/foo/bar', path: '/fee/bar' } + ]; + + const res = jsonPatch( before, operations ); + + expect( res ).toBe( after ); + } ); + it( 'from object to object', ( ) => { const before = `{\n "foo": { "bar": "baz" },\n "fee": { }\n}`; @@ -362,6 +437,20 @@ describe( 'rfc6902', ( ) => expect( res ).toBe( after ); } ); + it( 'from non-flow array to array (source becomes empty)', ( ) => + { + const before = `{\n "foo": [\n "bar"\n ],\n "fee": [ ]\n}`; + const after = `{\n "foo": [ ],\n "fee": [\n "bar"\n ]\n}`; + + const operations: Operation[ ] = [ + { op: 'move', from: '/foo/0', path: '/fee/-' } + ]; + + const res = jsonPatch( before, operations ); + + expect( res ).toBe( after ); + } ); + it( 'from array to object', ( ) => { const before = `{\n "foo": [ "bar", "baz" ],\n "fee": { }\n}`; @@ -400,5 +489,36 @@ describe( 'rfc6902', ( ) => expect( ( ) => jsonPatch( before, operations ) ) .toThrowError( /remove element/i ); } ); + + it( 'move object props deeply (flow source)', ( ) => + { + const before = `{\n "foo": { "bar": "baz" }\n}`; + const after = `{\n "foo": { },\n "fxx": { "bar": "baz" }\n}`; + + const operations: Operation[ ] = [ + { op: 'add', path: '/fxx', value: { } }, + { op: 'move', from: '/foo/bar', path: '/fxx/bar' } + ]; + + const res = jsonPatch( before, operations ); + + expect( res ).toBe( after ); + } ); + + it( 'move object props deeply (non-flow source)', ( ) => + { + const before = `{\n "foo": {\n "bar": "baz"\n }\n}`; + const after = + `{\n "foo": { },\n "fxx": {\n "bar": "baz"\n }\n}`; + + const operations: Operation[ ] = [ + { op: 'add', path: '/fxx', value: { } }, + { op: 'move', from: '/foo/bar', path: '/fxx/bar' } + ]; + + const res = jsonPatch( before, operations ); + + expect( res ).toBe( after ); + } ); } ); } );