Use YAMLMap#add in internal map item additions

[aduth]

Related: #44

This proposal is associated with the addition of the new test case, where it appears that the new sortMapEntries option does not work as expected when parsing via YAML.parseDocument. From what I can tell, the creation of maps in the document parse flow bypasses YAMLMap#add behaviors by pushing directly to the items array, thereby missing the sorting implementation.

The test changes in tests/doc/anchors.js were not initially expected, though on further examination appear to be necessary? In other words, the expected output from the previous test fixtures do not align to how other parsers behave with the same input.

Example in Ruby (Psych):

irb(main):002:0> require 'yaml'; YAML.load("\nx:\n  - &a\n    k0: v1\n    k1: v1\n  - &b\n    k1: v2\n    k2: v2\ny:\n  k0: v0\n  <<: *a\n  <<: *b")
=> {"x"=>[{"k0"=>"v1", "k1"=>"v1"}, {"k1"=>"v2", "k2"=>"v2"}], "y"=>{"k0"=>"v1", "k1"=>"v2", "k2"=>"v2"}}

There are still test failures with these changes, where the current sequence implementation does not match the updated expected output. I wanted to push this up early to get a gut check on this proposal before I'd spend much time digging into this issue.

[eemeli]

Umm, sortMapEntries isn't meant to have any effect when parsing. Did you notice its description in the docs?

When stringifying, sort map entries. If true, sort by comparing key values using the native less-than < operator.

So I gather your proposal be that it should also apply when parsing, yes? I would be interested to hear the use case for this, to determine whether something like a reviver or a visitor could also serve the same need.

[aduth]

I don't have an opinion on whether sorting of keys be done during the parse, but I am interested in sorting keys in the stringified output.

Essentially, I'd like to take a document like this:

---
b: true
a: false

...and produce one which contains the same information, with keys sorted:

---
a: false
b: true

Based on my interpretation of the documentation, here's what I tried:

1. YAML.parseDocument(original, { sortMapEntries: true }).toString()
   • This retains the document structure, but does not sort the keys.
2. YAML.stringify(YAML.parseDocument(original), { sortMapEntries: true })
   • This sorts the keys, but drops details like directives, ---, comments, maybe more.

From what I could tell, the problem with the first option is that sortMapEntries is only considered in YAMLMap#add and createMap, neither of which is called during parseDocument or Document#toString.

I can see that it may not be ideal to sort keys during parse if the sorting is only relevant for a subsequent stringification, but in that case, would it make sense to handle sortMapEntries in the stringify end of the implementation instead?

[eemeli]

Okay, that clarifies your situation a bit. From what I gather, your need is to 1) parse a YAML document, 2) sort its maps, and 3) re-stringify it.

The best available tool for your use case is almost certainly YAML.visit(). It works like this:

const doc = YAML.parseDocument('[ 4, 2, 3, 1 ]')
YAML.visit(doc, { Seq(_, seq) { seq.items.sort() } })
doc.toString()
> '[ 1, 2, 3, 4 ]\n'

That's of course a simplified example, see the docs for more details. For a map, items is an array of { key, value } pairs, so you'll need to define a sort function that's appropriate for your use case. The default used by sortMapEntries is pretty simple:

const sortMapEntriesByKey = (a, b) => a.key < b.key ? -1 : a.key > b.key ? 1 : 0

Would that work for you?

[aduth]

Thanks for the tip, @eemeli. I was able to use your advice successfully, which ended up looking like...

const doc = YAML.parseDocument('---\nb: true\na: false')
YAML.visit(doc, { Map(_, { items }) { items.sort(new YAML.Schema({ sortMapEntries: true }).sortMapEntries); } })
doc.toString()
// '---\na: false\nb: true'

Still, my read of the documentation is that the original approach of passing { sortMapEntries: true } to parseDocument would sort the keys as well, since it mentions the option as being "used by" parseDocument. Is this a wrong interpretation on my part?

In any case, since I have a workable solution, and since it seems like if it were to work this way, it'd probably need to be implemented better than what I've proposed here, I'll go ahead and close the pull request.

[eemeli]

It's entirely possible that the docs could be improved. Could you specify a bit from where you got the understanding that the option was used by YAML.parseDocument?

[aduth]

The "Schema Options" section describes that the options, including sortMapEntries, are "Used by: [...] parseDocument".

It also shows up as a property of the second parameter via the included TypeScript types autocompletion.