Re: [PATCH 04/13] Add skeleton dump editor

[Jonathan Nieder <jrnieder@gmail.com>]

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.

It could make sense to squash this with patch 5 as a demo.

> --- /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?

[...]
> +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.

[...]
> +++ 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.

> +};