From 289bc7114c01f9c4551dbc03dc7e27ed98b4f4ee Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sun, 1 Nov 2015 03:29:35 +0100 Subject: [PATCH] implement mutation of the current element (_THIS_) while implementing this, I've discovered a conceptual error: we allow to accept attributes, even when we've already entered the child scope. This means that we can not predictable get back at the "last" (i.e. the currently touched) element, because this might be such an attribute. So a really correct implementation would have to memorise the "current" element, which is really tricky, given the various ways of touching elements in our diff language. In the end I've decided to ignore this problem (maybe a better solution would have been to disallow those "late" attributes?) My reasoning is that attributes are unlikely to be full records, rather just values, and values are never mutated. (but note that it is definitively possible to have an record as attribute!) --- src/lib/diff/record.hpp | 19 +++++++++++++++++++ src/lib/diff/tree-diff-application.hpp | 18 ++++++++++++++++-- .../diff/diff-tree-application-test.cpp | 2 +- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/lib/diff/record.hpp b/src/lib/diff/record.hpp index 73754d6f1..a768a22ef 100644 --- a/src/lib/diff/record.hpp +++ b/src/lib/diff/record.hpp @@ -474,6 +474,25 @@ namespace diff{ alteredContent.resetPos(); } + /** get the tail element. + * @return either the last child, or the last attribute, when children are empty. + * @note typically this might be used to get back at the element "just added", + * as when muting a child node in diff application. But there is a loophole: + * we might have added an attribute even when there are already children. + */ + VAL const& + accessLast() + { + if (record_.empty()) + throw error::State("Record is empty, unable to access (last) element."); + + if (record_.children_.empty()) + return record_.attribs_.back(); + else + return record_.children_.back(); + } + + /* === extension point for building specific value types === */ /* * the following builder functions need to be specialised diff --git a/src/lib/diff/tree-diff-application.hpp b/src/lib/diff/tree-diff-application.hpp index 5ab996f4b..53152f88c 100644 --- a/src/lib/diff/tree-diff-application.hpp +++ b/src/lib/diff/tree-diff-application.hpp @@ -204,6 +204,20 @@ namespace diff{ GenNode const& find_child (GenNode::ID const& idi) { + if (alteredRec().empty()) + throw error::State(_Fmt("Attempt to mutate element %s, but current target data scope is empty. " + "Sender and receiver out of sync?") % idi.getSym() + , LUMIERA_ERROR_DIFF_CONFLICT); + + // Short-cut-mutation: look at the last element. + // this should be the one just added. BUT NOTE: this fails + // when adding an attribute after entering the child scope. + // Since attributes are typically values and not mutated, + // this inaccuracy was deemed acceptable + auto& current = out().accessLast(); + if (Ref::THIS.matches(idi) or current.matches(idi)) + return current; + for (auto & child : alteredRec()) if (child.idi == idi) return child; @@ -217,7 +231,7 @@ namespace diff{ move_into_new_sequence (Iter pos) { if (src().currIsAttrib()) - out().appendAttrib (move(*pos)); + out().appendAttrib (move(*pos)); //////////////TICKET #969 was it a good idea to allow adding attributes "after the fact"? else out().appendChild (move(*pos)); } @@ -233,7 +247,7 @@ namespace diff{ if (n.isTypeID()) out().setType (n.data.get()); else - out().appendAttrib(n); + out().appendAttrib(n); //////////////TICKET #969 dto. else { out().appendChild(n); diff --git a/tests/library/diff/diff-tree-application-test.cpp b/tests/library/diff/diff-tree-application-test.cpp index 0c1508d6c..fbde445d3 100644 --- a/tests/library/diff/diff-tree-application-test.cpp +++ b/tests/library/diff/diff-tree-application-test.cpp @@ -113,7 +113,7 @@ namespace test{ , skip(CHILD_T) , del(CHILD_T) , pick(Ref::CHILD) // pick a child anonymously - , mut(Ref::THIS) + , mut(Ref::THIS) // mutate the current element (the one just picked) , ins(ATTRIB3) , ins(ATTRIB_NODE) , find(CHILD_A)