libunity

Merge lp://qastaging/~kamstrup/libunity/merge-strategy into lp://qastaging/libunity

  1. merge-strategy
  2. Merge into trunk
Proposed by Mikkel Kamstrup Erlandsen
Status: Merged
Approved by: Michal Hruby
Approved revision: 115
Merged at revision: 103
Proposed branch: lp://qastaging/~kamstrup/libunity/merge-strategy
Merge into: lp://qastaging/libunity
Diff against target: 390 lines (+205/-34)
8 files modified
examples/merge-strategy.py (+27/-0)
src/Makefile.am (+1/-0)
src/unity-lens-merge-strategy.vala (+38/-0)
src/unity-lens-private.vala (+31/-7)
src/unity-lens-tools.vala (+41/-24)
src/unity-lens.vala (+19/-0)
src/unity-scope-proxy-local.vala (+1/-3)
test/vala/test-lens.vala (+47/-0)
To merge this branch: bzr merge lp://qastaging/~kamstrup/libunity/merge-strategy
Reviewer Review Type Date Requested Status
Michal Hruby (community) Approve
Review via email: mp+87585@code.qastaging.launchpad.net

Description of the change

Adds a 'merge-strategy' property to Unity.Lens that can be set to an
implementation of the Unity.MergeStrategy interface.

This can be used to control how results from scopes are merged into the
global results model for the lens.

There is a Python example added in the examples/ folder, but it requires
the pygobject patch sets from https://bugzilla.gnome.org/show_bug.cgi?id=667244
in order to work.

To post a comment you must log in.
Revision history for this message
Michal Hruby (mhr3) wrote :

Apart from the conflict, I think the changed on_scope_added method breaks stuff for non-local scopes - when ScopeProxyRemote is instantiated (and ScopeFactory emits "scope-added") the model name/address is not yet known, so the notification needs to be there. (+ it can change if the remote scope crashes)

review: Needs Fixing
Revision history for this message
Michal Hruby (mhr3) wrote :

Also, didn't you want to use array_length = false, for the Variant[] parameter?

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

afaics we don't require the model name/address when adding a remote scope as a provider in on_scope_added() - all we need to know is a pointer to the model - and that should be there from the get go, no? (unless the model is set and allocated lazily...)

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Ok, it seems that Unity.ScopeProxyRemote indeed does set up models lazily...

Revision history for this message
Michal Hruby (mhr3) wrote :

Unfortunately now the test can fail (if it's moved right after the LocalScope/Initialize test), I wouldn't mind if we added the models immediately in case that the proxy is ScopeProxyLocal (even better if the synchronizer uses a duplicate-ignoring collection.)

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Pushed a fix. Please re-review.

Revision history for this message
Michal Hruby (mhr3) wrote :

Ok, looks better, but please add a FIXME to the notifications/synchronizers that they should remove the models from their internal lists if the instance changes. (which will be completely common with private connections, might happen with remote models even now as well)

review: Approve
Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Pushed FIXMEs. Please mark the branch Approved - unless you have any last minute doubts :-)

Revision history for this message
Unity Merger (unity-merger) wrote :

Attempt to merge into lp:libunity failed due to conflicts:

text conflict in src/unity-lens-private.vala
text conflict in src/unity-lens.vala
text conflict in src/unity-scope-proxy-local.vala

Revision history for this message
Mikkel Kamstrup Erlandsen (kamstrup) wrote :

Conflicts resolved. Please re-approve.

lp://qastaging/~kamstrup/libunity/merge-strategy updated
115. By Mikkel Kamstrup Erlandsen

Sync with trunk

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches