Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Drop type property from all but Scalar nodes #240

Merged
merged 6 commits into from Mar 12, 2021
Merged

Drop type property from all but Scalar nodes #240

merged 6 commits into from Mar 12, 2021

Conversation

eemeli
Copy link
Owner

@eemeli eemeli commented Mar 12, 2021

Originally, type was a thing that was carried into the AST nodes from the previous CST parser. Now, it's a clumsy & sometimes misleading API. So let's drop it elsewhere, but leave it for scalars where it still makes good sense. This is yet another BREAKING CHANGE.

For scalars, let's move the constants from e.g. Type.BLOCK_LITERAL to Scalar.BLOCK_LITERAL. Otherwise, everything stays pretty much the same.

For collections, we need to add a new flow: boolean property that can now be more easily set or toggled to control whether to use block or flow representation. As before with type: 'FLOW_MAP' | 'FLOW_SEQ', setting flow: true will force that choice for all of its child nodes as well.

Dropping the type values also made it clear that Merge is no longer a good idea as a separate class; dropping it allows for merge nodes to be duck-typed when merge: true or version: '1.1' is set.

-import { Scalar, Type, YAMLMap, YAMLSeq } from 'yaml'
+import { isMap, isSeq, Scalar, YAMLMap, YAMLSeq } from 'yaml'

let scalar: Scalar
-scalar.type = Type.BLOCK_LITERAL
+scalar.type = Scalar.BLOCK_LITERAL

let map: YAMLMap
-map.type = Type.FLOW_MAP
+map.flow = true
-const mapIsMap = map.type === Type.FLOW_MAP || map.type === Type.MAP // only if parsed
+const mapIsMap = isMap(map)

let seq: YAMLSeq
-seq.type = Type.FLOW_SEQ
+seq.flow = true
-const seqIsSeq = seq.type === Type.FLOW_SEQ || seq.type === Type.SEQ // only if parsed
+const seqIsSeq = isSeq(seq)

BREAKING CHANGE: YAMLMap, YAMLSeq, and Document no longer provide a
`type` member with a value in FLOW_MAP | FLOW_SEQ | MAP | SEQ | DOCUMENT.
Instead, YAMLMap & YAMLSeq now provide a boolean `flow` member that is
set during composition, and which controls the flow/block styling of its
stringified representation.
BREAKING CHANGE: The Merge class is no more.
BREAKING CHANGE: As all the other type values have been dropped, it's
clearer to drop the generic Type, and to use Scalar instead:

-import { Type } from 'yaml'
-scalar.type = Type.BLOCK_LITERAL
+import { Scalar } from 'yaml'
+scalar.type = Scalar.BLOCK_LITERAL
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant