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

Allow custom JSON pointer class #114

Merged
merged 12 commits into from Nov 23, 2020

Conversation

tzoiker
Copy link

@tzoiker tzoiker commented Nov 12, 2020

@stefankoegl It would be nice to be able to pass a custom implementation of the JSON pointer.

@coveralls
Copy link

coveralls commented Nov 15, 2020

Coverage Status

Coverage increased (+0.1%) to 65.953% when pulling eca4f8a on tzoiker:feature/custom-pointer into 24b5e86 on stefankoegl:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.05%) to 64.989% when pulling 124eb76 on tzoiker:feature/custom-pointer into 4fe5c2c on stefankoegl:master.

@tzoiker
Copy link
Author

tzoiker commented Nov 16, 2020

@stefankoegl is it possible to accept soon? I want to use it in another project and avoid even temporary copy-pasting.

Copy link
Collaborator

@Alanscut Alanscut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, would you mind adding more tests for custom Pointer which is not extended from python-json-pointer?
Just for curiosity, can't python-json-pointer meet your current requirment?

tests.py Outdated Show resolved Hide resolved
@tzoiker
Copy link
Author

tzoiker commented Nov 17, 2020

LGTM, would you mind adding more tests for custom Pointer which is not extended from python-json-pointer?
Just for curiosity, can't python-json-pointer meet your current requirment?

Added a toy example. The idea was to at least be able to extend (or replace with the same, but custom implementation in C or whatever). In my particular case, I wanted to add an extension (out of spec, obviously) that allows to dynamically match array indexes based on the nested values of array elements (something like /@foo="bar"/val instead of /1/val for [{"foo": "baz", "val": 0}, {"foo": "bar", "val": 1}}]).

@Alanscut
Copy link
Collaborator

Nice work! it will make this lib more flexiable.

@stefankoegl
Copy link
Owner

@tzoiker, looks good. Please revert the change of version number - that will be done when a new release is prepared.

Could you please also briefly describe your use case?

jsonpatch.py Outdated Show resolved Hide resolved
@tzoiker
Copy link
Author

tzoiker commented Nov 17, 2020

@tzoiker, looks good. Please revert the change of version number - that will be done when a new release is prepared.

Could you please also briefly describe your use case?

Described it in this comment #114 (comment)

Hmm, for some reason coverage decreased after I reverted the package version.

Copy link
Owner

@stefankoegl stefankoegl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now. I just merged another pull request, so there's a conflict now. If you can resolve that, I can merge right afterwards.

# Conflicts:
#	jsonpatch.py
#	tests.py
@stefankoegl stefankoegl merged commit 3a95635 into stefankoegl:master Nov 23, 2020
@stefankoegl
Copy link
Owner

I have just released v1.27 which inlcudes this PR. Thanks for the contribution!

@tzoiker tzoiker deleted the feature/custom-pointer branch November 23, 2020 20:18
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 this pull request may close these issues.

None yet

4 participants