Review of attachment 167429 [details] [review]:

The first two commits are fine; please merge.

The third commit seems mostly fine, but we need to add a "replaced" signal, so anything exposing an Individual (eg, Empathy's contact list) doesn't get stuck with a stale Individual. It'd probably be easiest as IndividualAggregator:individual-replaced (it could be plural if we can think of any decent reason).

Also, in the third commit, it'd be nicer to re-use one of the Individuals instead of destroying all and immediately creating a new one.

(In reply to comment #1)
> Also, in the third commit, it'd be nicer to re-use one of the Individuals
> instead of destroying all and immediately creating a new one.

It makes the code simpler to destroy them all and create a new one, and means that the "removed" signal is emitted for the Individual which is destroyed, ensuring that things like Empathy's contact list don't continue to expose the stale Individual.

(In reply to comment #2)
> (In reply to comment #1)
> > Also, in the third commit, it'd be nicer to re-use one of the Individuals
> > instead of destroying all and immediately creating a new one.
> 
> It makes the code simpler to destroy them all and create a new one, and means
> that the "removed" signal is emitted for the Individual which is destroyed,
> ensuring that things like Empathy's contact list don't continue to expose the
> stale Individual.

Hmm. That's true for the contact list. What about an "Individual info" dialog, like Empathy has? When the Individual disappears, it'd have to close itself, display some hint that it's stale, or just continue to display stale info. If we had a "replaced" signal, then it could stay up-to-date.

And Empathy's info window is fairly trivial, but what about a program whose purpose was just to expose a large amount of information about a person in a single view? It would be a little broken if it had to guess which new Individuals map to recently-deleted ones.

(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > Also, in the third commit, it'd be nicer to re-use one of the Individuals
> > > instead of destroying all and immediately creating a new one.
> > 
> > It makes the code simpler to destroy them all and create a new one, and means
> > that the "removed" signal is emitted for the Individual which is destroyed,
> > ensuring that things like Empathy's contact list don't continue to expose the
> > stale Individual.
> 
> Hmm. That's true for the contact list. What about an "Individual info" dialog,
> like Empathy has? When the Individual disappears, it'd have to close itself,
> display some hint that it's stale, or just continue to display stale info. If
> we had a "replaced" signal, then it could stay up-to-date.
> 
> And Empathy's info window is fairly trivial, but what about a program whose
> purpose was just to expose a large amount of information about a person in a
> single view? It would be a little broken if it had to guess which new
> Individuals map to recently-deleted ones.

Good point. A "replaced" signal seems ugly to me though, since applications would have to do much the same work in their signal handlers for "replaced" and "removed". How about extending the Individual.removed signal to provide an optional "replaced_by_this_individual" argument?

(In reply to comment #4)
> > And Empathy's info window is fairly trivial, but what about a program whose
> > purpose was just to expose a large amount of information about a person in a
> > single view? It would be a little broken if it had to guess which new
> > Individuals map to recently-deleted ones.
> 
> Good point. A "replaced" signal seems ugly to me though, since applications
> would have to do much the same work in their signal handlers for "replaced" and
> "removed". How about extending the Individual.removed signal to provide an
> optional "replaced_by_this_individual" argument?

Sounds good to me. As discussed on IRC, we'll just add an internal function in Individual for the aggregator to communicate it to the Individual so it can include argument. It's kind of round-about, but should work fine.

http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/late-linking-rebase1

How's that?

(In reply to comment #6)
> http://git.collabora.co.uk/?p=user/pwith/folks;a=shortlog;h=refs/heads/late-linking-rebase1
> 
> How's that?

Looks fine. Just fix this trivial whitespace error and commit:

+      /* If all the personas have been removed, remove the individual */
+      if (this._personas.length () < 1)
+        {
+          this.removed (replacement_individual);
+           return;
+        }

commit b049c5e8d9d48c38ed8e9230bcbfbcd4a69a7114
Author: Philip Withnall <philip.withnall@collabora.co.uk>
Date:   Fri Aug 6 15:05:53 2010 +0100

    Allow late linking of Individuals if Personas are added at runtime
    
    If a Persona is added after the initial aggregation phase and it should be
    linked to more than one existing Individual, the Individuals are now destroyed
    and their Personas added to a new linked Individual containing the late
    Persona.
    
    This is necessary for supporting runtime linking and unlinking of Personas.

 folks/individual-aggregator.vala |  139 +++++++++++++++++++++++++++++---------
 1 files changed, 107 insertions(+), 32 deletions(-)

commit a6e337363592f6bd2375496d91bdd702a4ddfbd4
Author: Philip Withnall <philip.withnall@collabora.co.uk>
Date:   Mon Aug 9 18:03:38 2010 +0100

    Specify a replacement Individual in Individual.removed()
    
    This allows clients to better keep track of which Individuals have been
    removed through the user linking them with other Individuals, and which have
    been removed for more mundane reasons.

 folks/individual-aggregator.vala |    2 +-
 folks/individual.vala            |  149 ++++++++++++++++++++------------------
 2 files changed, 79 insertions(+), 72 deletions(-)