Discussion for libtaskmanager internals

[Eike Hein]

On 08/05/2016 06:12 PM, Michail Vourlakos wrote:
> Is this the intended behavior?

Yes. A stable lifecycle in the view is achieved by the combination of
two effects: The appearance of certain items causes items "earlier" in
the lifecycle to be filtered out and sorting things into the same place
(depending on options). In our applets that's fine to achieve the
desired visual result.


> I believe that a more friendly approach is needed in my use case for
> example instead of 2 removals and 2 adds just an update in the values.
> The launcher is already there, so updating its state to startup and then
> to a window would be enough.

None of the lifecycle stages are guaranteed to occur, nor is their
order guaranteed, nor is it always possible to reliably relate them
to each other because the metadata for each can be incomplete or
unreliable. The code is therefore written for simplicity to treat
these all as separate and collapse things by filtering and sorting
where possible. It keeps all the models simple list models and the
proxy that collapses-by-filtering and sorts relatively simple as
well.

Yes, replacing add+remove with change transactions would be desirable
for some views, but it's not needed for the ones we ship, and would
make the code a fair bit more complicated, which is why it's not a
goal for now.

Feel free to work on that and we iterate on getting it in - but you
should look for feedback early and make sure you have the patience
because it's likely to be a fairly long review process (high quality
and clarity goals for this code).


> Another important aspect in the TasksModel which bothers me is that each
> item in the model does not have the same pair values. for example a
> startup item does not have an "IsWindow" value, qml triggers "undefined"
> for property "IsWindow" in its case and this is a way to ckeck its
> existence, not by true or false but from the property's existence.
> 
> Is this also the intended behavior?

AbstractTasksModel implementations that don't implement particular
data roles just fall through to QVariant() for them, which is
undefined in Qt Quick. What you can do in your QML code is to
check for true/false with the ===/==! operators, but I'd also
accept a patch that implements "missing" data roles across all the
AbstractTasksModel subclasses - I consider that a bit noisy and opted
not to do it, but it's a personal style choice and I don't mind
accomodating other styles. Feel free to submit a patch for this.


> What I am proposing is that there shouldnt be adds and removals from
> startup to window, only updates. The same is when there is a launcher,
> only updates. I believe adds and removals should be only for new items,
> for example when windows are not grouped together or are not grouped
> with their launcher. I dont consider a startup different in the model
> from the window or at least there should be a choice for the developer
> if he wants to have two different items in the model for startup and window.

See above - I agree it's nice to have for the optimal case, but the
code is essentially written for simplicity and conservativeness (i.e.
assume the optimal case won't be hit). I'm fine with someone doing
additional engineering effort to optimize the optimal case, but since
it will considerably complicate the code it needs to be reviewed very
closely.


Cheers,
Eike