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

Fix application of '-' pointer in array #23

Closed
amccausl opened this issue Mar 23, 2014 · 2 comments · Fixed by #26
Closed

Fix application of '-' pointer in array #23

amccausl opened this issue Mar 23, 2014 · 2 comments · Fixed by #26

Comments

@amccausl
Copy link
Contributor

Excerpt from JSON-patch spec (section 4.1):

An element to add to an existing array - whereupon the supplied
      value is added to the array at the indicated location.  Any
      elements at or above the specified index are shifted one position
      to the right.  The specified index MUST NOT be greater than the
      number of elements in the array.  If the "-" character is used to
      index the end of the array (see [RFC6901]), this has the effect of
      appending the value to the array.
var obj = { arr: [ "item 1", "item 2" ] }
var patch = [ { op: 'add', path: '/arr/-', value: 'item 3' } ]

// obj should be { arr: [ "item 1", "item 2", "item 3" ] }
// but is { arr: [ "item 3", "item 1", "item 2" ] }

I think it's reasonable to not generate hyphen patches, but it should be possible to apply them.

I think change to https://github.com/Starcounter-Jack/JSON-Patch/blob/master/src/json-patch.js#L91 would work as follows:

var index = keys[t] === '-' ? obj.length : parseInt(keys[t], 10);

I can create formal pull request with updated tests if you would prefer. What's your build process?

@warpech
Copy link
Collaborator

warpech commented Apr 3, 2014

Sorry for late response. This is a great catch! Would you find time to implement that?

If so, please do so on a separate branch and create a pull request. The pull request should contain:

  • changes in export function apply in json-patch-duplex.ts, plus a in the JS file (compiled using tsc)
  • tests in test-duplex.js

Then it can be tested as defined in README.md

If you can't find time, we will implement it one day but I cannot give exact date yet.

@amccausl
Copy link
Contributor Author

amccausl commented Apr 3, 2014

Sweet. I will prep change this evening. Thanks for the great work on this project :)

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 a pull request may close this issue.

2 participants