microsoft/node-jsonc-parser

Clarify whether / how `Edit[]` can be concatenated

Marcono1234 opened this issue · 3 comments

The documentation currently does not make it clear whether or how Edit[] from multiple modify() or format() calls can be concatenated before being applied using applyEdits().

Both the documentation for modify() and format() contains the following:

Edits can be either inserts, replacements or removals of text segments. All offsets refer to the original state of the document. No two edits must change or remove the same range of text in the original document. However, multiple edits can have the same offset, for example multiple inserts, or an insert followed by a remove or replace. The order in the array defines which edit is applied first.

Personally I think this documentation should be moved to applyEdits() respectively to the documentation of the type Edit instead since it is more relevant there.

The issue is that the output of multiple modify() calls cannot be concatenated without risking to produce malformed JSON when calling applyEdits(). Here are two examples (from #48 (comment)):

  • Removing subsequent array elements / object properties.
    This will cause both elements / properties to remove the same ,, which in the end results in one character too much being removed:
    const json = '{"a":1,"b":2}'
    let edits = jsoncParser.modify(json, ["a"], undefined, {})
    edits = edits.concat(jsoncParser.modify(json, ["b"], undefined, {}))
    
    // Prints only `{`; the closing `}` is missing
    console.log(jsoncParser.applyEdits(json, edits))
  • Removing array elements / object properties in reverse order:
    const json = '{"a":1,"b":2,"c":3}'
    let edits = jsoncParser.modify(json, ["c"], undefined, {})
    edits = edits.concat(jsoncParser.modify(json, ["a"], undefined, {}))
    
    // Prints `{"b":2,"c":3`
    console.log(jsoncParser.applyEdits(json, edits))

So currently the only safe usage of modify() and applyEdits() is to call both functions for every modification you want to make. It is not safe to try concatenating the result of multiple modify() calls. This is rather inefficient when a user wants to remove multiple properties.

In case this is intended, then it should ideally be clarified in the documentation for the modify() (and format()?) function.

The results of multiple modify (and or format) operations can't be concatenated unless you are sure they don't impact each other.

A different API is needed that would let you compute the text operations for more than one JSON modification. I don't have plans to implement such functionality.

I tried to improve the spec around edit operations.

Thanks! Would it be possible to add one more sentence to the EditResult documentation, mentioning what you wrote in your comment? That is (for example):

In general multiple EditResults must not be concatenated because they might impact each other, producing incorrect or malformed JSON documents.

I think even with your improvements that is currently still not clear.

Thanks for the suggestion, I added it!