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

Removal of the root #17

Open
espadrine opened this issue Aug 23, 2016 · 2 comments
Open

Removal of the root #17

espadrine opened this issue Aug 23, 2016 · 2 comments

Comments

@espadrine
Copy link

espadrine commented Aug 23, 2016

The patch { "op": "remove", "path": "" } is currently undefined.

I believe it should have the same result as { "op": "add", "path": "", "value": null }. As a side-result, it would also validate the definition of { "op": "replace", "path": "", "value": null }, which states:

This operation is functionally identical to a "remove" operation for
a value, followed immediately by an "add" operation at the same
location with the replacement value.

In any case, it would benefit from being specified. Some implementations currently throw, and that is unfortunate, as it breaks the specification as quoted above..

@d-frey
Copy link

d-frey commented Sep 2, 2016

I disagree with the solution. I think { "op": "remove", "path": "" } should be an error. If you want to replace the root element, you can already use { "op": "add", "path": "", "value": null } today. Even that is strange.

It could be reasonable to allow the replace to work with an empty path to replace the element completely, and, compatibility aside, disallow add to do the same. This would provide the most intuitive semantics. add and remove require a non-empty path, replace is able to replace the root.

But since we most likely want to be compatible, I think add should stay as it is, replace should explicitly allow to replace the root and remove on the root should be an error.

@espadrine
Copy link
Author

What matters most is that the undefined behavior gets specified.

That being said, if we go with your solution, we lose the functional equivalence of replace to remove + add, which will be a bit counterintuitive.

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

No branches or pull requests

2 participants