Skip to content

Commit

Permalink
API BREAK: Diff a/b logic fix for insert & delete
Browse files Browse the repository at this point in the history
Fixed a ksconf.conf.delta glitch where a/b were backwards for DIFF_OP_INSERT
and DIFF_OP_DELETE.  This correction could impact any 3rd party code that
relies on the old behavior.  All built-in commands have been updated.

I'm confident this fixes #50.  Most likely this root issue of diff weirdness
and so it likely fixes #18.  Keep an eye open for regressions.
  • Loading branch information
lowell80 committed Jun 5, 2019
1 parent e84144f commit 1c9a974
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 18 deletions.
6 changes: 6 additions & 0 deletions docs/source/changelog.rst
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ Release v0.7.3 (DRAFT)
- Improved some resource allocation in corner cases.


.. attention:: **API BREAKAGE**

The ``DiffOp`` output values for ``DIFF_OP_INSERT`` and ``DIFF_OP_DELETE`` have been changed in a backwards-compatible breaking way.
The values of ``a`` and ``b`` were previously reversed for these two operations, leading to some code confusion.


Release v0.7.2 (2019-03-22)
~~~~~~~~~~~~~~~~~~~~~~~~~~~

Expand Down
4 changes: 2 additions & 2 deletions ksconf/commands/promote.py
Original file line number Diff line number Diff line change
Expand Up @@ -340,13 +340,13 @@ def prompt_yes_no(prompt):
# Move entire stanza
show_diff(self.stdout, [op])
if prompt_yes_no("Apply [{0}]".format(op.location.stanza)):
out_cfg[op.location.stanza] = op.a
out_cfg[op.location.stanza] = op.b
del out_src[op.location.stanza]
else:
show_diff(self.stdout, [op])
if prompt_yes_no("Apply [{0}] {1}".format(op.location.stanza, op.location.key)):
# Move key
out_cfg[op.location.stanza][op.location.key] = op.a
out_cfg[op.location.stanza][op.location.key] = op.b
del out_src[op.location.stanza][op.location.key]
# If last remaining key in the src stanza? Then delete the entire stanza
if not out_src[op.location.stanza]:
Expand Down
20 changes: 10 additions & 10 deletions ksconf/conf/delta.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ def compare_stanzas(a, b, stanza_name):
return [DiffOp(DIFF_OP_EQUAL, DiffStanza("stanza", stanza_name), a, b) ]
elif not b:
# A only
return [ DiffOp(DIFF_OP_DELETE, DiffStanza("stanza", stanza_name), None, a) ]
return [ DiffOp(DIFF_OP_DELETE, DiffStanza("stanza", stanza_name), a, None) ]
elif not a:
# B only
return [ DiffOp(DIFF_OP_INSERT, DiffStanza("stanza", stanza_name), b, None) ]
return [ DiffOp(DIFF_OP_INSERT, DiffStanza("stanza", stanza_name), None, b) ]
else:
return list(_compare_stanzas(a, b, stanza_name))

Expand All @@ -72,9 +72,9 @@ def _compare_stanzas(a, b, stanza_name):

# Level 2 - Key comparisons
for key in kv_a:
yield DiffOp(DIFF_OP_DELETE, DiffStzKey("key", stanza_name, key), None, a[key])
yield DiffOp(DIFF_OP_DELETE, DiffStzKey("key", stanza_name, key), a[key], None)
for key in kv_b:
yield DiffOp(DIFF_OP_INSERT, DiffStzKey("key", stanza_name, key), b[key], None)
yield DiffOp(DIFF_OP_INSERT, DiffStzKey("key", stanza_name, key), None, b[key])
for key in kv_common:
a_ = a[key]
b_ = b[key]
Expand Down Expand Up @@ -155,10 +155,10 @@ def compare_cfgs(a, b, allow_level0=True):
delta.extend(_compare_stanzas(a[stanza], b[stanza], stanza))
elif stanza in a:
# A only
delta.append(DiffOp(DIFF_OP_DELETE, DiffStanza("stanza", stanza), None, a[stanza]))
delta.append(DiffOp(DIFF_OP_DELETE, DiffStanza("stanza", stanza), a[stanza], None))
else:
# B only
delta.append(DiffOp(DIFF_OP_INSERT, DiffStanza("stanza", stanza), b[stanza], None))
delta.append(DiffOp(DIFF_OP_INSERT, DiffStanza("stanza", stanza), None, b[stanza]))
return delta


Expand Down Expand Up @@ -303,9 +303,9 @@ def f(v):
for op in diffs:
if isinstance(op.location, DiffStanza):
if op.tag in (DIFF_OP_DELETE, DIFF_OP_REPLACE):
show_value(op.b, op.location.stanza, None, "-")
show_value(op.a, op.location.stanza, None, "-")
if op.tag in (DIFF_OP_INSERT, DIFF_OP_REPLACE):
show_value(op.a, op.location.stanza, None, "+")
show_value(op.b, op.location.stanza, None, "+")
continue # pragma: no cover (peephole optimization)

if op.location.stanza != last_stanza:
Expand All @@ -318,9 +318,9 @@ def f(v):
last_stanza = op.location.stanza

if op.tag == DIFF_OP_INSERT:
show_value(op.a, op.location.stanza, op.location.key, "+")
show_value(op.b, op.location.stanza, op.location.key, "+")
elif op.tag == DIFF_OP_DELETE:
show_value(op.b, op.location.stanza, op.location.key, "-")
show_value(op.a, op.location.stanza, op.location.key, "-")
elif op.tag == DIFF_OP_REPLACE:
if is_multiline(op.a) or is_multiline(op.b):
show_multiline_diff(op.a, op.b, op.location.key)
Expand Down
13 changes: 7 additions & 6 deletions tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -410,13 +410,14 @@ def test_compare_keys_props(self):

op = self.find_op_by_location(diffs, "key", stanza="imapsync", key="DATETIME_CONFIG")
self.assertEqual(op.tag, DIFF_OP_DELETE)
self.assertIsNone(op.a)
self.assertIsNotNone(op.b)
self.assertIsNotNone(op.a)
self.assertIsNone(op.b)

op = self.find_op_by_location(diffs, "key", stanza="imapsync", key="description")
self.assertEqual(op.tag, DIFF_OP_INSERT)
self.assertIsNotNone(op.a)
self.assertIsNone(op.b)
self.assertIsNone(op.a)
self.assertIsNotNone(op.b)


def test_imballanced_stanas(self):
""" Imbalanced stanzas """
Expand Down Expand Up @@ -487,11 +488,11 @@ def test_empty_stanzas(self):

op = delta_search("stanza", stanza="in_b1")
self.assertEqual(op.tag, DIFF_OP_INSERT)
self.assertEqual(op.a["live_in"], "b")
self.assertEqual(op.b["live_in"], "b")

op = delta_search("stanza", stanza="in_a1")
self.assertEqual(op.tag, DIFF_OP_DELETE)
self.assertEqual(op.b["live_in"], "a")
self.assertEqual(op.a["live_in"], "a")

def test_summarize_compare_results(self):
c1 = parse_string(self.cfg_props_imapsync_1)
Expand Down

0 comments on commit 1c9a974

Please sign in to comment.