Re: [PATCH 04/13] Add skeleton dump editor
- From
Ramkumar Ramachandra <artagnon@gmail.com>
- Date
- Jul 8, 2010, 06:17 UTC
- Message-ID
- <20100708061736.GB3264@debian>
- In-Reply-To
- <20100707181619.GA2617@burratino>
Hi,
Jonathan Nieder writes:
Show 13 quoted lines
> Ramkumar Ramachandra wrote: > > > Add a dump editor and write out skeleton callback functions according > > to the API documentation of svn_delta_editor_t. Also expose > > get_dump_editor through a header. > > This commit message tells me nothing... Maybe: > > Add a no-op svn_editor. The function to retrieve it is called > get_dump_editor because it is planned to tweak it to write a > dumpfile. But for now it is more useful when used with the > debug_editor, to get a list of editor operations printed to > stderr.
Fixed.
> It could make sense to squash this with patch 5 as a demo.
Hm, I'm not too competent with handling lots of conflicts during an interactive rebase: I'll try this out in a new branch and let you know how it went.
Show 19 quoted lines
> > --- /dev/null > > +++ b/dump_editor.c > > @@ -0,0 +1,143 @@ > > +/* Licensed under a two-clause BSD-style license. > > + * See LICENSE for details. > > + */ > > + > > +#include "svn_pools.h" > > +#include "svn_error.h" > > +#include "svn_iter.h" > > +#include "svn_repos.h" > > +#include "svn_string.h" > > +#include "svn_dirent_uri.h" > > +#include "svn_path.h" > > +#include "svn_time.h" > > +#include "svn_checksum.h" > > +#include "svn_props.h" > > Are these all needed?
I figured that adding some implies adding some others. Fixed.
Show 27 quoted lines
> [...]
> > +svn_error_t *open_root(void *edit_baton,
> > + svn_revnum_t base_revision,
> > + apr_pool_t *pool,
> > + void **root_baton)
> > +{
> > + return SVN_NO_ERROR;
> > +}
>
> Might make sense to use
>
> *root_baton = NULL;
>
> for easier debugging.
>
> [...]
> > +svn_error_t *add_directory(const char *path,
> [...]
> > +svn_error_t *open_directory(const char *path,
> [...]
> > +svn_error_t *add_file(const char *path,
> [...]
> > +svn_error_t *open_file(const char *path,
> [...]
> > +svn_error_t *apply_textdelta(void *file_baton, const char *base_checksum,
>
> Likewise.Fixed. Excellent suggestion.
Show 44 quoted lines
> [...]
> > +++ b/dumpr_util.h
> > @@ -0,0 +1,29 @@
> > +#ifndef DUMPR_UTIL_H_
> > +#define DUMPR_UTIL_H_
> > +
> > +struct edit_baton {
>
> A more specific name might be nice (or might not, depending on the
> prevailing style in svn; I ought to check but I am too lazy).
>
> > + /* The stream to dump to: stdout */
> > + svn_stream_t *stream;
> > +
> > + /* pool is for per-edit-session allocations */
> > + apr_pool_t *pool;
>
> Unused; probably should delay adding these until there is a user to
> explain them.
>
> > +
> > + svn_revnum_t current_rev;
>
> Used.
>
> > +
> > + /* Store the properties that changed */
> > + apr_hash_t *properties;
> > + apr_hash_t *del_properties; /* Value is always 0x1 */
> > + svn_stringbuf_t *propstring;
> > +
> > + /* Path of changed file */
> > + const char *changed_path;
> > +
> > + /* Was a copy command issued? */
> > + svn_boolean_t is_copy;
> > +
> > + /* Temporary file to write delta to along with its checksum */
> > + char *temp_filepath;
> > + svn_checksum_t *checksum;
>
> All unused.
>
> > +};Here's the diff after your review. It took me quite a long time to get the interactive rebase right.
Add skeleton dump editor
Add a no-op svn_editor and expose the function get_dump_editor; this dump editor will later be filled in to write the dumpfile. Currently, it is more useful to wrap it in the debug_editor to get a list of editor operations printed to stderr.
diff --git a/dump_editor.c b/dump_editor.c index 2fdf93c..e1f3fca 100644 --- a/dump_editor.c +++ b/dump_editor.c @@ -3,14 +3,8 @@ */ #include "svn_pools.h" -#include "svn_error.h" -#include "svn_iter.h" #include "svn_repos.h" -#include "svn_string.h" -#include "svn_dirent_uri.h" #include "svn_path.h" -#include "svn_time.h" -#include "svn_checksum.h" #include "svn_props.h" #include "dumpr_util.h" @@ -20,6 +14,7 @@ svn_error_t *open_root(void *edit_baton, apr_pool_t *pool, void **root_baton) { + *root_baton = NULL; return SVN_NO_ERROR; } @@ -38,6 +33,7 @@ svn_error_t *add_directory(const char *path, apr_pool_t *pool, void **child_baton) { + *child_baton = NULL; return SVN_NO_ERROR; } @@ -47,6 +43,7 @@ svn_error_t *open_directory(const char *path, apr_pool_t *pool, void **child_baton) { + *child_baton = NULL; return SVN_NO_ERROR; } @@ -63,6 +60,7 @@ svn_error_t *add_file(const char *path, apr_pool_t *pool, void **file_baton) { + *file_baton = NULL; return SVN_NO_ERROR; } @@ -72,6 +70,7 @@ svn_error_t *open_file(const char *path, apr_pool_t *pool, void **file_baton) { + *file_baton = NULL; return SVN_NO_ERROR; } @@ -96,6 +95,8 @@ svn_error_t *apply_textdelta(void *file_baton, const char *base_checksum, svn_txdelta_window_handler_t *handler, void **handler_baton) { + *handler = svn_delta_noop_window_handler; + *handler_baton = NULL; return SVN_NO_ERROR; } diff --git a/dumpr_util.h b/dumpr_util.h index d206c19..2906543 100644 --- a/dumpr_util.h +++ b/dumpr_util.h @@ -2,28 +2,8 @@ #define DUMPR_UTIL_H_ struct edit_baton { - /* The stream to dump to: stdout */ svn_stream_t *stream; - - /* pool is for per-edit-session allocations */ - apr_pool_t *pool; - svn_revnum_t current_rev; - - /* Store the properties that changed */ - apr_hash_t *properties; - apr_hash_t *del_properties; /* Value is always 0x1 */ - svn_stringbuf_t *propstring; - - /* Path of changed file */ - const char *changed_path; - - /* Was a copy command issued? */ - svn_boolean_t is_copy; - - /* Temporary file to write delta to along with its checksum */ - char *temp_filepath; - svn_checksum_t *checksum; }; #endif