Hello Greg,
On Donnerstag, 2. April 2009, Greg Stein wrote:
> Here are my notes on a rewrite of the editor vtable. I'll be
> continuing to expand on this file, particularly as people ask
> questions. Being notes, it isn't entirely clear in some spots, but
> I'll expand/rewrite as discussion continues.
here are some remarks.
Please excuse any stupid questions.
Regards,
Phil
> > @@ -0,0 +1,100 @@
> > +NOTES on a revamped editor interface
> > +
> > +-- all paths are relpaths
> > + -- could be: relative to wcroot, or repos, or <what else?>
I'd hope that they're relative to the object the "parent" baton belongs to -
ie. for ->delete_entry the directory of the given dir_baton.
> > +-- expectations around *what* is being changed
...
> > + 5. move REV: source will be like a delete, so could have been
> > edited/deleted +
I'd expect that a MOVE will have to give a "base" revision, and if that's no
longer current there'd be an "out of date".
> > +API will be like svn_stream_t, RA, and FS
...
> > + -- no return values, so scratch_pool everywhere
What does that mean? I'm a bit worried about APR's behaviour to close the file
on pool cleanup, because it's not that easy anymore to check for errors.
Or do you think about something completely different here, ie. return "values"
as in "results" (like a path, struct, ...)?
> > +add_directory(path, children, props)
> > + -- name all children. receiver can put on "pending" list
> > + -- MUST be followed (at some point) by add_*() calls for each child
Why should children be given, if they're to be done explicitly later anyway?
> > +set_props(path, against_rev, props, complete)
> > + -- 'complete' indicates whether this completes the change to this node
> > + -- MUST be TRUE for directories
> > + -- if TRUE, then a set_text() MUST follow if node is a file
> > + -- if TRUE, then a set_target() MUST follow if node is a symlink
Maybe a flag to say "append/replace these" vs. "replace all properties with
that hash"?
> > +set_text(path, against_rev, checksum, stream)
> > + -- completes the change for this file (ie. if set_props was called)
> > + -- stream can be ignored by recipient if it knows the checksum
> > + -- include a mime type to help recipient manage stream processing?
might checksum be a hash? Eg. "I know the MD5 and the SHA1 - take what you
want"
> > +set_target(path, against_rev, target)
> > + -- completes the change for this symlink (ie. if set_props was called)
For the three above: why a against_rev? Is the number given to open_root()
et.al. not enough?
> > +callback:
> > +get_info(path, *kind, *revision, *checksum, *children, *target,
> > + *props, *stream)
Again: Post a wish regarding the checksum returned? Maybe the repository
stores more than one.
> > +new interface: svn_editor.h
> > + -- don't try to rev svn_delta_editor_t. creates a huge mess
> > + -- svn_editor_* prefix
> > + -- svn_editor_setcb_* to set the various callbacks
> > + -- svn_editor_setcb_many(vtable) to copy funcs in
Could use a some other parameter (size of table) or a hash instead, to be
completely future-proof.
Received on 2009-04-06 08:47:56 CEST