You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2010/06/07 14:43:19 UTC
svn commit: r952205 - /subversion/trunk/subversion/libsvn_client/diff.c
Author: dannas
Date: Mon Jun 7 12:43:19 2010
New Revision: 952205
URL: http://svn.apache.org/viewvc?rev=952205&view=rev
Log:
Use several smaller functions for printing git diff headers instead of one.
* subversion/libsvn_client/diff.c
(print_git_diff_header): Split this one into ...
(print_git_diff_header_added): ..this ...
(print_git_diff_header_deleted): .. and this ...
(print_git_diff_header_copied): .. and this ...
(print_git_diff_header_moved): .. and this ...
(print_git_diff_header_modified): .. and this.
(diff_content_changed): Adjust caller.
Modified:
subversion/trunk/subversion/libsvn_client/diff.c
Modified: subversion/trunk/subversion/libsvn_client/diff.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/diff.c?rev=952205&r1=952204&r2=952205&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/diff.c (original)
+++ subversion/trunk/subversion/libsvn_client/diff.c Mon Jun 7 12:43:19 2010
@@ -307,65 +307,89 @@ display_prop_diffs(const apr_array_heade
}
#ifdef SVN_EXPERIMENTAL_PATCH
+
/*
* Print a git diff header for PATH to the stream OS using HEADER_ENCODING.
- * The header lines are determined from what operation is performed on the
- * file using DELETED, COPIED, MOVED and ADDED. When the
- * operation is a move or copy, copyfrom_path is used to determine the
- * source. All allocations are done in RESULT_POOL. */
-static svn_error_t *
-print_git_diff_header(svn_stream_t *os, const char *copyfrom_path,
- svn_boolean_t deleted, svn_boolean_t copied,
- svn_boolean_t moved, svn_boolean_t added,
- const char *header_encoding, const char *path,
- apr_pool_t *result_pool)
+ * All allocations are done in RESULT_POOL. */
+static svn_error_t *
+print_git_diff_header_added(svn_stream_t *os, const char *header_encoding,
+ const char *path, apr_pool_t *result_pool)
{
- if (copied)
- {
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "diff --git a/%s b/%s%s",
- copyfrom_path, path, APR_EOL_STR));
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "copy from %s%s", path, APR_EOL_STR));
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "copy to %s%s", copyfrom_path,
- APR_EOL_STR));
- }
- else if (moved)
- {
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "diff --git a/%s b/%s%s",
- copyfrom_path, path, APR_EOL_STR));
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "rename from %s%s", path,
- APR_EOL_STR));
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "rename to %s%s", copyfrom_path,
- APR_EOL_STR));
- }
- else if (deleted)
- {
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "diff --git a/%s b/%s%s",
- path, path, APR_EOL_STR));
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "deleted file mode 10644"
- APR_EOL_STR));
- }
- else if (added)
- {
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "diff --git a/%s b/%s%s",
- path, path, APR_EOL_STR));
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "new file mode 10644" APR_EOL_STR));
- }
- else
- {
- SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
- "diff --git a/%s b/%s%s",
- path, path, APR_EOL_STR));
- }
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "diff --git a/%s b/%s%s",
+ path, path, APR_EOL_STR));
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "new file mode 10644" APR_EOL_STR));
+ return SVN_NO_ERROR;
+}
+
+/*
+ * Print a git diff header for PATH to the stream OS using HEADER_ENCODING.
+ * All allocations are done in RESULT_POOL. */
+static svn_error_t *
+print_git_diff_header_deleted(svn_stream_t *os, const char *header_encoding,
+ const char *path, apr_pool_t *result_pool)
+{
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "diff --git a/%s b/%s%s",
+ path, path, APR_EOL_STR));
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "deleted file mode 10644"
+ APR_EOL_STR));
+ return SVN_NO_ERROR;
+}
+
+/*
+ * Print a git diff header for PATH to the stream OS using HEADER_ENCODING.
+ * COPYFROM_PATH is the origin of the operation. All allocations are done
+ * in RESULT_POOL. */
+static svn_error_t *
+print_git_diff_header_copied(svn_stream_t *os, const char *header_encoding,
+ const char *path, const char *copyfrom_path,
+ apr_pool_t *result_pool)
+{
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "diff --git a/%s b/%s%s",
+ copyfrom_path, path, APR_EOL_STR));
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "copy from %s%s", path, APR_EOL_STR));
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "copy to %s%s", copyfrom_path,
+ APR_EOL_STR));
+ return SVN_NO_ERROR;
+}
+
+/*
+ * Print a git diff header for PATH to the stream OS using HEADER_ENCODING.
+ * COPYFROM_PATH is the origin of the operation. All allocations are done
+ * in RESULT_POOL. */
+static svn_error_t *
+print_git_diff_header_moved(svn_stream_t *os, const char *header_encoding,
+ const char *path, const char *copyfrom_path,
+ apr_pool_t *result_pool)
+{
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "diff --git a/%s b/%s%s",
+ copyfrom_path, path, APR_EOL_STR));
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "rename from %s%s", path,
+ APR_EOL_STR));
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "rename to %s%s", copyfrom_path,
+ APR_EOL_STR));
+ return SVN_NO_ERROR;
+}
+
+/*
+ * Print a git diff header for PATH to the stream OS using HEADER_ENCODING.
+ * All allocations are done in RESULT_POOL. */
+static svn_error_t *
+print_git_diff_header_modified(svn_stream_t *os, const char *header_encoding,
+ const char *path, apr_pool_t *result_pool)
+{
+ SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
+ "diff --git a/%s b/%s%s",
+ path, path, APR_EOL_STR));
return SVN_NO_ERROR;
}
#endif
@@ -681,42 +705,43 @@ diff_content_changed(const char *path,
path, equal_string));
#ifdef SVN_EXPERIMENTAL_PATCH
- /* We adjust the labels to comply with the git unidiff standard.
- *
- * ### Move this logic into print_git_diff_header?
- *
- * ### If the paths has different length, the revisions will not
- * ### be in the same column. Is that a problem? */
+ /* Add git headers and adjust the labels.
+ * ### Once we're using the git format everywhere, we can create
+ * ### one func that sets the correct labels in one place. */
if (diff_cmd_baton->deleted)
{
+ SVN_ERR(print_git_diff_header_deleted(
+ os,
+ diff_cmd_baton->header_encoding,
+ path, subpool));
+
label1 = diff_label(apr_psprintf(subpool, "a/%s", path1), rev1,
subpool);
label2 = diff_label("/dev/null", rev2, subpool);
}
else if (diff_cmd_baton->added)
{
+ SVN_ERR(print_git_diff_header_added(
+ os,
+ diff_cmd_baton->header_encoding,
+ path, subpool));
label1 = diff_label("/dev/null", rev1, subpool);
label2 = diff_label(apr_psprintf(subpool, "b/%s", path2), rev2,
subpool);
}
else
{
+ SVN_ERR(print_git_diff_header_modified(
+ os,
+ diff_cmd_baton->header_encoding,
+ path, subpool));
label1 = diff_label(apr_psprintf(subpool, "a/%s", path1), rev1,
subpool);
label2 = diff_label(apr_psprintf(subpool, "b/%s", path2), rev2,
subpool);
}
-
- /* ### Passing FALSE for added, copied and moved and NULL for
- * ### copyfrom_path since we're only dealing with deleted paths for
- * ### git headers at this early point. */
- SVN_ERR(print_git_diff_header(os, NULL, /* copyfrom_path */
- diff_cmd_baton->deleted,
- FALSE, /* copied */
- FALSE, /* moved */
- diff_cmd_baton->added,
- diff_cmd_baton->header_encoding,
- path, subpool));
+
+ /* ### Print git headers for copies and renames too. */
#endif
/* Output the actual diff */
SVN_ERR(svn_diff_file_output_unified3
Re: svn commit: r952205 -
/subversion/trunk/subversion/libsvn_client/diff.c
Posted by Daniel Näslund <da...@longitudo.com>.
On Mon, Jun 07, 2010 at 03:33:16PM +0200, Stefan Sperling wrote:
> On Mon, Jun 07, 2010 at 12:43:19PM -0000, dannas@apache.org wrote:
> > +static svn_error_t *
> > +print_git_diff_header_copied(svn_stream_t *os, const char *header_encoding,
> > + const char *path, const char *copyfrom_path,
> > + apr_pool_t *result_pool)
> > +{
> > + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> > + "diff --git a/%s b/%s%s",
> > + copyfrom_path, path, APR_EOL_STR));
> > + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> > + "copy from %s%s", path, APR_EOL_STR));
> ^^^^ ^^^^^
> > + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> > + "copy to %s%s", copyfrom_path,
> ^^^ ^^^^
> > + APR_EOL_STR));
>
> Shouldn't the copyfrom_path be printed for copy from, and the path for
> copy to?
>
> > +static svn_error_t *
> > +print_git_diff_header_moved(svn_stream_t *os, const char *header_encoding,
> > + const char *path, const char *copyfrom_path,
> > + apr_pool_t *result_pool)
> > +{
> > + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> > + "diff --git a/%s b/%s%s",
> > + copyfrom_path, path, APR_EOL_STR));
> > + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> > + "rename from %s%s", path,
> > + APR_EOL_STR));
> > + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> > + "rename to %s%s", copyfrom_path,
> > + APR_EOL_STR));
>
> Same here.
Doh, fixed in r952344.
Thanks,
Daniel
Re: svn commit: r952205 -
/subversion/trunk/subversion/libsvn_client/diff.c
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Jun 07, 2010 at 12:43:19PM -0000, dannas@apache.org wrote:
> Author: dannas
> Date: Mon Jun 7 12:43:19 2010
> New Revision: 952205
>
> URL: http://svn.apache.org/viewvc?rev=952205&view=rev
> Log:
> Use several smaller functions for printing git diff headers instead of one.
>
> * subversion/libsvn_client/diff.c
> (print_git_diff_header): Split this one into ...
> (print_git_diff_header_added): ..this ...
> (print_git_diff_header_deleted): .. and this ...
> (print_git_diff_header_copied): .. and this ...
> (print_git_diff_header_moved): .. and this ...
> (print_git_diff_header_modified): .. and this.
> (diff_content_changed): Adjust caller.
>
> Modified:
> subversion/trunk/subversion/libsvn_client/diff.c
> +/*
> + * Print a git diff header for PATH to the stream OS using HEADER_ENCODING.
> + * COPYFROM_PATH is the origin of the operation. All allocations are done
> + * in RESULT_POOL. */
> +static svn_error_t *
> +print_git_diff_header_copied(svn_stream_t *os, const char *header_encoding,
> + const char *path, const char *copyfrom_path,
> + apr_pool_t *result_pool)
> +{
> + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> + "diff --git a/%s b/%s%s",
> + copyfrom_path, path, APR_EOL_STR));
> + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> + "copy from %s%s", path, APR_EOL_STR));
^^^^ ^^^^^
> + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> + "copy to %s%s", copyfrom_path,
^^^ ^^^^
> + APR_EOL_STR));
Shouldn't the copyfrom_path be printed for copy from, and the path for
copy to?
> + return SVN_NO_ERROR;
> +}
> +
> +/*
> + * Print a git diff header for PATH to the stream OS using HEADER_ENCODING.
> + * COPYFROM_PATH is the origin of the operation. All allocations are done
> + * in RESULT_POOL. */
> +static svn_error_t *
> +print_git_diff_header_moved(svn_stream_t *os, const char *header_encoding,
> + const char *path, const char *copyfrom_path,
> + apr_pool_t *result_pool)
> +{
> + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> + "diff --git a/%s b/%s%s",
> + copyfrom_path, path, APR_EOL_STR));
> + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> + "rename from %s%s", path,
> + APR_EOL_STR));
> + SVN_ERR(svn_stream_printf_from_utf8(os, header_encoding, result_pool,
> + "rename to %s%s", copyfrom_path,
> + APR_EOL_STR));
Same here.
Stefan
Re: svn commit: r952205 -
/subversion/trunk/subversion/libsvn_client/diff.c
Posted by Daniel Näslund <da...@longitudo.com>.
On Mon, Jun 07, 2010 at 03:41:27PM +0200, Stefan Sperling wrote:
> On Mon, Jun 07, 2010 at 12:43:19PM -0000, dannas@apache.org wrote:
> > Author: dannas
> > Date: Mon Jun 7 12:43:19 2010
> > New Revision: 952205
> >
> > URL: http://svn.apache.org/viewvc?rev=952205&view=rev
> > Log:
> > Use several smaller functions for printing git diff headers instead of one.
> >
> > * subversion/libsvn_client/diff.c
> > (print_git_diff_header): Split this one into ...
> > (print_git_diff_header_added): ..this ...
> > (print_git_diff_header_deleted): .. and this ...
> > (print_git_diff_header_copied): .. and this ...
> > (print_git_diff_header_moved): .. and this ...
> > (print_git_diff_header_modified): .. and this.
> > (diff_content_changed): Adjust caller.
>
>
> > @@ -681,42 +705,43 @@ diff_content_changed(const char *path,
>
> > + SVN_ERR(print_git_diff_header_deleted(
> > + os,
> > + diff_cmd_baton->header_encoding,
> > + path, subpool));
> > +
> > label1 = diff_label(apr_psprintf(subpool, "a/%s", path1), rev1,
> > subpool);
> > label2 = diff_label("/dev/null", rev2, subpool);
>
> You could make the print_git_diff_headet_* functions return the appropriate
> labels as output parameters. E.g:
>
> SVN_ERR(print_git_diff_header_deleted(
> &label1, &label2, os,
> diff_cmd_baton->header_encoding, path, subpool));
To create labels we need two paths (haven't fully understood why we have
two different paths. How can we invoke 'svn diff' to compare two
different paths to each other?) and two revisions. That's a lot of
parameter passing for only executing two lines. I do want to put the
label creation inside a function but doing it in the
print_git_diff_header_*() seems too bulky.
Hopefully I can change diff_label() once we use 'git diffs' everywhere
and just call it once.
Thanks,
Daniel
Re: svn commit: r952205 -
/subversion/trunk/subversion/libsvn_client/diff.c
Posted by Stefan Sperling <st...@elego.de>.
On Mon, Jun 07, 2010 at 12:43:19PM -0000, dannas@apache.org wrote:
> Author: dannas
> Date: Mon Jun 7 12:43:19 2010
> New Revision: 952205
>
> URL: http://svn.apache.org/viewvc?rev=952205&view=rev
> Log:
> Use several smaller functions for printing git diff headers instead of one.
>
> * subversion/libsvn_client/diff.c
> (print_git_diff_header): Split this one into ...
> (print_git_diff_header_added): ..this ...
> (print_git_diff_header_deleted): .. and this ...
> (print_git_diff_header_copied): .. and this ...
> (print_git_diff_header_moved): .. and this ...
> (print_git_diff_header_modified): .. and this.
> (diff_content_changed): Adjust caller.
> @@ -681,42 +705,43 @@ diff_content_changed(const char *path,
> + SVN_ERR(print_git_diff_header_deleted(
> + os,
> + diff_cmd_baton->header_encoding,
> + path, subpool));
> +
> label1 = diff_label(apr_psprintf(subpool, "a/%s", path1), rev1,
> subpool);
> label2 = diff_label("/dev/null", rev2, subpool);
You could make the print_git_diff_headet_* functions return the appropriate
labels as output parameters. E.g:
SVN_ERR(print_git_diff_header_deleted(
&label1, &label2, os,
diff_cmd_baton->header_encoding, path, subpool));
Stefan