Erik Johansson wrote on Tue, Dec 14, 2010 at 19:19:17 +0100:
> On Tue, Dec 14, 2010 at 18:17, Julian Foad <julian.f...@wandisco.com> wrote:
> > Hi Erik.  The attachment didn't reach the mailing list.  This is
> > probably because the mailing list strips off attachments that aren't
> > marked as plain text.  Please could you try again, maybe changing the
> > patch's name to '*.txt' or trying some other trick to encourage your
> > client software to mark it as a plain-text MIME type.  Cheers.
> 
> Trying again...

> [[[
> r846201 added svn_repos_replay as a "replacement not so much for
> svn_repos_dir_delta, but for at least a whole class of functionality that
> svn_repos_dir_delta was never intended to handle."
> 
> One place where svn_repos_replay replaced svn_repos_dir_delta was in
> combination with svn_repos_node_editor (only used in svnlook). This commit
> updates the documentation for svn_repos_node_editor and friends to reflect
> this.
> 
> * subversion/include/svn_repos.h
>   (svn_repos_node_editor, svn_repos_node_from_baton): Doc update
> ]]]

The patch basically does s/svn_repos_dir_delta2/svn_repos_replay2/g.
However, despite the log message, I still don't understand why that is
a correct change.

Why are we mentioning a specific driver at all?  Can svn_repos_node_editor()
be driven only by svn_repos_replay2()?

(I don't see why that would be the case.)  If not, then its doc string
should avoid mention that the returned editor can be driven only by %s,
when in fact it can be driven by anything.

> Index: subversion/include/svn_repos.h
> ===================================================================
> --- subversion/include/svn_repos.h    (revision 1045318)
> +++ subversion/include/svn_repos.h    (working copy)
> @@ -2223,20 +2223,20 @@
>  /* ---------------------------------------------------------------*/
>  
>  /**
> - * @defgroup svn_repos_inspection Data structures and editor things for
> + * @defgroup svn_repos_inspection Data structures and editor things for \
>   * repository inspection.
>   * @{
>   *
> - * As it turns out, the svn_repos_dir_delta2() interface can be
> + * As it turns out, the svn_repos_replay2() interface can be
>   * extremely useful for examining the repository, or more exactly,
> - * changes to the repository.  svn_repos_dir_delta2() allows for
> + * changes to the repository.  svn_repos_replay2() allows for
>   * differences between two trees to be described using an editor.
>   *

We have three mechanisms: svn_repos_dir_delta2(), svn_repos_begin_report2(),
and svn_repos_replay2().  I think it would be useful to just mention all of
them --- after all, none of them is deprecated and all of them can represent
a tree delta using an editor.

>   * By using the editor obtained from svn_repos_node_editor() with
> - * svn_repos_dir_delta2(), the description of how to transform one tree
> + * svn_repos_replay2(), the description of how to transform one tree
>   * into another can be used to build an in-memory linked-list tree,
>   * which each node representing a repository node that was changed as a
> - * result of having svn_repos_dir_delta2() drive that editor.
> + * result of having svn_repos_replay2() drive that editor.
>   */
>  
>  /** A node in the repository. */
> @@ -2276,7 +2276,7 @@
>  
>  
>  /** Set @a *editor and @a *edit_baton to an editor that, when driven by
> - * svn_repos_dir_delta2(), builds an <tt>svn_repos_node_t *</tt> tree
> + * svn_repos_replay2(), builds an <tt>svn_repos_node_t *</tt> tree
>   * representing the delta from @a base_root to @a root in @a repos's
>   * filesystem.
>   *

> @@ -2300,7 +2300,7 @@
>  
>  /** Return the root node of the linked-list tree generated by driving
>   * the editor created by svn_repos_node_editor() with
> - * svn_repos_dir_delta2(), which is stored in @a edit_baton.  This is
> + * svn_repos_replay2(), which is stored in @a edit_baton.  This is
>   * only really useful if used *after* the editor drive is completed.
>   */
>  svn_repos_node_t *

What do you think?

Reply via email to