I understand this is an unfinished patch, but can you please post with
a log message always? I don't know about the others, but I won't even
review a patch that doesn't have a log message, unless it's a trivial
web page fix or something. A patch like this is too hard to digest
without an initial summary to guide me.
1 person is posting, N people are reviewing. If you write a log
message that on average saves a person T minutes of review time, you
save the project N*T time in total :-).
-Karl
Martin Hauner <hauner@web.de> writes:
> i created a partial patch for diff summarize, Issue 2015.
>
> This patch is far from finished and a little dirty at some places.
> It could be a starting point if this is an acceptable solution.
>
> 'svn diff -s .' will print the summarize for modified files
> (adds and deletes are not implemented).
>
> It doesn't make much sense for this case (being nearly the same as
> svn status) but i think it is enough for demonstration of the
> general idea.
>
> I created a new api function svn_client_diff_summarize with a
> callback function that prints the summarize to the command line.
>
> svn_client_diff_summarize calls do_diff with svn_wc_diff_callbacks2_t
> functions that delegate back to the summarize callback.
>
>
> To make it cleaner the diff_cmd_baton would need new members to
> propagate the summarize callback and its baton to the
> svn_wc_diff_callbacks2_t functions.
>
> Another topic is that #2015 wants to list the author. The
> svn_wc_diff_callbacks2_t functions don't take an author parameter.
> That would have to be added.
>
> I wonder if the author is really necessary. A normal diff doesn't
> output an author so why should summarize?
>
> But maybe adding the author to the diff output is interesting too?
>
>
> --
> Martin
>
> Subcommander, http://subcommander.tigris.org
> a x-platform Win32/Unix/Linux/MacOSX svn gui client & a text diff/merge tool.
> Index: subversion/include/svn_client.h
> ===================================================================
> --- subversion/include/svn_client.h (revision 14883)
> +++ subversion/include/svn_client.h (working copy)
> @@ -1111,6 +1111,70 @@
> svn_client_ctx_t *ctx,
> apr_pool_t *pool);
>
> +/**
> + * svn diff summarize
> + */
> +
> +typedef enum svn_diff_summarize_kind
> +{
> + /** an added item */
> + svn_diff_summarize_kind_add,
> +
> + /** a modified item */
> + svn_diff_summarize_kind_modified,
> +
> + /** a deleted item */
> + svn_diff_summarize_kind_deleted
> +
> +} svn_diff_summarize_kind_t;
> +
> +
> +typedef struct svn_diff_summarize_t
> +{
> + /** item path, relative to svn_client_diff_summarize path1/path2 */
> + const char* path;
> +
> + /** file or dir */
> + svn_node_kind_t node_kind;
> +
> + /** change kind */
> + svn_diff_summarize_kind_t sum_kind;
> +
> + /** revision prior to changes */
> + svn_revnum_t prior_revision;
> +
> + /** revision after all changes */
> + svn_revnum_t after_revision;
> +
> + /** properties changed? */
> + svn_boolean_t props_changed;
> +
> + /** pool */
> + apr_pool_t* pool;
> +
> +} svn_diff_summarize_t;
> +
> +
> +typedef void (*svn_diff_summarize_func_t) (void *summarize_baton,
> + svn_diff_summarize_t *diff);
> +
> +
> +svn_error_t *
> +svn_client_diff_summarize (const apr_array_header_t *options,
> + const char *path1,
> + const svn_opt_revision_t *revision1,
> + const char *path2,
> + const svn_opt_revision_t *revision2,
> + svn_boolean_t recurse,
> + svn_boolean_t ignore_ancestry,
> + svn_boolean_t no_diff_deleted,
> + svn_boolean_t ignore_content_type,
> + svn_diff_summarize_func_t summarize_func,
> + void *summarize_baton,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool);
> +
> +
> /** Merge changes from @a source1/@a revision1 to @a source2/@a revision2 into
> * the working-copy path @a target_wcpath.
> *
> Index: subversion/libsvn_client/diff.c
> ===================================================================
> --- subversion/libsvn_client/diff.c (revision 14883)
> +++ subversion/libsvn_client/diff.c (working copy)
> @@ -2628,3 +2628,181 @@
>
> return SVN_NO_ERROR;
> }
> +
> +/*-----------------------------------------------------------------*/
> +
> +/*** possible solution for svn diff summarize ***/
> +
> +/* A svn_wc_diff_callbacks2_t function. Used for both file and directory
> + property merges. */
> +static svn_error_t *
> +summarize_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 *baton)
> +{
> + return SVN_NO_ERROR;
> +}
> +
> +/* A svn_wc_diff_callbacks2_t function. */
> +static svn_error_t *
> +summarize_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 *mine,
> + const char *older,
> + const char *yours,
> + svn_revnum_t older_rev,
> + svn_revnum_t yours_rev,
> + const char *mimetype1,
> + const char *mimetype2,
> + const apr_array_header_t *prop_changes,
> + apr_hash_t *original_props,
> + void *baton)
> +{
> + struct diff_cmd_baton* diff_cmd_baton = baton;
> +
> + svn_diff_summarize_func_t summarize_func = (svn_diff_summarize_func_t)diff_cmd_baton->outfile;
> + void* summarize_baton = diff_cmd_baton->errfile;
> +
> + svn_diff_summarize_t summarize;
> +
> + summarize.node_kind = svn_node_file;
> + summarize.sum_kind = svn_diff_summarize_kind_modified;
> + summarize.prior_revision = older_rev;
> + summarize.after_revision = yours_rev;
> + summarize.path = mine;
> + summarize.props_changed = prop_changes->nelts > 0 ? TRUE : FALSE;
> + summarize.pool = diff_cmd_baton->pool;
> +
> +
> +
> + summarize_func( diff_cmd_baton->errfile, &summarize );
> +
> + return SVN_NO_ERROR;
> +}
> +
> +/* A svn_wc_diff_callbacks2_t function. */
> +static svn_error_t *
> +summarize_file_added (svn_wc_adm_access_t *adm_access,
> + svn_wc_notify_state_t *content_state,
> + svn_wc_notify_state_t *prop_state,
> + const char *mine,
> + const char *older,
> + const char *yours,
> + 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 *baton)
> +{
> + return SVN_NO_ERROR;
> +}
> +
> +/* A svn_wc_diff_callbacks2_t function. */
> +static svn_error_t *
> +summarize_file_deleted_no_diff (svn_wc_adm_access_t *adm_access,
> + svn_wc_notify_state_t *state,
> + const char *mine,
> + const char *older,
> + const char *yours,
> + const char *mimetype1,
> + const char *mimetype2,
> + apr_hash_t *original_props,
> + void *baton)
> +{
> + return SVN_NO_ERROR;
> +}
> +
> +static svn_error_t *
> +summarize_file_deleted_with_diff (svn_wc_adm_access_t *adm_access,
> + svn_wc_notify_state_t *state,
> + const char *mine,
> + const char *older,
> + const char *yours,
> + const char *mimetype1,
> + const char *mimetype2,
> + apr_hash_t *original_props,
> + void *baton)
> +{
> + return SVN_NO_ERROR;
> +}
> +
> +
> +/* A svn_wc_diff_callbacks2_t function. */
> +static svn_error_t *
> +summarize_dir_added (svn_wc_adm_access_t *adm_access,
> + svn_wc_notify_state_t *state,
> + const char *path,
> + svn_revnum_t rev,
> + void *baton)
> +{
> + return SVN_NO_ERROR;
> +}
> +
> +/* A svn_wc_diff_callbacks2_t function. */
> +static svn_error_t *
> +summarize_dir_deleted (svn_wc_adm_access_t *adm_access,
> + svn_wc_notify_state_t *state,
> + const char *path,
> + void *baton)
> +{
> + return SVN_NO_ERROR;
> +}
> +
> +
> +svn_error_t *
> +svn_client_diff_summarize ( const apr_array_header_t *options,
> + const char *path1,
> + const svn_opt_revision_t *revision1,
> + const char *path2,
> + const svn_opt_revision_t *revision2,
> + svn_boolean_t recurse,
> + svn_boolean_t ignore_ancestry,
> + svn_boolean_t no_diff_deleted,
> + svn_boolean_t ignore_content_type,
> + svn_diff_summarize_func_t summarize_func,
> + void *summarize_baton,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + struct diff_cmd_baton summarize_cmd_baton;
> + svn_wc_diff_callbacks2_t summarize_callbacks;
> +
> + summarize_callbacks.file_changed = summarize_file_changed;
> + summarize_callbacks.file_added = summarize_file_added;
> + summarize_callbacks.file_deleted = no_diff_deleted
> + ? summarize_file_deleted_no_diff
> + : summarize_file_deleted_with_diff;
> +
> + summarize_callbacks.dir_added = summarize_dir_added;
> + summarize_callbacks.dir_deleted = summarize_dir_deleted;
> + summarize_callbacks.dir_props_changed = summarize_props_changed;
> +
> + summarize_cmd_baton.orig_path_1 = path1;
> + summarize_cmd_baton.orig_path_2 = path2;
> +
> + summarize_cmd_baton.options = options;
> + summarize_cmd_baton.pool = pool;
> + summarize_cmd_baton.outfile = (apr_file_t*)summarize_func;
> + summarize_cmd_baton.errfile = summarize_baton;
> + summarize_cmd_baton.revnum1 = SVN_INVALID_REVNUM;
> + summarize_cmd_baton.revnum2 = SVN_INVALID_REVNUM;
> +
> + summarize_cmd_baton.config = ctx->config;
> + summarize_cmd_baton.force_empty = FALSE;
> + summarize_cmd_baton.force_binary = ignore_content_type;
> +
> + return do_diff (options,
> + path1, revision1,
> + path2, revision2,
> + recurse,
> + ignore_ancestry,
> + &summarize_callbacks, &summarize_cmd_baton,
> + ctx,
> + pool);
> +}
> Index: subversion/clients/cmdline/cl.h
> ===================================================================
> --- subversion/clients/cmdline/cl.h (revision 14883)
> +++ subversion/clients/cmdline/cl.h (working copy)
> @@ -141,6 +141,7 @@
> svn_boolean_t autoprops; /* enable automatic properties */
> svn_boolean_t no_autoprops; /* disable automatic properties */
> const char *native_eol; /* override system standard eol marker */
> + svn_boolean_t summarize; /* show a diff summarize */
> } svn_cl__opt_state_t;
>
>
> Index: subversion/clients/cmdline/diff-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/diff-cmd.c (revision 14883)
> +++ subversion/clients/cmdline/diff-cmd.c (working copy)
> @@ -22,6 +22,7 @@
>
> /*** Includes. ***/
>
> +#include "svn_cmdline.h"
> #include "svn_pools.h"
> #include "svn_client.h"
> #include "svn_string.h"
> @@ -35,6 +36,51 @@
>
> /*** Code. ***/
>
> +static char
> +generate_summarize_code (enum svn_diff_summarize_kind kind)
> +{
> + switch (kind)
> + {
> + case svn_diff_summarize_kind_add: return 'A';
> + case svn_diff_summarize_kind_modified: return 'M';
> + case svn_diff_summarize_kind_deleted: return 'D';
> + default: return '?';
> + }
> +}
> +
> +void summarize_func( void *summarize_baton, svn_diff_summarize_t *diff )
> +{
> + const char *prior_rev;
> + const char *after_rev;
> +
> + if ( SVN_IS_VALID_REVNUM(diff->prior_revision) )
> + {
> + prior_rev = apr_psprintf (diff->pool, "%ld", diff->prior_revision);
> + }
> + else
> + {
> + prior_rev = "?";
> + }
> +
> + if ( SVN_IS_VALID_REVNUM(diff->after_revision) )
> + {
> + after_rev = apr_psprintf (diff->pool, "%ld", diff->after_revision);
> + }
> + else
> + {
> + after_rev = "?";
> + }
> +
> + /*SVN_ERR*/ (svn_cmdline_printf (diff->pool, "%c%c%c %6s %6s %s\n",
> + generate_summarize_code(diff->sum_kind),
> + diff->props_changed ? 'M' : ' ',
> + diff->node_kind == svn_node_dir ? 'F' : ' ',
> + prior_rev,
> + after_rev,
> + diff->path));
> +}
> +
> +
> /* An svn_opt_subcommand_t to handle the 'diff' command.
> This implements the `svn_opt_subcommand_t' interface. */
> svn_error_t *
> @@ -182,20 +228,39 @@
> svn_pool_clear (subpool);
> target1 = svn_path_join (old_target, path, subpool);
> target2 = svn_path_join (new_target, path, subpool);
> -
> - SVN_ERR (svn_client_diff2 (options,
> - target1,
> - &(opt_state->start_revision),
> - target2,
> - &(opt_state->end_revision),
> - opt_state->nonrecursive ? FALSE : TRUE,
> - opt_state->notice_ancestry ? FALSE : TRUE,
> - opt_state->no_diff_deleted,
> - opt_state->force,
> - outfile,
> - errfile,
> - ((svn_cl__cmd_baton_t *)baton)->ctx,
> - pool));
> +
> + if (! opt_state->summarize)
> + {
> + SVN_ERR (svn_client_diff2 (options,
> + target1,
> + &(opt_state->start_revision),
> + target2,
> + &(opt_state->end_revision),
> + opt_state->nonrecursive ? FALSE : TRUE,
> + opt_state->notice_ancestry ? FALSE : TRUE,
> + opt_state->no_diff_deleted,
> + opt_state->force,
> + outfile,
> + errfile,
> + ((svn_cl__cmd_baton_t *)baton)->ctx,
> + pool));
> + }
> + else
> + {
> + SVN_ERR (svn_client_diff_summarize
> + (options,
> + target1,
> + &(opt_state->start_revision),
> + target2,
> + &(opt_state->end_revision),
> + opt_state->nonrecursive ? FALSE : TRUE,
> + opt_state->notice_ancestry ? FALSE : TRUE,
> + opt_state->no_diff_deleted,
> + opt_state->force,
> + summarize_func,0,
> + ((svn_cl__cmd_baton_t *)baton)->ctx,
> + pool));
> + }
> }
> else
> {
> Index: subversion/clients/cmdline/main.c
> ===================================================================
> --- subversion/clients/cmdline/main.c (revision 14883)
> +++ subversion/clients/cmdline/main.c (working copy)
> @@ -149,6 +149,8 @@
> N_("maximum number of log entries")},
> {"no-unlock", svn_cl__no_unlock_opt, 0,
> N_("don't unlock the targets")},
> + {"summarize", 's', 0,
> + N_("summarize diffs")},
> {0, 0, 0, 0}
> };
>
> @@ -300,7 +302,7 @@
> {'r', svn_cl__old_cmd_opt, svn_cl__new_cmd_opt, 'N',
> svn_cl__diff_cmd_opt, 'x', svn_cl__no_diff_deleted,
> svn_cl__notice_ancestry_opt, svn_cl__force_opt, SVN_CL__AUTH_OPTIONS,
> - svn_cl__config_dir_opt} },
> + svn_cl__config_dir_opt, 's'} },
>
> { "export", svn_cl__export, {0},
> N_("Create an unversioned copy of a tree.\n"
> @@ -1104,6 +1106,8 @@
> case svn_cl__no_unlock_opt:
> opt_state.no_unlock = TRUE;
> break;
> + case 's':
> + opt_state.summarize = TRUE;
> default:
> /* Hmmm. Perhaps this would be a good place to squirrel away
> opts that commands like svn diff might need. Hmmm indeed. */
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jun 1 21:18:04 2005