From dd016667adb6ec2298cc9f24967a815cb1abdbe7 Mon Sep 17 00:00:00 2001 From: Ichthyostega Date: Sun, 15 Mar 2020 23:11:14 +0100 Subject: [PATCH] Diff-Listener: add a variant to trigger also on value assignment (see #1206) As it turned out, it is rather easy to extend the existing listener for structural changes to detect also value assignments. Actually it seems we'd need both flavours, so be it. --- .../diff/tree-mutator-listener-binding.hpp | 55 +++++++++++++++---- src/lib/diff/tree-mutator.hpp | 10 +++- .../diff/diff-tree-mutation-listener-test.cpp | 25 +++++++-- wiki/thinkPad.ichthyo.mm | 47 ++++++++++++---- 4 files changed, 109 insertions(+), 28 deletions(-) diff --git a/src/lib/diff/tree-mutator-listener-binding.hpp b/src/lib/diff/tree-mutator-listener-binding.hpp index 071ab3e88..bed507f06 100644 --- a/src/lib/diff/tree-mutator-listener-binding.hpp +++ b/src/lib/diff/tree-mutator-listener-binding.hpp @@ -60,23 +60,40 @@ namespace diff{ /** * Decorator for TreeMutator bindings, to fire a listener function - * when the applied diff describes a structural change. By convention, - * all changes pertaining the sequence of children count as structural. - * Thus, effectively a structural change incurs usage of the `INS`, `DEL` - * `SKIP` or `FIND` verbs, which in turn are translated into the three - * API operation intercepted here. + * when the applied diff describes a relevant change. Changes can be + * _structural,_ they can be _value mutations_ or _child mutations._ + * By convention, all changes pertaining the sequence of children are + * classified as structural changes. Thus, effectively a structural + * change incurs usage of the `INS`, `DEL`, `SKIP` or `FIND` verbs, + * which in turn will be translated into the three API operations + * intercepted here in the basic setup. When value assignments + * count as "relevant", then we'll also have to intercept the + * `assignElm` API operation. However, the relevance of + * mutations to child elements is difficult to assess + * on this level, since we can not see what a nested + * scope actually does to the mutated child elements. + * @tparam assign also trigger on assignments in addition to + * structural changes (which will always trigger). + * Defaults to `false` * @note TreeMutator is a disposable one-way object; * the triggering mechanism directly relies on that. * The listener is invoked, whenever a scope is complete, * including processing of any nested scopes. */ - template + template class Detector4StructuralChanges : public PAR { LIS changeListener_; bool triggered_ = false; + void + trigger(bool relevant =true) + { + if (relevant) + triggered_ = true; + } + public: Detector4StructuralChanges (LIS functor, PAR&& chain) : PAR(std::forward(chain)) @@ -86,9 +103,9 @@ namespace diff{ // move construction allowed and expected to happen Detector4StructuralChanges (Detector4StructuralChanges&&) =default; - /** once the diff is for this level is completely applied, - * the TreeMutator will be discarded, and we can fire our - * change listener at that point. */ + /** once the diff for this level is completely applied, + * the TreeMutator will be discarded, and we can fire + * our change listener at that point. */ ~Detector4StructuralChanges() { if (triggered_) @@ -99,9 +116,10 @@ namespace diff{ using Elm = GenNode const&; - bool injectNew (Elm elm) override { triggered_ = true; return PAR::injectNew (elm); } - bool findSrc (Elm elm) override { triggered_ = true; return PAR::findSrc (elm); } - void skipSrc (Elm elm) override { triggered_ = true; PAR::skipSrc (elm); } + bool injectNew (Elm elm) override { trigger(); return PAR::injectNew (elm); } + bool findSrc (Elm elm) override { trigger(); return PAR::findSrc (elm); } + void skipSrc (Elm elm) override { trigger(); PAR::skipSrc (elm); } + bool assignElm (Elm elm) override { trigger(assign); return PAR::assignElm (elm); } }; @@ -118,6 +136,19 @@ namespace diff{ return chainedBuilder> (changeListener); } + /** Entry point for DSL builder: attach a functor as listener to be notified after either + * a structural change, or a value assignment within the local scope of this TreeMutator. + */ + template + template + inline auto + Builder::onLocalChange (LIS changeListener) + { + ASSERT_VALID_SIGNATURE (LIS, void(void)) + // vvvv---note: including assignments + return chainedBuilder> (changeListener); + } + }//(END)Mutator-Builder decorator components... }} // namespace lib::diff diff --git a/src/lib/diff/tree-mutator.hpp b/src/lib/diff/tree-mutator.hpp index 7b67dda8f..6c9817e4b 100644 --- a/src/lib/diff/tree-mutator.hpp +++ b/src/lib/diff/tree-mutator.hpp @@ -458,11 +458,19 @@ namespace diff{ * be invoked _after_ applying a diff with any `INS`, `DEL`, `FIND`, `SKIP` verb. * @remark in theory, one could also drop contents indirectly, by sending only * part of the necessary PICK verbs (or using AFTER(...)). However, - * such a diff is formally not prohibited, and will indeed be detected as + * such a diff is formally not allowed, and will indeed be detected as * error when leaving a scope, in ChildCollectionMutator::completeScope() */ template auto onSeqChange (LIS changeListener); + + /** attach a listener function, to be invoked on any _local change._ + * This includes the [structural changes](\ref onSeqChange()), but also + * value assignments to any attribute or element. + * @note mutation of a nested child scope will _not_ trigger this listener. + */ + template + auto onLocalChange (LIS changeListener); }; }//(END) Mutator-Builder... diff --git a/tests/library/diff/diff-tree-mutation-listener-test.cpp b/tests/library/diff/diff-tree-mutation-listener-test.cpp index 0fc0b349d..72815160b 100644 --- a/tests/library/diff/diff-tree-mutation-listener-test.cpp +++ b/tests/library/diff/diff-tree-mutation-listener-test.cpp @@ -87,7 +87,10 @@ namespace test{ * @test When creating a TreeMutator binding, a listener (lambda) can be attached, * to be invoked on structural changes... * - inserting, removing and reordering of children counts as "structural" change - * - whereas assignment of a new value or nested mutation does not trigger + * - whereas assignment of a new value will only trigger `onLocalChange()` + * - mutation of nested scopes does not trigger any of these listeners, + * since (within the existing framework) there is no simple way to + * intercept also the child mutation stream to check for relevance. * @note This test binds the test class itself for diff mutation, applying changes * onto a vector with test data. The binding itself is somewhat unusual, * insofar it allows to re-assign elements within the vector, which can be @@ -115,10 +118,13 @@ namespace test{ { std::vector subject_; int structChanges_ = 0; + int localChanges_ = 0; /** rig the test class itself to receive a diff mutation. * - bind the #subject_ data collection to be changed by diff - * - attach a listener, to be invoked on _structural changes + * - attach a listener, to be invoked on _structural changes_ + * - attach another listener, activated both on structural + * changes and on _value assignment._ */ void buildMutator (TreeMutator::Handle buff) override @@ -137,7 +143,11 @@ namespace test{ })) .onSeqChange ([&]() { - ++structChanges_; // Note: this lambda is the key point for this test + ++structChanges_; // Note: these lambdas are the key point for this test + }) + .onLocalChange ([&]() + { + ++localChanges_; }) ); } @@ -148,6 +158,7 @@ namespace test{ { CHECK (isnil (subject_)); CHECK (0 == structChanges_); + CHECK (0 == localChanges_); DiffApplicator applicator{*this}; @@ -158,6 +169,7 @@ namespace test{ }}); CHECK ("a, c, d, c" == contents(subject_)); CHECK (1 == structChanges_); + CHECK (1 == localChanges_); applicator.consume (MutationMessage{{after(Ref::END) , set (VAL_C_UPPER) // Note: the current element is tried first, which happens to match @@ -165,6 +177,7 @@ namespace test{ }}); CHECK ("a, c, D, C" == contents(subject_)); CHECK (1 == structChanges_); // Note: the listener has not fired, since this counts as value change. + CHECK (2 == localChanges_); applicator.consume (MutationMessage{{pick(VAL_A) , ins (VAL_B) @@ -174,12 +187,14 @@ namespace test{ , del (VAL_C) }}); CHECK ("a, b, D, c" == contents(subject_)); - CHECK (2 == structChanges_); // Note: this obviously is a structure change, so the listener fired. + CHECK (2 == structChanges_); // Note: this obviously is a structure change, so both listeners fired. + CHECK (3 == localChanges_); applicator.consume (MutationMessage{{after(Ref::END) }}); CHECK ("a, b, D, c" == contents(subject_)); - CHECK (2 == structChanges_); // Note: contents confirmed as-is, listener not invoked. + CHECK (2 == structChanges_); // Note: contents confirmed as-is, none of the listeners is invoked. + CHECK (3 == localChanges_); } }; diff --git a/wiki/thinkPad.ichthyo.mm b/wiki/thinkPad.ichthyo.mm index d78de799c..0654062e3 100644 --- a/wiki/thinkPad.ichthyo.mm +++ b/wiki/thinkPad.ichthyo.mm @@ -27794,8 +27794,7 @@ kann erst erfolgen, wenn's soweit ist

- - + @@ -27805,13 +27804,38 @@ ...und muß damit in den State-Change-Mechanismus für den Präsentationsstil verlegt werden

- - + + + + + + + + + + + + + + + + + + + +

+ Nebenbei bemerkt: die Aktion muß idempotent sein +

+ + + + + @@ -27855,8 +27879,7 @@ wir verwenden das ClipDelegate aber auch, um in Clips eingebettete Effekte oder die einzelnen Spuren darzustellen. Die Logik für den Anzeigestil muß das mit berücksichtigen

- - + @@ -27902,8 +27925,7 @@ Letzteres gefällt mir definitiv besser

- - + @@ -27989,8 +28011,7 @@ ...diese erstreckt sich typischerweise über die gesamte Länge des umschließenden Containers, und paßt sich dieser ohne weiteres dynamisch an

- - + @@ -37943,6 +37964,12 @@ + + + + + +