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

Re: [PATCH] "svnlook diff" option to filter files revisited

From: Pete Gonzalez <pgonzalez_at_bluel.com>
Date: 2007-04-07 22:36:55 CEST

Hyrum K. Wright wrote:
> Thanks for the patch! Although I haven't had a chance to review it, I
> do have a couple of suggestions. First, your patch stands a much better
> chance of getting some attention from a reviewer if you include a log
> message along with it. See the relevant portions of HACKING[1] for more
> details.

[[[
Added a new option "--no-diff" to the "svnlook diff" subcommand

    * subversion/subversion/svnlook/main.c
      (print_diff_tree, do_diff): Added no_diff_patterns parameter.
      (get_ctxt_baton): Added no_diff member to svnlook_ctxt_t.
]]]

And here are some more detailed notes:

This patch adds a new option --no-diff to the "svnlook diff" subcommand. This
new option excludes filenames that match a list of regular expressions. It is
similar to the options --no-diff-deleted and --no-diff-added which exclude newly
added/deleted files.

For example, if you wanted to generate a report of the changes in revision 1234,
but wanted to exclude Visual Studio machine generated files, you might type
something like this:

   svnlook diff the_repository -r 1234 \
     --no-diff "*.resx *.Designer.cs *.csproj *.vcproj *.sln"

> Second, when referring to previous threads on the list, it is much
> easier for the readers if you just post the address to the relevant
> message in the list archive. Most of the developers use the archive
> here[2]. Otherwise, it may be difficult to see where the old
> conversation ends and the new one begins, and your patch may get lost in
> the noise.

For reference, here is the original thread:
http://svn.haxx.se/dev/archive-2005-11/0994.shtml

Karl Fogel wrote:
> Are there any arguments for 'svnlook diff --no-diff "..."' that
> wouldn't also apply to 'svn diff --no-diff "..."' ? (In other words,
> wouldn't we patch both?)

This suggestion makes sense to me, and I would be willing to be the person who
implements it if the other patch is accepted. However, I should note that
personally I would rarely choose to filter "svn diff", and if I did the rules
would be somewhat different.

> Further thoughts: would an 'svn:no-diff' property do this better, so
> people wouldn't have to construct the option's parameter all the time?
> A --force flag could override it. Perhaps it could co-exist with the
> --no-diff option.

In my case, svnlook is being run from a shell script that supplies the options.
  The shell script is interested in different things than an ordinary person
doing their daily work, and I would find it awkward to modify the entire
repository database to accommodate one shell script. :-) However, I can see
how the "svn:no-diff" attribute might be useful to an administrator who wants to
provide system-wide defaults, especially if the upcoming "Repository-defined
autoprops" feature were available.

> Yes, I'm engaging in rampant speculation here. But I'd like us to
> identify the problem and think about all ways to solve it. Often a
> first patch turns out to be a subset of what's really called for.

No problem -- I'm open to any other suggestions or feedback.

Cheers,
-Pete

--- subversion/svnlook/main.c.orig 2007-04-07 13:22:32.589579944 -0400
+++ subversion/svnlook/main.c 2007-04-07 13:41:04.750505784 -0400
@@ -75,6 +75,7 @@
     svnlook__show_ids,
     svnlook__no_diff_deleted,
     svnlook__no_diff_added,
+ svnlook__no_diff,
     svnlook__diff_copy_from,
     svnlook__revprop_opt,
     svnlook__full_paths,
@@ -113,6 +114,9 @@
   {"no-diff-added", svnlook__no_diff_added, 0,
    N_("do not print differences for added files")},
 
+ {"no-diff", svnlook__no_diff, 1,
+ N_("do not print differences for files matching ARG")},
+
   {"diff-copy-from", svnlook__diff_copy_from, 0,
    N_("print differences against the copy source")},
 
