Re: Some fixes



Philip Van Hoof wrote:
> On Tue, 2007-03-13 at 19:24 +0100, Philip Van Hoof wrote:
>>> The folder store used to create the change object was also wrong.
>> -       change = tny_folder_store_change_new (TNY_FOLDER_STORE (self));
>> +       change = tny_folder_store_change_new (into);
>>
>> Are you sure that it should be "into" as the store being changed?
>> Because it's the parent store that is being changed, not the created or
>> deleted folder.

Indeed, into is the new parent of the copied/moved folder.

> The folder-store that you need to pass to TnyFolderStoreChange's
> constructor is the one that is being changed. In case of deleting a
> subfolder, it's not the subfolder being deleted that changed but the
> parent of that subfolder being deleted that looses a subfolder.
> 
> Same for creations: it can't be the created one, as no one will be
> listening for that one. For the deleted one: no one will keep listening
> for that one (which reminds me to check whether in case a folder is
> deleted, that all its observers are forcefully unsubscribed too).
> 
> Not the same in case of renames. It's not the parent that notices a
> rename, it's the @self itself that feels the rename. Because other than
> a name change, nothing to @self's state changed (it's not a destruction
> nor a creation).

Mmm, so the code should be slightly different, because currently
tinymail always creates a new TnyCamelFolder. So, IMO it should be
something like this

if (folder_has_been_renamed) { // <- a move within the same folder store
  change = tny_folder_change_new (self)
  tny_folder_change_set_rename (change, name);
  notify_folder_observers_about (self, change);
}
else {
  new_folder = _tny_camel_folder_new ();

  [...] initialization stuff

  change = tny_folder_store_change_new (into);
  tny_folder_store_change_add_created_folder (change, new_folder);
  if (del)
    tny_folder_store_change_add_removed_folder (change, self);
  notify_folder_store_observers_about (into, change);

  [...]
}

The removed_folder stuff works because the tree model checks all the
nodes no matter the FolderStore used to create the FolderStoreChange but
I think it would be better to create another Change object and notify
the removal with another call.

Br



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]