In svnserve/serve.c the two functions "switch_cmd" and "diff" are
identical except that one passes TRUE and the other FALSE to the
"text_deltas" parameter of svn_repos_begin_report. The functions are 50
lines long - not terribly complex but definitely worth factoring out to
make the similarity obvious. Of course they might not remain this
similar in future, but at present they are. A suggested factoring is
given in the first diff below. Is this appropriate?
Closely related, and therefore in this same message:
I think the comment for the "text_deltas" flag has a typo that could be
fixed by removing two spurious words, as in the second diff below. But
maybe there is more to it. I don't quite understand how this single
flag makes the whole difference between "svn diff" and "svn switch".
Could someone who knows please verify that "false" just means "don't
generate text deltas"; if it means "do something else instead", then the
comment ought to say what it does instead.
- Julian
* subversion/svnserve/serve.c:
Factor out the common body of functions "switch_cmd" and "diff".
* subversion/include/svn_repos.h:
Fix typo in comment.
Index: subversion/svnserve/serve.c
===================================================================
--- subversion/svnserve/serve.c (revision 4704)
+++ subversion/svnserve/serve.c (working copy)
@@ -571,13 +571,16 @@
return handle_report(conn, pool, b->repos_url, report_baton);
}
-static svn_error_t *switch_cmd(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
- apr_array_header_t *params, void *baton)
+/* Presently the implementations of "switch" and "diff" are so similar that
+ * they share this code. They need not continue to be so similar in
future. */
+static svn_error_t *switch_or_diff(svn_ra_svn_conn_t *conn, apr_pool_t
*pool,
+ apr_array_header_t *params, void *baton,
+ svn_boolean_t do_switch)
{
server_baton_t *b = baton;
svn_revnum_t rev;
const char *target;
- const char *switch_url, *switch_path;
+ const char *other_url, *other_path;
svn_boolean_t recurse;
const svn_delta_editor_t *editor;
void *edit_baton, *report_baton;
@@ -586,31 +589,31 @@
/* Parse the arguments. */
SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "[r]cbc", &rev, &target,
- &recurse, &switch_url));
+ &recurse, &other_url));
if (svn_path_is_empty(target))
target = NULL; /* ### Compatibility hack, shouldn't be needed */
if (!SVN_IS_VALID_REVNUM(rev))
SVN_CMD_ERR(svn_fs_youngest_rev(&rev, b->fs, pool));
- /* Verify that switch_url is in the same repository and get its fs
path. */
- switch_url = svn_path_uri_decode(switch_url, pool);
+ /* Verify that other_url is in the same repository and get its fs
path. */
+ other_url = svn_path_uri_decode(other_url, pool);
len = strlen(b->repos_url);
- if (strncmp(switch_url, b->repos_url, len) != 0)
+ if (strncmp(other_url, b->repos_url, len) != 0)
{
err = svn_error_createf (SVN_ERR_RA_ILLEGAL_URL, NULL,
"'%s'\n"
"is not the same repository as\n"
- "'%s'", switch_url, b->repos_url);
+ "'%s'", other_url, b->repos_url);
/* Wrap error so that it gets reported back to the client. */
return svn_error_create(SVN_ERR_RA_SVN_CMD_ERR, err, NULL);
}
- switch_path = switch_url + len;
+ other_path = other_url + len;
/* Make an svn_repos report baton. Tell it to drive the network editor
* when the report is complete. */
svn_ra_svn_get_editor(&editor, &edit_baton, conn, pool, NULL, NULL);
SVN_CMD_ERR(svn_repos_begin_report(&report_baton, rev, b->user,
b->repos,
- b->fs_path, target, switch_path, TRUE,
+ b->fs_path, target, other_path,
do_switch,
recurse, editor, edit_baton, pool));
/* Write an empty command-reponse, telling the client to start
reporting. */
@@ -621,6 +624,18 @@
return handle_report(conn, pool, b->repos_url, report_baton);
}
+static svn_error_t *switch_cmd(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
+ apr_array_header_t *params, void *baton)
+{
+ return switch_or_diff(conn, pool, params, baton, TRUE);
+}
+
+static svn_error_t *diff(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
+ apr_array_header_t *params, void *baton)
+{
+ return switch_or_diff(conn, pool, params, baton, FALSE);
+}
+
static svn_error_t *status(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
apr_array_header_t *params, void *baton)
{
@@ -652,56 +667,6 @@
return handle_report(conn, pool, b->repos_url, report_baton);
}
-static svn_error_t *diff(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
- apr_array_header_t *params, void *baton)
-{
- server_baton_t *b = baton;
- svn_revnum_t rev;
- const char *target, *versus_url, *versus_path;
- svn_boolean_t recurse;
- const svn_delta_editor_t *editor;
- void *edit_baton, *report_baton;
-
- int len;
- svn_error_t *err;
-
- /* Parse the arguments. */
- SVN_ERR(svn_ra_svn_parse_tuple(params, pool, "[r]cbc", &rev, &target,
- &recurse, &versus_url));
- if (svn_path_is_empty(target))
- target = NULL; /* ### Compatibility hack, shouldn't be needed */
- if (!SVN_IS_VALID_REVNUM(rev))
- SVN_CMD_ERR(svn_fs_youngest_rev(&rev, b->fs, pool));
-
- /* Verify that versus_url is in the same repository and get its fs
path. */
- versus_url = svn_path_uri_decode(versus_url, pool);
- len = strlen(b->repos_url);
- if (strncmp(versus_url, b->repos_url, len) != 0)
- {
- err = svn_error_createf (SVN_ERR_RA_ILLEGAL_URL, NULL,
- "'%s'\n"
- "is not the same repository as\n"
- "'%s'", versus_url, b->repos_url);
- /* Wrap error so that it gets reported back to the client. */
- return svn_error_create(SVN_ERR_RA_SVN_CMD_ERR, err, NULL);
- }
- versus_path = versus_url + len;
-
- /* Make an svn_repos report baton. Tell it to drive the network editor
- * when the report is complete. */
- svn_ra_svn_get_editor(&editor, &edit_baton, conn, pool, NULL, NULL);
- SVN_CMD_ERR(svn_repos_begin_report(&report_baton, rev, b->user, b->repos,
- b->fs_path, target, versus_path,
FALSE,
- recurse, editor, edit_baton, pool));
-
- /* Write an empty command-reponse, telling the client to start
reporting. */
- SVN_ERR(svn_ra_svn_write_cmd_response(conn, pool, ""));
-
- /* Handle the client's report; when it's done, svn_repos will drive the
- * network editor with the diff. */
- return handle_report(conn, pool, b->repos_url, report_baton);
-}
-
/* Send a log entry to the client. */
static svn_error_t *log_receiver(void *baton, apr_hash_t *changed_paths,
svn_revnum_t rev, const char *author,
Index: subversion/include/svn_repos.h
===================================================================
--- subversion/include/svn_repos.h (revision 4704)
+++ subversion/include/svn_repos.h (working copy)
@@ -182,7 +182,7 @@
* transform the reported heirarchy to revision @a revnum, preserving the
* reported heirarchy.
*
- * @a text_deltas instructs the driver of the @a editor to enable to
disable
+ * @a text_deltas instructs the driver of the @a editor to enable
* the generation of text deltas.
*
* @a recurse instructs the driver of the @a editor to send a recursive
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Feb 3 23:34:15 2003