Skip to content

Commit

Permalink
Rename diff format actions/ops to more verbose words similar to JSON …
Browse files Browse the repository at this point in the history
…Patch format.

For dicts, these represent the same type of action as JSON Patch:
    "add", "remove", "replace"
For sequences, "add" and "remove" are replaced with batch operations:
    "addrange", "removerange"
For both dicts and sequences, the recursive "patch" operation does not
exist in JSON Patch.
  • Loading branch information
Martin Sandve Alnæs committed Jan 15, 2016
1 parent 57461e9 commit d8c258c
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 36 deletions.
17 changes: 7 additions & 10 deletions docs/source/diffing.rst
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,10 @@ A diff of two dicts is a list of diff entries:

A dict diff entry is a list of action and argument (except for deletion):

* ["-", key]: delete existing value at key
* ["+", key, value]: insert new value at key
* [":", key, value]: replace value at key with newvalue
* ["!", key, diff]: patch value at key with diff
* ["remove", key]: delete existing value at key
* ["add", key, value]: insert new value at key
* ["replace", key, value]: replace value at key with newvalue
* ["patch", key, diff]: patch value at key with diff

Insert is an error if the key already exists.
Delete, replace and patch are errors if the key does not already exist.
Expand All @@ -72,10 +72,7 @@ A diff of two sequences is an ordered list of diff entries:

A sequence diff entry is a list of action, index and argument (except for deletion):

* ["-", index]: delete entry at index
* ["+", index, newvalue]: insert single newvalue before index
* ["--", index, n]: delete n entries starting at index
* ["++", index, newvalues]: insert sequence newvalues before index
* ["!", index, diff]: patch value at index with diff
* ["addrange", index, n]: delete n entries starting at index
* ["removerange", index, newvalues]: insert sequence newvalues before index
* ["patch", index, diff]: patch value at index with diff

Possible simplification: Remove single-item insert/delete for sequences.
31 changes: 22 additions & 9 deletions nbdime/dformat.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,12 @@


# Valid values for the action field in diff entries
PATCH = "!"
INSERT = "+"
DELETE = "-"
REPLACE = ":"
SEQINSERT = "++"
SEQDELETE = "--"
INSERT = "add"
DELETE = "remove"
REPLACE = "replace"
PATCH = "patch"
SEQINSERT = "addrange"
SEQDELETE = "removerange"

ACTIONS = [
PATCH,
Expand All @@ -28,6 +28,19 @@
SEQDELETE,
]

SEQUENCE_ACTIONS = [
SEQINSERT,
SEQDELETE,
PATCH
]

MAPPING_ACTIONS = [
INSERT,
DELETE,
REPLACE,
PATCH
]


sequence_types = string_types + (list,)

Expand All @@ -54,7 +67,7 @@ def validate_diff_entry(s, deep=False):
The diff entry format is a list
s[0] # action (one of PATCH, INSERT, DELETE, REPLACE)
s[1] # key (str for diff of dict, int for diff of sequence (list or str))
s[2] # action specific argument, omitted if action is "-"
s[2] # action specific argument, omitted if action is DELETE
For sequences (lists and strings) the actions
SEQINSERT and SEQDELETE are also allowed.
Expand All @@ -72,8 +85,8 @@ def validate_diff_entry(s, deep=False):
if not (is_sequence or is_mapping):
raise NBDiffFormatError("Diff entry key '{}' has type '{}', expecting int or unicode/str.".format(s[1], type(s[1])))

# Experimental sequence diff actions ++, --, :: are not valid for mapping diffs
if is_mapping and len(s[0]) > 1:
# Sequence diff actions ++, --, :: are not valid for mapping diffs
if is_mapping and s[0] not in MAPPING_ACTIONS:
raise NBDiffFormatError("Diff action '{}' only valid in diff of sequence.".format(s[0]))

if s[0] == INSERT:
Expand Down
2 changes: 1 addition & 1 deletion nbdime/merging/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -432,7 +432,7 @@ def merge(base, local, remote):
Takeaways:
- Ensure that diff always uses patch on collections unless the type changes and replace on values.
- The only recursion will happen on the patch / patch action of equal type collections!
- Patch action is ["!", key, subdiff], providing subdiff for both sides, and meaning values exist on both sides.
- Patch action is [PATCH, key, subdiff], providing subdiff for both sides, and meaning values exist on both sides.
## Next trying to figure out list situations:
Expand Down
8 changes: 6 additions & 2 deletions nbdime/prettyprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ def present_dict_diff(a, d, path):
for key in keys:
e = d[key]
action = e[0]
arg = e[1] if len(e) > 1 else None

