Greg Hudson <ghudson@MIT.EDU> writes:
> There is no way for an editor to tell when the edit is done except by
> counting opens and closes? I hadn't noticed that ramification of the
> change #1 proposal and I think it's a total non-starter.
>
> (If you guys actually pointed this out in the description of change #1
> and I missed it, then I apologize for not speaking up until now.)
It wasn't in the description of Change #1 (no one realized the
implication for the xml output editor), but the working copy update
editor has worked this way for a long time now.
In general, I'm not thrilled about the ref counting either, but don't
see how to avoid something like it. It was a compromise: given that
we wanted to have a nested tree delta, and yet defer the sending of
txdeltas until the end, then "closing" a directory doesn't really mean
that everything in that directory is done yet. Ref counting seemed
the cleanest way to do it.
> I don't really relish debugging this, given how fundamentally you've
> changed the design assumptions of the editor.
I don't blame you, and apologize for dumping the burden on you when
you (effectively) weren't consulted abut the decision in the first
place.
> If it's okay, I'm going
> to start over and redo the change by keeping a count of the number of
> open files in edit_context. That way the deletion of directory batons
> doesn't need to be deferred.
Sure. I didn't think of that.
By the way, doing things this way *was* actually part of the design
assumptions of the editor, but this particular assumption was never
very well publicized. The following comment, which has been in
svn_delta.h for some time, pretty much sums it up:
/* [...]
5. When the producer calls `replace_file' or `add_file', either:
(a) The producer must follow with the changes to the file
(`change_file_prop' and/or `apply_textdelta', as applicable)
followed by a `close_file' call, before issuing any other file
or directory calls, or
(b) The producer must follow with a `change_file_prop' call if
it is applicable, before issuing any other file or directory
calls; later, after all directory batons including the root
have been closed, the producer must issue `apply_textdelta'
and `close_file' calls.
6. When the producer calls `apply_textdelta', it must make all of
the window handler calls (including the NULL window at the
end) before issuing any other edit_fns calls.
So, the producer needs to use directory and file batons as if it
is doing a single depth-first traversal of the tree, with the
exception that the producer may keep file batons open in order to
make apply_textdelta calls at the end.
These restrictions make it easier to write a consumer that
generates an XML-style tree delta. An XML tree delta mentions
each directory once, and includes all the changes to that
directory within the <directory> element. However, it does allow
text deltas to appear at the end. */
If you come to the conclusion that the whole Change #1 is bogus,
please post (I know you will :-) ).
I do think it's a good thing for Subversion the long term, because of
the way replace_root() was somewhat redundant with getting the editor
in the first place.
Although I can certainly sympathize with what you wrote:
> There is no way for an editor to tell when the edit is done except by
> counting opens and closes? I hadn't noticed that ramification of the
> change #1 proposal and I think it's a total non-starter.
...I also think that not having close_edit() makes more sense. I
mean, what if one *hadn't* yet balanced opens with closes, but called
close_edit() anyway? What would it mean?
If the editor is really a tree editor, it's reasonable that closing
the top-level change -- the one on the root directory -- should
signify the end of the edit.
However, I'm also feeling the pain of the change, and if you'd like to
discuss reverting it, I think everyone's certainly willing to listen.
I promise that the 2.5 days of work I put into the change will not
bias me one way or the other (seriously).
-K
Received on Sat Oct 21 14:36:20 2006