@@ -158,7 +162,7 @@
    N_("usage: svnlook diff REPOS_PATH\n\n"
       "Print GNU-style diffs of changed files and properties.\n"),
    {'r', 't', svnlook__no_diff_deleted, svnlook__no_diff_added,
- svnlook__diff_copy_from} },
+ svnlook__no_diff, svnlook__diff_copy_from} },
 
   {"dirs-changed", subcommand_dirschanged, {0},
    N_("usage: svnlook dirs-changed REPOS_PATH\n\n"
@@ -238,6 +242,7 @@
   svn_boolean_t help; /* --help */
   svn_boolean_t no_diff_deleted; /* --no-diff-deleted */
   svn_boolean_t no_diff_added; /* --no-diff-added */
+ const char *no_diff; /* --no-diff */
   svn_boolean_t diff_copy_from; /* --diff-copy-from */
   svn_boolean_t verbose; /* --verbose */
   svn_boolean_t revprop; /* --revprop */
@@ -254,6 +259,7 @@
   svn_boolean_t show_ids;
   svn_boolean_t no_diff_deleted;
   svn_boolean_t no_diff_added;
+ const char *no_diff;
   svn_boolean_t diff_copy_from;
   svn_boolean_t full_paths;
   svn_boolean_t copy_info;
@@ -780,6 +786,7 @@
                 const char *path /* UTF-8! */,
                 const char *base_path /* UTF-8! */,
                 const svnlook_ctxt_t *c,
+ apr_array_header_t *no_diff_patterns,
                 const char *tmpdir,
                 apr_pool_t *pool)
 {
@@ -892,10 +899,17 @@
 
   if (do_diff)
     {
+ int ignore_me = FALSE;
+
+ if (no_diff_patterns != NULL)
+ ignore_me = svn_cstring_match_glob_list (path, no_diff_patterns);
+
       SVN_ERR(svn_cmdline_printf(pool, "%s\n", equal_string));
       SVN_ERR(svn_cmdline_fflush(stdout));
 
- if (binary)
+ if (ignore_me)
+ SVN_ERR (svn_cmdline_printf (pool, _("(Skipping this file type)\n")));
+ else if (binary)
         SVN_ERR(svn_cmdline_printf(pool, _("(Binary files differ)\n")));
       else
         {
@@ -964,7 +978,7 @@
   SVN_ERR(print_diff_tree(root, base_root, node,
                           svn_path_join(path, node->name, subpool),
                           svn_path_join(base_path, node->name, subpool),
- c, tmpdir, subpool));
+ c, no_diff_patterns, tmpdir, subpool));
   while (node->sibling)
     {
       svn_pool_clear(subpool);
@@ -972,7 +986,7 @@
       SVN_ERR(print_diff_tree(root, base_root, node,
                               svn_path_join(path, node->name, subpool),
                               svn_path_join(base_path, node->name, subpool),
- c, tmpdir, subpool));
+ c, no_diff_patterns, tmpdir, subpool));
     }
   apr_pool_destroy(subpool);
 
@@ -1300,12 +1314,17 @@
   if (tree)
     {
       const char *tmpdir;
+ apr_array_header_t *no_diff_patterns = NULL;
+
+ /* parse the --no-diff patterns */
+ if (c->no_diff != NULL)
+ no_diff_patterns = svn_cstring_split ( c->no_diff, " ", TRUE, pool);
 
       SVN_ERR(svn_fs_revision_root(&base_root, c->fs, base_rev_id, pool));
       SVN_ERR(svn_io_temp_dir(&tmpdir, pool));
 
       SVN_ERR(print_diff_tree(root, base_root, tree, "", "",
- c, tmpdir, pool));
+ c, no_diff_patterns, tmpdir, pool));
     }
   return SVN_NO_ERROR;
 }
@@ -1547,6 +1566,7 @@
   baton->show_ids = opt_state->show_ids;
   baton->no_diff_deleted = opt_state->no_diff_deleted;
   baton->no_diff_added = opt_state->no_diff_added;
+ baton->no_diff = opt_state->no_diff;
   baton->diff_copy_from = opt_state->diff_copy_from;
   baton->full_paths = opt_state->full_paths;
   baton->copy_info = opt_state->copy_info;
@@ -1980,6 +2000,10 @@
           opt_state.no_diff_added = TRUE;
           break;
 
+ case svnlook__no_diff:
+ opt_state.no_diff = opt_arg;
+ break;
+
         case svnlook__diff_copy_from:
           opt_state.diff_copy_from = TRUE;
           break;

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Apr 7 22:38:17 2007

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.