nextpath = "/".join((path, key))

Expand All @@ -39,15 +38,18 @@ def present_dict_diff(a, d, path):
pp += present_value("- ", a[key])

elif action == INSERT:
arg = e[1]
pp.append("insert at {}:".format(nextpath))
pp += present_value("+ ", arg)

elif action == REPLACE:
arg = e[1]
pp.append("replace at {}:".format(nextpath))
pp += present_value("- ", a[key])
pp += present_value(" +", arg)

elif action == PATCH:
arg = e[1]
pp.append("patch {}:".format(nextpath))
pp += present_diff(a[key], arg, nextpath)

Expand All @@ -63,15 +65,16 @@ def present_list_diff(a, d, path):
for e in d:
action = e[0]
index = e[1]
arg = e[2] if len(e) > 2 else None

nextpath = "/".join((path, str(index)))

if action == SEQINSERT:
arg = e[2]
pp.append("insert before {}:".format(nextpath))
pp += present_value("+ ", arg)

elif action == SEQDELETE:
arg = e[2]
if arg > 1:
r = "{}-{}".format(index, index+arg-1)
else:
Expand All @@ -80,6 +83,7 @@ def present_list_diff(a, d, path):
pp += present_value("- ", a[index:index+arg])

elif action == PATCH:
arg = e[2]
pp.append("patch {}:".format(nextpath))
pp += present_diff(a[index], arg, nextpath)

Expand Down
17 changes: 9 additions & 8 deletions nbdime/tests/test_merge.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import pytest

from nbdime import merge
from nbdime.dformat import PATCH, INSERT, DELETE, REPLACE, SEQINSERT, SEQDELETE


def cut(li, *indices):
Expand Down Expand Up @@ -416,15 +417,15 @@ def test_merge_conflicting_nested_dicts():
r = {"a": {"x": 3, "y": 5}, "d": {"x": 5}, "m": {"x": 27}, "n": {"q": 19}}
m, lc, rc = merge(b, l, r)
assert m == {"a": {}, "d": {}, "m": {}, "n": {}}
assert lc == {"a": ["!", {"x": [":", 2], "y": ["+", 4]}],
"d": ["!", {"x": ["-"], "y": [":", 6]}],
"m": ["!", {"x": [":", 17]}],
"n": ["!", {"q": ["+", 9]}],
assert lc == {"a": [PATCH, {"x": [REPLACE, 2], "y": [INSERT, 4]}],
"d": [PATCH, {"x": [DELETE], "y": [REPLACE, 6]}],
"m": [PATCH, {"x": [REPLACE, 17]}],
"n": [PATCH, {"q": [INSERT, 9]}],
}
assert rc == {"a": ["!", {"x": [":", 3], "y": ["+", 5]}],
"d": ["!", {"x": [":", 5], "y": ["-"]}],
"m": ["!", {"x": [":", 27]}],
"n": ["!", {"q": ["+", 19]}],
assert rc == {"a": [PATCH, {"x": [REPLACE, 3], "y": [INSERT, 5]}],
"d": [PATCH, {"x": [REPLACE, 5], "y": [DELETE]}],
"m": [PATCH, {"x": [REPLACE, 27]}],
"n": [PATCH, {"q": [INSERT, 19]}],
}
#assert c == {"a": {"x": [1, 2, 3], "y": [None, 4, 5]}, "d": {"x": [4, None, 5], "y": [5, 6, None]},
# "m": {"x": [7, 17, 27]}}
12 changes: 6 additions & 6 deletions webapp/static/nbdime.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@

// from the nbdime diff format:
// Valid values for the action field in diff entries
var PATCH = "!";
var INSERT = "+";
var DELETE = "-";
var REPLACE = ":";
var SEQINSERT = "++";
var SEQDELETE = "--";
var INSERT = "add";
var DELETE = "remove";
var REPLACE = "replace";
var PATCH = "patch";
var SEQINSERT = "addrange";
var SEQDELETE = "removerange";


function convert_merge_data(data) {
Expand Down

0 comments on commit d8c258c

Please sign in to comment.