Idea is to use the window list, which should hold any workspace window
ever created, and pick the first one marked as 'active' by GTK
(whatever that means)
After quite some pondering, it occured to me that we both
- need some top-level model::Tangible to correspond to the RootMO in the session
- need some Controller to handle globally relevant actions
- need a way to link action invocation to transient interaction state (like focus)
This leads to the introduction of a new top-level controller, which is better
suited to fill that role than the depreacted model-controller or the demoted window-manager
looks like we're in management business here ;-)
we chop off heads, slaughter the holy cows and then install -- a new manager
...allows us to get rid of a lot of sigc boilerplate syntax.
The downside is that the resulting functors are not sigc::trackable.
This seems adequate here, since the whole top-level UI backbone is
maintained by GtkLumiera, and thus ensured to exist as long as the
main GTK event loop is running.
WARNING: beware of creating "wild" background thrads in the UI, without
proper scheduling of any communication via the event loop!
no need for a further translation unit,
rather, definition of global menu now becomes part of the
ui-manager.cpp translation unit, which allows for some additional
inlining and simplifications by the compiler
it turns out to be essentially an implementation detail,
it is a builder class and it acts as closure for the bound
menu actions, but it is not accessed after initialisation.
This allows to reduce the header inclusion load significantly
This is a very pervasive change and basically turns the whole top-level
of the GTK-UI bottom-up. If this change turns out right, it would likely
solve #1048
WARNING: in parts not implemented, breaks UI
...which itself is obsolete and needs to be redesigned from scratch.
For now we create a local instance of this obsolete PlaybackController
in each viewer panel and we use a static accessor function to just some
instance. Which would break if we start playback with multiple viewer
panels. But we can't anyway, since the Player itself is also a broken
leftover from an obsoleted design study from the early days.
so why care...
unfortunately boost/program-options make the boost reference-wrapper visible
And it doesn't help to alias to std::ref at the definition site of the
problematic function (in TimeControl), because this itself is picked up
via ADL
So this is not really a solution, rather a workaround, in the hope
that boost will clean-up this ambiguity eventually
as a rule, one should not rely on "using namespace xyz",
since this makes organisation of minimal header includes near impossible.
You end up with mass includes in some "top level" headers, resulting
in painfully slow compilation turnaround times.
In exceptional cases, using namespace foo might be adequate though
- WindowList (ex WindowManager)
- Project & Controller
the latter ones are defunct and can be replicated down into each
of the old timeline pannel instances. They just serve the purpose
to keep this old code barely functional, so it can be used as reference
for building the new timeline
There seems to be a mismatch in the arrangement of the top-level entities
* we support multiple windows, yet from reading the code, you'd ge the impression we aren't really aware we have multiple top-level windows
* the `WindowManager` is the core UI manager, which feels like a mix-up in concerns
* the `WorkspaceWindow::createUI()` does the global UI initialisation. Again, we have multiple workspace windows.
* `GtkLumiera::main()` creates a `Model` and a `Controller` in local function scope, but stores the `WindowManager` in an object field.
* it seems, for that very reason, `GtlLumiera` needed to be a singleton, to allow by-name access to "the" `WindowManager`
* needless to say, this causes a host of problems when shutting down the UI.
The idea is to introduce a dedicated UiManager, to deal with the central
framework induced concerns solely, and to demote the WindowManager and the
WorkspaceWindows to care only for their local concerns
in fact it just does not fulfil any of the behavioural properties
of a full-fledged UI-Element. All it needs is an uplink bus connection,
so let's just keep it as that
Sidenote: I've realised today that such a "free standing" BusTerm
without registration in Nexus is a good idea and acceptable solution.
yes, it's a cycle and indeed quite tricky.
Just verified it (again) with the debugger and saw all
dtor calls happening in the expected order. Also the number
of Nexus registration is sane
Now I've realised that there are two degrees of connectedness.
It is very much possible to have a "free standing" BusTerm, which
only allows to send uplink messages. In fact, this is how CoreService
is implemented, and probably it should also the way how to connect
the GuiNotification service...
due to investigating that Heisenbug, I understand the storage layout
more clearly. It occured to me that there is no reason to copy the
terminationHandler (functor) into an instance variable, since it is
easily possible to keep all of the invocation and error handling
confined within the scope of the run function, i.e. on stack.
So the effective memory layout does not change, but the legibility
of the code is improved, since we're able to remove the dtor and
simplyfy the ctor and avoid most of the member fields.
Reason was some insideous detail regarding Lambdas:
When a Lambda captures context, a *closure* is created.
And while the Lambda itself is generated code, pretty much
like an anonymous function, the closure depends on the context
that was captured. In our case here, the Lambda used to start
the thread was the problem: it captured the termCallback functor
from the argument of the enclosing function. In fact it did not
help or change anything if we successively package that lambda
into a function objet and store this by value, because the
lambda still refers to the transient function context present
on stack at the moment it was captured.
The solution is to revert back to a bind expression, since this
creates a dedicated storage for the bound function arguments
managed within the bind-functor. This makes us independent
from the call context
...because some Bus connections stem from elements which are
member of CoreService, thus the'll still be connected when the
sanity check in the dtor runs
But even with this fix, we still get a SEGFAULT
TODO
- is this actually a sensible idea, from a design viewpoint?
- in which way to bind GuiNotification for receiving diff messages?
- Problem with disconnnecting from Nexus on shutdown
Writing and debugging such tests is always an interesting challenge...
Fortunately this exercise didn't unveil any problem in the newly written
code, only some insidious problems in the test fixture itself. Which
again highlights the necessity, that each *command instance* needs
to be an independent clone from the original *command prototype*,
since argument binding messages and trigger messages can appear
in arbitrary order.
This is a little bit of functionality needed again and again;
first I thought to use the TypedCounter, but this would be overkill,
since we do not actually need different instances, and we do not need
to select by type when incrementing the counter. In fact, we do not
even need anything beyond just allocating a number.
So I made a new class, which can be used RAII style
the intention is to cover more of the full invocation path,
without running all of the application infrastructure. So this
second test cases simulates how messages are handled in CoreService,
where the CommandHandler (visitor) actually invokes the SessionCommand
facade
this was a spin-off activity from writing the SessionCommand
function(integration) test, where I noted that we can't just
capture "a time value" as command memento
basically this is not necessary, since the compiler figures out
to use the conversion to target type when attempting to resolve
an equality comparison. But it helps to avoid ambiguities in cases
where several conversion paths do exist, e.g. when comparing string
with C-string
explicitly observed with the debugger that the call path is sane;
the code looks innocuous, but it is quite magic how the compiler
picks precisely the right ctors and inserts conversions apropriately
command processing against the session is not yet implemented,
so to allow for unit testing, we magically recognise all commands
starting with "test." and invoke them directly within the Dispatcher.
With this addition, the basic functionality of the dispatcher works now
From a purely logical viewpoint, it looked sensible to require an actual
value for an offset, especially since our time values are immutable.
But this has the unfortunate consequence that we'd be unable to use
an offset value as parameter for any command, since we store the arguments
as tuple and the tuple type has a default constructor. We might be able
to get around that problem, but such looks brittle to me; it is just
plain surprising for anyone not familiar with the internals of the
command system.
For that reason, I've now added a default ctor to the Offset type
not quite sure how to get the design straight.
Also a bit concerned because we'll get this much indirections;
the approach to send invocations via the UI-Bus needs to prove its viability
...since the session loop will be notified on any change via the
interface, adding a command will activate the loop, and the builder
timeout is handled separately via the dirty state. So there is no
need to spin around the loop in idle state.
As a aside, timeout waiting on a condition variable can be intentional
and should thus not be logged as an error automatically. It is up to the
calling context to decide if a timeout constitutes an exceptional situation.
It is always a trade-off performance vs. readability.
Sometimes a single-threaded implementation of self-contained logic
is preferable to a slightly more performant yet obscure implementation
based on our threadpool and scheduler.
Did a full review of state and locking logic, seems airtight now.
- command processing itself is unimplemented, we log a TODO message for now
- likewise, builder is not implemented
- need to add the deadlock safeguard #1054
Due to object scoping we can conclude reliably that the only one
ever to delete the DispacherLoop object will be the the loop thread
from within this object itself, when invoking the termination callback.
Btw, the lock on the inner object was insufficient and will be
replaced by taking the outer lock
We found out that it's best to run it single threaded
within the session loop thread. This does not mean the Builder
itself is necessarily single threaded, but the Builder's top level
will block any other session operation, and this is a good thing.
For this reason it makes more sense to have the Builder integrated
as a component into the session subsystem.
It turns out we *do* support the use of anonymous commands
(while it is not clear yet if we really need this feature).
Basically, client code may either create and register a new
instance from another command used as prototype, by invoking
Command::storeDef(ID). Or, alternatively it may just invoke
newInstance() on the command, which creates a new handle
and a valid new implementation (managed by the handle as
smart-ptr), but never stores this implementation into the
CommandRegistry. In that case, client code may use such a
command just fine, as long as it cares to hold onto that
handle; but it is not possible to retrieve this command
instance later by symbolic ID.
In the light of this (possible) usage pattern, it doesn't
make sense to throw when accessing a command-ID. Rather, we
now return a placeholder-Symbol ("_anonymous_")
And yes, this warning is for real, while the compiler has no way
to decide if there is actual danger lurking. A type with internal
linkage (e.g. defined in an anonymous namespace) will be treated
by the linker as a separate entity on each encounter (i.e. in
each distinct compilation unit). When multiple translation units
start collaborating on such a type, they *might* be referring
to different memory locations, while semantically the intention
is to refer to the same location.
And since we're dealing with a library facility here, *we* have
likewise now power to ensure proper usage, so we better be cautious.
after reading some related code, I am leaning towards a design
to mirror the way command messages are sent over the UI-Bus.
Unfortunately this pretty much abandons the possibility to
invoke these operations from a client written in C or any
other hand made language binding. Which pretty much confirms
my initial reservation towards such an excessively open
and generic interface system.
...this means to turn Looper into a state machine.
Yet it seems more feasible, since the DispatcherLoop has a nice
checkpoint after each iteration through the while loop, and we'd
keep that whole builder-dirty business completely confined within
the Looper (with a little help of the DispatcherLoop)
Let's see if the state transition logic can actually be implemented
based just on such a checkpoint....?
....if by some weird coincidence, a command dispatched into the session
happens to trigger session shutdown or re-loading, this will cause a deadlock,
since decommissioning of session data structures must wait for the
ProcDispatcher to disable command processing -- and this will obviously
never happen when in a callstack below some command execution!
After some consideration, it became clear that this service implementation
is closely tied to the DispatcherLoop -- which will consequently be
responsible to run and expose this service implementation
need to keep state variables on both levels,
since the session manager (lifecycle) "opens" the session
for external access by starting the dispatcher; it may well happen
thus that the session starts up, while the *session subsystem*
is not(yet) started
"command dispatching" == the public session interface
so we'll better implement this important causal link directly,
instead of some obscure trickery with lifecycle events.
turns out that I've created a race and consistency problem
just by a silly idiotic fixation on performance. Never ever
leave out a lock to "improve" performance, mind me.
mark TODOs in code to make that happen.
Actually, it is not hard to do so, it just requires to combine
all the existing building blocks. When this is done, we can define
the "Session" subsystem as prerequisite for "GUI" in main.cpp
Unless I've made some (copy-n-paste) mistake with defining the facades,
this should be sufficient to pull up "the Session" and automatically
let the Gui-Plugin connect against the SessionCommandService
...the sheer amount of mechanical replacements scattered all over these
files might be a vivid indication, that the design of the interface system
is subobptimal ;-)
up to now this happened from the GuiRunner, which was a rather bad idea
- it can throw and thus interfer with the startup process
- the GuiNotification can not sensibly be *implemented* just backed
by the GuiRunner. While CoreService offers access to the necessary
implementation facilities to do so
so the true reason is an inner contradiction in the design
- I want it to be completely self similar
- but the connection to CoreService does not conform
- and I do not want to hard code CoreService into the Nexus classdefinition
So we treat CoreService as uplink für Nexus and Nexus as uplink for CoreService,
with the obvious consequences that we're f**ed at init and shutdown.
And since I want to retain the overall design, I resort to implement
a short circuit detector, which suppresses circular deregistration calls
Decision was made to use the CoreService as PImpl to organise
all those technical aspects of running the backbone. Thus,
the Nexus (UI-Bus hub) becomes part of CoreService
...problem is, I actually don't know much about what kinds of markers
we'll get, and how we handle them. Thus introducing a marker kind
is just a wild guess, in order to get *any* tangible attribute
Phew, convoluted.
And I was doubtful that we need to support multiple typed child collection
Well, we get three such collections already in the first real world example...
...it occurred to me that very likely a casual reader of the code
will encounter here the first instance of such a diff binding function.
I am well aware this looks intimidating (and it is a tricky technical detail)
Even more so, if what you expect is just some access to a shared data model,
you might be completely puzzled by this code and nor recognise its importance.
this is a tricky problem and a tough decision.
After quite some pondering I choose to enforce mandatory fields
through the ctor, and not to allow myself cheating my way around it
it occurred to me that effectively we abandoned the use of
a business facade and proxy model in the UI. The connection
becomes entirely message based now.
To put that into context, the originally intended architecture
never came to life. The UI development stalled before this could
happen; possibly it was also hampered by the "impedance mismatch"
between our intentions in the core and such a classical, model centric
architecture. Joel several times complained that he felt blocked; but
I did not really understand this issue. Only recently, when I came to
adapting the timeline display to GTK-3, I realised the model centric
approach can not possibly work with such an open model as intended
in our case. It would lead to endless cascades of introspection.
...shows again why its not adwisable to use wildcard namespace include.
Well, the old timeline code is going away soon, and for the rewritten new one,
we'll learn from such structural problems
these are just empty class files, but writing a basic description
for each made me flesh out a lot of organisational aspects of what
I am about to build now
reason is, only files with a @file comment will be processed
with further documentation commands. For this reason, our Doxygen
documentation is lacking a lot of entries.
HOWTO:
find src -type f \( -name '*.cpp' -or -name '*.hpp' \) -not -exec egrep -q '\*.+@file' {} \; -print -exec sed -i -r -e'\_\*/_,$ { 1,+0 a\
\
\
/** @file §§§\
** TODO §§§\
*/
}' {} \;
bottom line
- seems we need to do that manually
- must wait until in the on_draw() callback
- use Container::foreach() to visit all child widgets
- Layout::set_size()
I am still suspicious the cleanup mechanism for child widgets works as expected...
But right now, we can't verify that, since on shutdown we get an assertion failure
from ld.so "dl-close.c: 762: _dl_close: Assertion `map->l_init_called' failed!"
Seems we're loading the GUI plugin not properly
- define tasks to be addressed during investigation
- read documentation, identify problematic aspects
- prepare a child widget class to be placed on the canvas
My intention is to use this space for experiments first,
and then as a construction site for a rewrite of the
custom timeline widget.
We really need a rewrite here, in order to be properly
aligned to the standard way of writing such a custom widget,
and also to build our first connection to the UI-Bus and then
remove the old placeholder UI model
Damn sideeffect of the suppport for move-only types: since we're
moving our binding now into place /after/ construction, in some cases
the end() iterator (embedded in RangeIter) becomes invalid. Indeed this
was always broken, but didn't hurt, as long as we only used vectors.
Solution: use a dedicated init() hook, which needs to be invoked
*after* the TreeMutator has been constructed and moved into the final
location in the stack buffer.
unintentionally we used copy construction in the builder expression,
wenn passing in the CollectionBinding to the ChildCollectionMutator.
The problem is that CollectionBinding owns a shaddow buffer, where
the contents of the target collection are moved temporarily while
applying the diff. The standard implementation of copy construction
would cause a copy of that shaddow buffer, which boils down to
a copy of the storage of the target collection.
If we want to support move-only types in the collection, most notably
std::unique_ptr, we can thus only use the move constructor. Beyond that
there is no problem, since we're only ever moving elements, and new
elements will be move constructed via emplace() or emplace_back()
actually this is a pragmatic extension for some special use cases,
and in general rather discurraged, since it contradicts the
established diff semantics. Yet with some precaution, it should
be possible to transport information via an intermediary ETD
Map -> ETD -> Map
for the record: while it is indeed sweet-and-simple to support Ref::THIS
here, it is near impossible to represent it in general, in a setup with
multiple "onion-layers". The reason is, we'd have to incorporate such
special treatment into the /selector predicate/, which in turn undermines
the ability to pick the right onion layer to handle a given diff verb,
since "Ref::THIS" is a generic marker and we have no other data to base
the decision in the selector on.
...this is the first attempt to integrate the Diff-Framework into (mock) UI code.
Right now there is a conceptual problem with the representation of attributes;
I tend to reject idea of binding to an "attribute map"
the generic typing to DiffMutatble does not make much sense,
since the desired implementation within gui::ctrl::Nexus
is bound to work on Tangibles only, since that is what
the UI-Bus stores in the routing table
Up to now, InPlaceBuffer used to default construct an instance of the
Interface class, and then you'd need to invoke the `create()` function
to actually create the desired subclass. This is not only inefficient,
but rules out the use of abstract interfaces / base classes.
Unfortunately, there is no way in C++ to specify an explicit template argument list
on ctor calls, so we resort to the trick of passing an additional dummy marker argument
yay! this piece of code has served its purpose:
it was the blueprint to build a way better design and implementation,
which can now cover this "generic tree" case as a special case as well
this adds kind of an extension point to diff::Record<GenNode>::Mutator,
which is then actually defined (implemented) within the diff framework.
This allows the TreeDiffTraits automatically to use this function
to get a TreeMutator for a given Rec::Mutator. Which in turn allows
the generic version of DiffApplicator automatically to attach and
bind to a Record<GenNode>
together this allows us to ditch the explicit specialisation
and dedicated, hand-written implementation of DiffApplication
to GenNode in favour of using the TreeMutator and friends.
this is a subtle change in the semantics of the diff language,
actually IMHO a change towards the better. It was prompted by the
desire to integrate diff application onto GenNode-trees into the
implementation framework based on TreeMutator, and do away with
the dedicated implementation.
Now it is a matter of the *selector* to decide if a given layer
is responsible for "attributes". If so, then *all* elements within
this layer count as "attribute" and an after(Ref::ATTRIBS) verb
will fast forward behind *the end of this layer*
Note that the meta token Ref::ATTRIBS is a named GenNode,
and thus trivially responds to isNamed() == true
needed to use a forward function declaration within the
lambda for recursive scope mutator building, since otherwise
everything is inline and thus the compilation fails when it
comes to deducing the auto return type of the builder.
Other than that, the whole mechanics seem to work out of the box!
previously they where included in the middle of tree-mutator.hpp
This was straight forward, since the builder relies on the classes
defined in the detail headers.
However, the GenNode-binding needs to use a specifically configured
collection binding, and this in turn requires writing a recursive
lambda to deal with nested scopes. This gets us into trouble with
circular definition dependencies.
As a workaround we now only *declare* the DSL builder functions
in the tree-mutator-builder object, and additionally use auto on
all return types. This allows us to spell out the complete builder
definition, without mentioning any of the implementation classes.
Obviously, the detail headers have then to be included *after*
the builder definition, at bottom of tree-mutator.hpp
This also allows us to turn these implementation headers into
completely normal headers, with namespaces and transitive #includes
In the end, the whole setup looks much more "innocent" now.
But beware: the #include of the implementation headers at bottom
of tree-mutator.hpp needs to be given in reverse dependency order,
due to the circular inclusion (back to tree-mutator.hpp) in
conjunction with the inclusion guards!
...instead of using a hand written implementation,
the idea is to rely on the now implemented building blocks,
with just some custom closures to make it work.
- esp. verify the proper inclusion of the Selector closure in all Operations
- straighten the implementation of Attribute binding
- clean-up the error checking helpers
similar reordering for the third part.
This time most operations are either passed down anyway,
or are NOP, since attribute binding has no notion of 'order'
yay! unit testing rocks.
Actually I changed the test definition for another reason, just to discover
that I've missed to implement that operation in this onion layer
now failing due to a contradiction in test fixture:
it is nonsensical to re-order attributes; rather, we should
cover re-ordering of children, to verify that the mutator binding
properly surpasses the attribute layers and forwards operations
to the lower layers responsible for handling child scopes...
In Theory, acceptSrc and skipSrc are to operate symmetrically,
with the sole difference that skipSrc does not move anything
into the new content.
BUT, since skipSrc is also used to implement the `skip` verb,
which serves to discard garbage left back by a preceeding `find`,
we cannot touch the data found in the src position without risk
of SEGFAULT. For this reason, there is a dedicated matchSrc operation,
which shall be used to generate the verification step to properly
implement the `del` verb.
I've spent quite some time to verify the logic of predicate evaluation.
It seems to be OK: whenever the SELECTOR applies, then we'll perform
the local match, and then also we'll perform the skipSrc. Otherwise,
we'll delegate both operations likewise to the next lower layer,
without touching anything here.
--> now it becomes obvious that we've mostly
missed to integrate the Selector predicate properly
in most bindings defined thus far. Which now causes
the sub-object binding to kick in, while actually
the sub-value collection should have handled
the nested values CHILD_B and CHILD_T
OMG, this is intricate stuff....
Questionable if anyone (other than myself) will be able
to get those bindings right???
Probably we'll need yet another abstraction layer to handle
the most common binding situations automatically, so that people
can use the diff framework without intricate knowledge of
TreeMutator construction.
- an extension to our custom toString and typeString helpers.
- currently just for shared_ptr and unique_ptr
- might add further overloads for other smart-ptr types
integrated into the generic DiffApplicationStrategy.
The dedicated, explicit specialisation for DiffMutable is
no longer needed, since the generic template will degrade or
fall back to precisely this functionality, when the target
implements the DiffMutable interface
This is the first skeleton to combine all the building blocks,
and it passes compilation, while of course most of the binding
implementation still needs to be filled in...
It occurred to me, that 90% of this template specialisation
are entirely generic and not dependant on the actual target type.
While the compiler/linker is able to sort such a situation out,
this might lead to template bloat and possibly subtle errors.
So it seems more adequate to emit the generic part of the code
right away from within a dedicated translation unit within the
library module; so the vtable is already in place and only
the flexible part of the code needs to be re-emitted on
each usage site.
- default recommendation is to implement DiffMutable interface
- ability to pick up similar non-virtual method on target
- for anything else client shall provide free function mutatorBinding(subject)
PERSONAL NOTE: this is the first commit after an extended leave,
where I was in hospital to get an abdominal cancer removed.
Right now it looks like surgery was successful.
this is at the core of the integration problem: how do we expose
the ability of some opaque data structure to create a TreeMutator?
The idea is
- to use a marker/capability interface
- to use template specialisation to fabricate an instance of that interface
based on the given access point to the opaque data structure
but unfortunately this runs straight into a tough problem,
which I tried to avoid and circumvent all the time:
At some point, we're bound to reveal the concrete type
of the Mutator -- at least to such an extent that we're
able to determine the size of an allocator buffer.
Moreover, by the design chosen thus far, the active
TreeMutator instance (subclass) is assumed to live within
the top-level of a Stack, which means that we need to
place-construct it into that location. Thus, either
we know the type, or we need to move it into place.
initially, even the diff applicator was meant to be a
"throwaway" object. But then, on writing some tests,
it seemed natural to allow re-using a single applicator,
after having attached it to some target.
With that change, I failed to care for the garbage
left back in the "old" sequence after applying one diff;
since in the typical usage sequence, the first use builds
content from scratch, this problem starts to show up only
with the third usage, where the garbage left from the input
of the second usage appears at the begin of the "new sequence"
Solution is to throw away that garbage explicitly on re-entrance
..because actually we don't know if the intention is
to drop those waste elements -- and for sure this
discarding of waste does not happen through the
invocation logged here; rather it happens by
abandoning the scope
...which mostly just is either ignoring the
operations or indicating failure on attempt to
'reorder' attributes (which don't have any notion of 'ordering')
overall, the structure of this implementation is still rather confusing,
yet any alternatives seem even less convincing
- if we want to avoid the delegation to base-class, we'd have
to duplicate several functions and the combined class would
handle two distinct concerns.
- any attempt to handle the IDs more "symmetrically" seems to
create additional problems on one side or the other
this also supersedes and removes the initial implementation
draft for attribute binding with the 'setAttribute' API
The elementary part of diff application incl. setting
new attribute values works by now.
While in general it is fine to clean-up any entity IDs
to be US-ASCII alphanumerics (plus some allowed interpunction),
the GenNodes and also keys in object-bindings for diff are
considerd internal interfaces, assuming that any passed
ID symbol is already sanitised and checked. So the
sanitise operation can be skipped. This changeset
adds the same option directly to lib::EntryID,
allowing to create an EntryID that matches
a similar GenNode's (hash) ID.
The way we build this attribute binding, there is no single
entity to handle all attribute bindings. Thus the only way
to detect a missing binding is when none of the binding layers
was able to handle a given INS verb
to summarise, it turned out that it is impossible to
provide an airtight 'emptySrc' implementation when binding
to object fields -- so we distinguish into positive and
negative tests, allowing to loosen the sanity check
only for the latter ones when binding to object fields.
..as concluded from the preceding analysis.
NOTE this entails a semantical change, since this
predicate is now only meant to be indicative, not conclusive
remarks: the actual implementation of the diff application process
as bound via the TreeMutator remains yet to be written...
how can ordinary object fields be treated as "Attributes"
and thus tied into the Diff framework defined thus far.
This turns out to be really tricky, even questionable
while simple to add into the implementation, this whole feature
seems rather qestionable to me now, thus I've added a Ticket
to be revisited later.
In a nutshell, right here, when implementing the binding layer
for STL collections, it is easy to enable the framework to treat
Ref::THIS properly, but the *actual implementation* will necessarily
be offloaded onto each and every concrete binding implementation.
Thus client code would have to add support for an rather obscure
shortcut within the Diff language. The only way to avoid this
would be to change the semantics of the "match"-lambda: if this
binding would rather be a back-translation of implementation data
into GenNode::ID values, then we'd be able to implement Ref::THIS
natively. But such an approach looks like a way inferiour deisgn
to me; having delegated the meaning of a "match" to the client
seems like an asset, since it is both natural and opens a lot
of flexibility, without adding complexity.
For that reason I tend to avoid that shortcut now, in the hope
to be able to drop it entirely from the language
...basically this worked right away and was easy to put together.
However, when considering how many components, indirections and
nested lambdas are working together here, I feel a bit dizzy...
:-/
write down a first draft for a definiton section,
to describe the fundamental parts involved, when
applying a diff message onto implementation defined
data structures
After a break of tree weeks, I found it difficult to find may way
amidst all those various levels of abstraction. In addition to this
definition, we'll probably also need a high level overview of the
whole diff system operation.
...all of this implementation boils down to slightly adjusting
the code written for the test-mutation-target. Insofar it pays off now
having implemented this diagnostic and demonstration first.
Moreover I'm implementing this basic scheme of "diff application"
roughly the fourth time, thus things kindof fall into place now.
What's really hard is all those layers of abstraction in between.
Lesson learned (after being off for three weeks, due to LAC and
other obligations): I really need to document the meaning of the
closures, and I need to document the "abstract operational semantics"
of diff application, otherwise no one will be able to provide
the correct closures.
while I still keep my stance not to allow reflection and
switch-on-type, access to the internal / semantic type of
an embedded record seems a valid compromise to allow
to deal with collections of object-like children
of mixed kind.
Indirectly (and quite intentional) this also opens a loophole
to detect if a given GenNode might constitute a nested scope,
but with the for the actual nested element indeed to cary
a type symbol. Effectively this limits the use of this shortcut
to situations where the handling context does have some pre-established
knowledge about what types *might* be expected. This is precisely
the kind of constraint I intend to uphold: I do not want the
false notion of "total flexibility", as is conveyed by introspection.
since we're moving elements around to apply the diff,
dangerous situation might arise in case anyone takes a copy
of the mutator. Thus we effectively limit the possible
usage pattern and only allow to build an anonymous
TreeMutator subclass through the Builder-DSL.
The concrete "onion layers" of the TreeMutator are now limited
- to be created by the chaining operations of the Builder DSl
- to be moved into target location, retaining ownership.
I still feel somewhat queasy with this whole situation!
We need to return the product of the DSL/Builder by value,
but we also want to swap away the current contents before
starting the mutation, and we do not want a stateful lifecycle
for the mutator implementation. Which means, we need to swap
right at construction, and then we copy -- TADAAA!
Thus I'm going for the solution to disallow copying of the
mutator, yet to allow moving, and to change the builder
to move its product into place. Probably should even push
this policy up into the base class (TreeMutator) to set
everyone straight.
Looks like this didn't show up with the test dummy implementation
just because in this case the src buffer also lived within th
TestMutationTarget, which is assumed to sit where it is, so
effectively we moved around only pointers.
the whole implementation will very much be based on
my experiences with the TestMutationTarget and TestWireTap.
Insofar it was a good idea to implement this test dummy first,
as a prototype. Basically what emerges here is a standard pattern
how to implement a tree mutator:
- the TreeMutator will be a one-way-off "throwaway" object.
- its lifecylce starts with sucking away the previous contents
- consuming the diff moves contents back in place
- thus the mutator always attaches onto a target by reference
and needs the ability to manipulate the target
the collection binding can be configured with various
lambdas to supply the basic building blocks of the generated binding.
Since we allow picking up basically anything (functors,
function pointers, function objects, lamdas), and since
we speculate on inlining optimisation of lambdas, we can not
enforce a specific signature in the builder functions.
But at least we can static_assert on the effective signature
at the point where we're generating the actual binding configuration
we can't generate a static assertion so easily here.
Problem is, when forming this type, we don't know if
the user will override and provide a custom binding
in some chained call within the nested DSL.
Might still be able to come up with some clever trick,
like e.g. returing an unsuitable marker type from these
dummy default implementations and then, later on, when
actually building the collection binding, to detect
those marker types and rise a static assert at that point.
This would at least give us a better error message,
and in theory, it should always be possible to
detect this kind of misuse at compile time
...through the use of partial specialisation and SFINAE.
There are some rather specific (yet expectedly not uncommon) cases,
where we'd be able to provide a sensible default for the
- match predicate
- new element constructor
of the binding. While in all other cases, the user
has to provide an explicit implementation for these
crucial building blocks anyway.
the reason is also to enable usage as metafunction,
to disable specialisations for some type which could
never live within a variant record in question
...but does not compile, since all of the fallback functions
will be instantiated, even while in fact we're overriding them
right away with something that *can* be compiled.
this prompts me to reconsider and question the basic approach
with closures for binding, while in fact what I am doing here
is to implement an ABC.
- the test will use some really private data types,
valid only within the scope of the test function.
- invoking the builder for real got me into problems
with the aggregate initialisation I'd used.
Maybe it's the function pointers? Anyway, working
around that by definint a telescope ctor
when setting up a binding to child elements within a STL collection,
all the variable elements are preconfigured to a more or less
disabled and inactive state.
the concern is for the structure of the builder to be
incomprehensible and completely buried within the
implementation details of the various binding layers
most of the mutation primitives return bool(true)
when /any/ layer or part of the TreeMuator was able
to cope with the diff verb.
This is based on the assumption to configure the
TreeMutator in such a way that at most one facility
will actually handle and apply a given verb. That is,
we'll assume that the TreeMutator acutally wraps and
adapts *one* custom data structure, to which the
diff has to be applied.
The TestWireTap is special, insofar it indeed targets
a *second* data structure, albeit not a "real" one,
just a dest and diagnostics dummy.
the first part of the unit test (now passing)
is able to demonstrate the full set of diff operations
just by binding to a TestMutationTarget.
Now, after verifying the design of those primmitive operations,
we can now proceed with bindings to "real" data structures
when implementing the assignment and mutation primitives
it became clear that the original approach of just storing
a log or string rendered elements does not work: for
assignment, we need to locate an element by ID
this one went through unnoticed, because the situation
is not covered in unit-test. The tests written thus fare
are more like a proof-of-concept. I didn't want to spend
weeks on writing extensive coverage of all corner cases,
at least not before all aspects of the tree diff protocol
are settled. Seemingly this backfires already
now the full API for the "mutation primitives" is shaped.
Of course the actual implementation is missing, but that
should be low hanging fuit by now.
What still requires some thinking though is how to implement
the selector, so we'll actually get a onion shaped decorator
basically we'll establish a collaboration where both sides
know only the interface (contract) of the partner; a safe margin
for allocation size has to be established through metaprogramming (TODO)
what's problematic is that we leave back waste in the
internal buffer holding the source. Thus it doesn't make
sense to check if this buffer is empty. Rather the
Mutator must offer an predicate emptySrc().
This will be relevant for other implementations as well
while the original name, 'replace', conveys the intention,
this more standard name 'swap' reveals what is done
and thus opens a wider array of possible usage
now this feels like making progress again,
even when just writing stubs ;-)
Moreover, it became clear that the "typing" of typed child collections
will always be ad hoc, and thus needs to be ensured on a case by case
base. As a consequence, all mutation primitives must carry the
necessary information for the internal selector to decide if this
primitive is applicable to a given decorator layer. Because
otherwise it is not possible to uphold the concept of a single,
abstracted "source position", where in fact each typed sub-collection
of children (and thus each "onion layer" in the decorator chain)
maintains its own private position
after sleeping one night over the problem, this seems to be
the most natural solution, since the possibility of assignment
naturally arises from the fact that, for tree diff, we have
to distinguish between the *identity* of an element node and
its payload (which could be recursive). Thus, IFF the payoad
is an assignable value, why not allow to assign it. Doing so
elegnatly solves the problem with assignment of attributes
Signed-off-by: Ichthyostega <prg@ichthyostega.de>
This basically finishes definition of the fundamental
UI-Element and Bus protocol -- with one notable exception:
how to mutate elements by diff.
This will be the next topic to address
- suppres sending redundant stat mark messages from MockElm
- emit a "reset" state mark when an actual reset happens
- let the PresentationStateManager discard recorded special state
when receiving a "reset" mark for a given element
I assumed that, since GenNode is composed of copyable and
assignable types, the standard implementation will do.
But I overlooked the run time type check on the opaque
payload type within lib::Variant. When a type mismatch
is detected, the default implementation has already
assigned and thus altered the IDs.
So we need to roll our own implementation, and to add
insult to injury, we can't use the copy-and-swap idiom either.
This is actually a STL library feature, and was added precisely
for the reason encountered here: if we want logarithmic search,
we'll have to construct a new GenNode object, just to have something
for the set to invoke the comparison operator.
C++14 introduced the convention that the Comparator of the set
may define a marker type `is_transparent` alongside with a generic
comparison operator. But, as is obvious from the source code of
our GNU Standard library implementation, our std::set has no such
overload to make use of that feature
http://en.cppreference.com/w/cpp/container/set/findhttp://stackoverflow.com/questions/20317413/what-are-transparent-comparators
The only good thing is that, just 10 minutes ago, I felt like
a complete moron because I'm writing a unit test for such a simple
storage class. ;-)
...and I made the decision *not* to consider any kind of
generic properties for now. YAGNI.
UI coding is notorious spaghetti code.
No point in fighting that, it is just the way it is,
because somewhere you're bound to get concrete, hands-on.
right now, what we actually need here is just some integer,
so the GenNode payload is typed to int (or just to anything
different than a Record, because the Record signals that
we intend to bind, not to invoke the command)
the values.child() call would also do a bounds check,
but only to rise a error::Invalid "index out of bounds".
So now we generate a clear message to indicate that
actually a runtime-checked type mismatch caused this problem
the functionality as such is already covered,
but it seems important enough to warrant a dedicated test.
incidentally, Duration still lacked a default ctor.
Time values are default constructible, yet immutable.
incidentally, this uncovered yet another unwanted narrowing conversion,
namely from double via gavl_time_t to TimeValue or alternatively
from double via FSecs (= rational<long>) to Duration.
As in all the previos cases, actually the compiler is to blame,
and GCC-5 is known to get that one right, i.e. let the SFINAE fail
instead of passing it with a "narrowing conversion" warning.
Note: the real test for command binding with immutable types
can be found in BusTerm_test
Completely removed the nested hierarchy, where
the top-level implementation forwarded to yet another
sub-implementation of the same interface. Rather, this
sub-implementation (OpClosure) is now a mere implementation
detail class without VTable, and without half-baked
re-implementation of the CmdClosure interface. And the
state-switch from unbound to bound arguments is now
implemented as a plain-flat boolean flag, instead of
hiding it in the VTable.
To make this possible, without having to rewrite lots of
tests, I've created a clone of StorageHolder as a
"proof-of-concept" dummy implementation, for the sole
purpose of writing test fixtures. This one behaves
similar to the real-world thing, but cares only
for closing the command operation and omits all
the gory details of memento capturing and undo.
...probably just an omission. TimeValue and Time are
also default constructible, and this makes sense, semantically.
Please note that Time values are *immutable* though.
Only TimeVar can be reassigned. This is so by design
