Updated dependencies and introduced delete option with test

[kristianmandrup]

All tests pass. Note, when trying to upgrade to Esprima 3:

     Error: Line 1: Unexpected token ILLEGAL
      at ErrorHandler.constructError (lib/aesprim.js:3378:22)
      at ErrorHandler.createError (lib/aesprim.js:3396:27)
      at ErrorHandler.throwError (lib/aesprim.js:3404:21)
      at Scanner.throwUnexpectedToken (lib/aesprim.js:3487:28)
      at Scanner.scanPunctuator (lib/aesprim.js:4008:19)
      at Scanner.lex (lib/aesprim.js:4610:22)
      at Parser.nextToken (lib/aesprim.js:634:30)
      at new Parser (lib/aesprim.js:474:15)
      at Object.parse (lib/aesprim.js:115:19)
      at Handlers.subscript-child-filter_expression (lib/handlers.js:111:23)

So the aesprima hack, trying to inject @ as a valid identifier fails. Likely because it is now a valid symbol for decorators? So I had to downgrade to esprima 2 to make it work. Still better than before ;)

[IvanGoncharov]

@kristianmandrup Why do you need special :delete: value you can simply detect when callback returns undefined.

• What if I want to replace the value with literal :delete:.
  IMHO, it's bad practice to create magic values.

[IvanGoncharov]

@kristianmandrup I don't see where filter is used?

[kristianmandrup]

See in index.js apply and tests at the top of sugar.js. Also see the updated Readme delete examples for apply. I now just renamed to removeItem so there is less chance of a conflict with a regular application update.

[kristianmandrup]

See test example

I'm using this version in json-operator where I really need a good delete option.

[dchester]

I agree it might be nice to support conditionally removing nodes based on a supplied callback function, but do we need to overload jp.apply? How about adding a new method like jp.filter, possibly modeled after Array.prototype.filter?

Either way, I agree with @IvanGoncharov that we should be able to find an approach where special strings are not part of the solution. Even undefined or null seem to have the same problem, since those could be valid values themselves, no? From a quick look it appears this point also applies to the special removeItem key, since a user may legitimately wish to return an object with that key, without the intention of deleting the node. If we absolutely need a special value we could test for strict equality with something like jp.DELETE, where that would be a reference to an object owned by the library itself.

But can we just skip all of that by implementing jp.filter instead?

[kristianmandrup]

Sounds good with jp.filter :)

[kristianmandrup]

Even better solution would be to simply add a second argument context to the apply callback.
The context should be an object with {parent, key, index}. Then you can easily implement insert, filter, delete etc. using that context.

Something like this, except how about index if object is inside an Array?

var val = node.value = fn.call(obj, parent[key], {parent: parent, key: key});

Ah, of course, now I see, then key will be 0 since an Array id an Object :)

[kristianmandrup]

Added new conditional filter method with tests and Readme update. Added context object to callback fn.call(obj, parent[key], {parent: parent, key: key});

[IvanGoncharov]

@kristianmandrup @dchester I think after jp.filter people will want other methods from lodash/underscore.

• IMHO, filter is little bit misleading name I would expect it to return modified object instead of making changes in place.

How about more generic solution?
Add function which return Iterables with wrapper object(parent, index) as value. This wrapper object will have following methods: value, parent, index, remove, replace, etc.
Example:

for (let node of jp.iterable('$..b.c')) {
  if (node.value() === '')
     node.remove();
}

If you want more advanced operations you can use any lib that support iteratable.

[kristianmandrup]

Sounds great!! :) So far my fork supports the new filter function, which filters (removes from parent) if a truthy value is returned. It uses Array splice and Object delete, hence it no longer depends on any lodash function.

I'm all for a more flexible/generic solution like you propose... then I will change my json-operator API to support it :)
Cheers!

[IvanGoncharov]

@kristianmandrup @dchester
I forget that iterable is ES6, and it doesn't support in all browsers yet.
How about forEach function instead which execute callback with wrapper object?
Example:

jp.forEachNode('$..b.c', function (node) {
  //remove all empty strings
  if (node.value() === '')
     node.delete();
});

[kristianmandrup]

Sure, looks sensible :)

I'm not aware of the node API you are using. Would be nice to be documented.
Is there a parent ref? a remove or delete method or that is part of the proposal?

[IvanGoncharov]

Is there a parent ref? a remove or delete method or that is part of the proposal?

@kristianmandrup Yes, they are.

BTW. Can you please split up dependency update into separate PR.

[kristianmandrup]

"Yes they are." Documented where?? just been browsing original article and your code, couldn't find any mention of the node api, such as node.value etc. Only usage.
There is no dependency update anymore. Just squash my commit in case you want to merge it into dev branch for now.

[IvanGoncharov]

a remove or delete method or that is part of the proposal?

"Yes they are." Documented where??

Yes it is of part of my proposal to create this wrapper class:

function Node (parent, index) {
  this.parent = parent;
  this.index = index;  
}
Node.prototype.value = function () {
  return this.parent[this.index];
}
Node.prototype.remove = function () {
  if (Array.isArray(this.parent))
    Array.prototype.splice.call(this.parent, parseInt(this.index), 1);
  else
    delete this.parent[this.index];
}
// Other methods

And use it as a parameter to the callback in forEachNode.

[kristianmandrup]

Wow! Looks perfect :)

[tomByrer]

Adding delete seems interesting; what is the status please?

[soujiro32167]

bump! Would love this functionality