Repository navigation
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Regression tests are needed to validate custom path generation and list-root replacement.
2 open issues
What changed in this PR
Adds configurable path generation to support alternative path syntaxes alongside custom pointer classes.
Changes:
- Exposes
path_jointhrough patch-generation APIs and uses it throughout diff construction. - Allows root-level
addoperations to replace list documents.
| File | Description |
|---|---|
| jsonpatch.py | Adds configurable path joining and handles list-root replacement. |
Review effort: Balanced
| if part is None: | ||
| obj = value # we're replacing the root |
| def from_diff( | ||
| cls, src, dst, optimization=True, dumps=None, | ||
| pointer_cls=JsonPointer, | ||
| pointer_cls=JsonPointer, path_join=_path_join, |
There was a problem hiding this comment.
Thanks, but I don’t think this is appropriate for this PR. Properly testing a custom path_join would require introducing another path format and an additional dependency, which is not justified for the current scope of the project.
The path_join hook is an extension point, and I’d rather not add a new format/dependency just to satisfy this coverage request. If maintainers explicitly want coverage, I can add a minimal in-test stub joiner/pointer class without a new dependency, but I don’t plan to add a new format as part of this change. Resolving as out of scope for now.
There was a problem hiding this comment.
I don't see it as necessary to add a whole format with new dependencies. However it would be helpful to have at least a bare minimum example, so see whether the function is correctly called in all required places.
JsonPatch.from_diff and make_patch accepted a pointer_cls, but always wrote RFC 6901 paths and then parsed them with pointer_cls. A pointer class with another path syntax therefore produced patches that could not be applied, even to the source document. DiffBuilder now writes paths with path_join(parts, pointer_cls). This covers the use case of #189 (other path styles in generated patches) without a separate path_join argument that would have to be kept in sync with pointer_cls. path_join no longer uses JsonPointer.from_parts, which always builds an RFC 6901 string and parses it with pointer_cls. Instead it sets the parts of a pointer_cls instance and returns its path, so a pointer class with another syntax only has to override __init__ and path. JsonPointer itself keeps the previous fast path. Refs #95, #189 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PewzphboJ9XrLY2h8AJ9iN
PatchOperation.path and MoveOperation.from_path joined the parts before the key with a plain '/': without a leading '/', without escaping keys that contain '/' or '~', and ignoring the syntax of pointer_cls. For '/a~1b/0/c' they returned 'a/b/0', which is a different location. They now return the location of the parent as a path, written with path_join in the syntax of pointer_cls: '/a~1b/0' for the example above, and '' for a target at the top level, as before. This changes the result for all nested paths, e.g. 'a/0' becomes '/a/0'. Refs #95, #189 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PewzphboJ9XrLY2h8AJ9iN
|
Closing as #256 was just released in 1.45. https://python-json-patch.readthedocs.io/en/latest/tutorial.html#custom-path-syntax should cover your use case. |
I need to add xpath support for my project. It can add support to another json path style