From: Ramkumar Ramachandra <artagnon@gmail.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: "Git Mailing List" <git@vger.kernel.org>,
"David Michael Barr" <david.barr@cordelta.com>,
"Sverre Rabbelier" <srabbelier@gmail.com>,
"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
"Daniel Shahaf" <d.s@daniel.shahaf.name>,
"Bert Huijben" <rhuijben@collab.net>,
"Junio C Hamano" <gitster@pobox.com>,
"Eric Wong" <normalperson@yhbt.net>,
dev@subversion.apache.org
Subject: Re: [PATCH 04/13] Add skeleton dump editor
Date: Thu, 8 Jul 2010 08:17:36 +0200 [thread overview]
Message-ID: <20100708061736.GB3264@debian> (raw)
In-Reply-To: <20100707181619.GA2617@burratino>
Hi,
Jonathan Nieder writes:
> 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.
> > --- /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.
> [...]
> > +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.
> [...]
> > +++ 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
next prev parent reply other threads:[~2010-07-08 6:16 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-07-07 0:14 [GSoC update] git-remote-svn: Week 10 Ramkumar Ramachandra
2010-07-07 0:14 ` [PATCH 01/13] Add LICENSE Ramkumar Ramachandra
2010-07-07 0:14 ` [PATCH 02/13] Add skeleton SVN client and Makefile Ramkumar Ramachandra
2010-07-07 16:25 ` Jonathan Nieder
2010-07-07 17:09 ` Ramkumar Ramachandra
2010-07-07 19:30 ` Jonathan Nieder
2010-07-07 20:47 ` Ramkumar Ramachandra
2010-07-07 17:51 ` Daniel Shahaf
2010-07-07 0:14 ` [PATCH 03/13] Add debug editor from Subversion trunk Ramkumar Ramachandra
2010-07-07 17:55 ` Jonathan Nieder
2010-07-07 0:14 ` [PATCH 04/13] Add skeleton dump editor Ramkumar Ramachandra
2010-07-07 18:16 ` Jonathan Nieder
2010-07-08 6:17 ` Ramkumar Ramachandra [this message]
2010-07-07 0:14 ` [PATCH 05/13] Drive the debug editor Ramkumar Ramachandra
2010-07-07 18:26 ` Jonathan Nieder
2010-07-07 19:08 ` Ramkumar Ramachandra
2010-07-07 19:53 ` Jonathan Nieder
2010-07-08 6:04 ` Ramkumar Ramachandra
2010-07-07 0:14 ` [PATCH 06/13] Dump the revprops at the start of every revision Ramkumar Ramachandra
2010-07-07 19:04 ` Jonathan Nieder
2010-07-21 18:55 ` Ramkumar Ramachandra
2010-07-26 14:03 ` Julian Foad
2010-07-26 17:53 ` Ramkumar Ramachandra
2010-07-07 0:14 ` [PATCH 07/13] Implement open_root and close_edit Ramkumar Ramachandra
2010-07-07 0:14 ` [PATCH 08/13] Implement dump_node Ramkumar Ramachandra
2010-07-07 0:14 ` [PATCH 09/13] Implement directory-related functions Ramkumar Ramachandra
2010-07-07 0:14 ` [PATCH 10/13] Implement file-related functions Ramkumar Ramachandra
2010-07-07 0:14 ` [PATCH 11/13] Implement apply_textdelta Ramkumar Ramachandra
2010-07-07 0:14 ` [PATCH 12/13] Implement close_file Ramkumar Ramachandra
2010-07-07 0:14 ` [PATCH 13/13] Add a validation script Ramkumar Ramachandra
2010-07-07 20:24 ` [GSoC update] git-remote-svn: Week 10 Ramkumar Ramachandra
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100708061736.GB3264@debian \
--to=artagnon@gmail.com \
--cc=avarab@gmail.com \
--cc=d.s@daniel.shahaf.name \
--cc=david.barr@cordelta.com \
--cc=dev@subversion.apache.org \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jrnieder@gmail.com \
--cc=normalperson@yhbt.net \
--cc=rhuijben@collab.net \
--cc=srabbelier@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).