On 9/13/05, Max Bowsher <maxb_at_ukf.net> wrote:> Christopher Ness wrote:> > [[[> > THIS PATCH IS NOT READY TO BE COMMITTED.> >> > Closure of the second half of issue 571, "svn update: D+A should say R"> > http://subversion.tigris.org/issues/show_bug.cgi?id=571> >> > Patch By: Chris Ness <chris_at_nesser.org>> > Design By: eh> > Review By:> >> > * subversion/include/svn_wc.h> > (svn_wc_notify_action_t): New enumerated type> > svn_wc_notify_update_replace> > * subversion/libsvn_wc/update_editor.c> > (): New defines> > (struct dir_baton): New hash table deleted_targets> > (make_dir_baton): Initialize hash table when a new dir_baton is> > created.> > (do_entry_deletion): No longer notifies about file deletions, this is> > delayed by use of the hash table.> > (delete_entry): Adds to hash table, and if key exists notifies of> > a replacement of a file and removes that key from the hash.> > (close_directory): Walks hash for leftover keys marked as deleted> > and notifies them as deleted.
Ignores added hashes.> > Removes entries from the hash.> > (close_file): Adds targets to the hash as added since reverse merges> > of replaced files can also occur. If target exists it notifies of> > a replacement and removes the target from the hash.> > (hash_get_set): New local function which tries to return the hash> > key. If not found it adds the target to the hash table.> > * subversion/clients/cmdline/notify.c> > (notify): New case to handle svn_wc_notify_update_replace> > ]]]> > > I'm a bit hesitant about the way many delete notifications can be saved up> until close_directory().
Well, I have a different point:
I know breser modified the code to send deletes first, but is that aneditor API guarantee? > Do we really need a hash at all? Are not all D+A combinations immediately> consecutive, meaning that all we have to do is to queue exactly one delete> notification at any one time, in case the next notification is an add for> the same path?> That ought to reduce the complexity of the code quite a lot.
Yes, but even if the current code does it that way, do we have aneditor guarantee that it will always work that way? > Pseudocode:> do_notify (notification)> {> if (pending_delete_notification)> {> if (notification->action == ADD &&> strcmp(notification->path, pending_delete_notification->path) == 0)> {> output (REPLACE, notification->path);> pending_delete_notification = NULL;> return; /* All done */> }> else> {> output (pending_delete_notification->action,> pending_delete_notification->path);> pending_delete_notification = NULL;> }> }> > if (notification->action == DELETE)> pending_delete_notification = notification;> else> output (notification->action, notification->path);> }> > Plus code to flush pending_delete_notification on close_directory()
If we require D+A to be consecutive, then, yes that's a possibility,but what if we only require that the list of changes is ordered insuch a way that D's are first and A's are later (but not consecutive)?Might any future code change benefit from not having too strictrequirements?
bye,
Erik.
Received on Tue Sep 13 16:50:22 2005