[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r12025 - in trunk/subversion: libsvn_client tests/clients/cmdline

From: <kfogel_at_collab.net>
Date: 2004-11-25 00:40:22 CET

lundblad@tigris.org writes:
> Log:
> Fix issue #2064. Merge prop changes before content changes so keyword
> expansion, special file handling etc. work.
>
> * subversion/libsvn_client/repos_diff.c (edit_baton): Use
> svn_wc_diff_callbacks2_t instead of svn_wc_diff_callbacks_t. Add empty_hash
> member.
> (delete_entry): Add properties to file_deleted call.
> (add_file): Set pristine_props to empty hash.
> (close_file): INclude properties in callback calls and call them for both
> file and prop changes.
> (close_directory): props_changed -> dir_props_changed. Update comment.
> (svn_client__get_diff_editor): Use new callbacks vtable. INitialize
> eb->empty_hash.
> * subversion/libsvn_client/client.h (svn_client__get_diff_editor): Use new
> callbacks vtable.
>
> * subversion/libsvn_client/diff.c (diff_props_changed, merge_props_changed):
> Move above using functions.
> (diff_content_changed): New function, extracted from diff_file_changed.
> (diff_file_changed): Implemented in terms of diff_contents_changed and
> diff_props_changed. Adjust to new callbacks API.
> (diff_file_added, diff_file_deleted_with_diff, diff_file_deleted_no_diff):
> Adjust to new callback API.
> (merge_file_changed): Adjust to new callbacks API. Merge prop changes
> as well as content changes.
> (merge_file_added): Adjust to new callbacks API. Provide new properties
> when adding or modifying the file.
> (merge_file_deleted): Adjust to new callbacks API.
> (merge_callbacks): Change type to svn_wc_diff_callbacks2_t.
> (do_merge): Take svn_wc_diff_callbacks2_t.
> (do_single_file_merge): Don't call merge_props_changed, but instead add
> propchanges to merge_file_changed call.
> (diff_wc_wc): Take svn_wc_diff_callbacks2_t. Use svn_wc_diff3 instead
> of diff2.
> (diff_repos_repos): Take svn_wc_diff_callbacks2_t.
> (diff_repos_wc): Take svn_wc_diff_callbacks2_t. Use svn_wc_get_diff_editor3.
> (do_diff, do_diff_peg): Take svn_wc_diff_callbacks2_t.
> (svn_client_diff, svn_client_diff_peg): Use svn_wc_diff_callbacks2_t.
>
> * subversion/tests/clients/cmdline/merge_tests.py (merge_keyword_expansions):
> --- trunk/subversion/libsvn_client/diff.c (original)
> +++ trunk/subversion/libsvn_client/diff.c Wed Nov 24 16:10:30 2004
> @@ -274,20 +274,43 @@
> return label;
> }
>
> +static svn_error_t *
> +diff_props_changed (svn_wc_adm_access_t *adm_access,
> + svn_wc_notify_state_t *state,
> + const char *path,
> + const apr_array_header_t *propchanges,
> + apr_hash_t *original_props,
> + void *diff_baton)
> +{
> + struct diff_cmd_baton *diff_cmd_baton = diff_baton;
> + apr_array_header_t *props;
> + apr_pool_t *subpool = svn_pool_create (diff_cmd_baton->pool);
> +
> + SVN_ERR (svn_categorize_props (propchanges, NULL, NULL, &props, subpool));
> +
> + if (props->nelts > 0)
> + SVN_ERR (display_prop_diffs (props, original_props, path,
> + diff_cmd_baton->outfile, subpool));
> +
> + if (state)
> + *state = svn_wc_notify_state_unknown;
> +
> + svn_pool_destroy (subpool);
> + return SVN_NO_ERROR;
> +}

Can you document static functions too, please? It's enough to say

  /* This implements the 'svn_wc_diff_callbacks2_t.dir_props_changed'
     interface. */

That tells the reader everything they need to know.

> /* The main workhorse, which invokes an external 'diff' program on the
> two temporary files. The path is the "true" label to use in the
> diff output. */
> static svn_error_t *
> -diff_file_changed (svn_wc_adm_access_t *adm_access,
> - svn_wc_notify_state_t *state,
> - const char *path,
> - const char *tmpfile1,
> - const char *tmpfile2,
> - svn_revnum_t rev1,
> - svn_revnum_t rev2,
> - const char *mimetype1,
> - const char *mimetype2,
> - void *diff_baton)
> +diff_content_changed (const char *path,
> + const char *tmpfile1,
> + const char *tmpfile2,
> + svn_revnum_t rev1,
> + svn_revnum_t rev2,
> + const char *mimetype1,
> + const char *mimetype2,
> + void *diff_baton)

This function should of course have a real doc string. If it is very
similar to the interface of 'svn_wc_diff_callbacks2_t.dir_props_changed',
then you could describe it in terms of that. (I realize you were
handed a legacy doc string, in a sense.)

> {
> struct diff_cmd_baton *diff_cmd_baton = diff_baton;
> const char *diff_cmd = NULL;
> @@ -411,8 +434,6 @@
> }
>
> /* Exit early. */
> - if (state)
> - *state = svn_wc_notify_state_unknown;
> svn_pool_destroy (subpool);
> return SVN_NO_ERROR;
> }
> @@ -485,21 +506,47 @@
> to need to write a diff plug-in mechanism that makes use of the
> two paths, instead of just blindly running SVN_CLIENT_DIFF. */
>
> - if (state)
> - *state = svn_wc_notify_state_unknown;
> -
> /* Destroy the subpool. */
> svn_pool_destroy (subpool);
>
> return SVN_NO_ERROR;
> }
>
> +static svn_error_t *
> +diff_file_changed (svn_wc_adm_access_t *adm_access,
> + svn_wc_notify_state_t *content_state,
> + svn_wc_notify_state_t *prop_state,
> + const char *path,
> + const char *tmpfile1,
> + const char *tmpfile2,
> + svn_revnum_t rev1,
> + svn_revnum_t rev2,
> + const char *mimetype1,
> + const char *mimetype2,
> + const apr_array_header_t *prop_changes,
> + apr_hash_t *original_props,
> + void *diff_baton)

Similar comments apply here too, of course.

And elsewhere in the file, also in the merge functions. I won't
mention them all here, you know the ones. Again, I know some were
undocumented before you got here, it's just your commit that made me
notice. Whether you want to fix them is up to you <evil grin> :-).

> + /* Special case: if a binary file isn't locally modified, and is
> + exactly identical to the 'left' side of the merge, then don't
> + allow svn_wc_merge to produce a conflict. Instead, just
> + overwrite the working file with the 'right' side of the merge. */
> + if ((! has_local_mods)
> + && ((mimetype1 && svn_mime_type_is_binary (mimetype1))
> + || (mimetype2 && svn_mime_type_is_binary (mimetype2))))
> + {
> + svn_boolean_t same_contents;
> + /* ### someday, we should just be able to compare
> + identity-strings here. */
> + SVN_ERR (svn_io_files_contents_same_p (&same_contents,
> + older, mine, subpool));

identity-strings? What idea is this? :-)

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Thu Nov 25 00:43:23 2004

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.